SERVER-15541 Clean up some usages of WriteUnitOfWork

Removes some WriteUnitOfWork objects where they are unneeded, and
moves the declaration of some existing WriteUnitOfWork objects to
limit their scope to the write being performed (so that
yield-capable reads are not inside them).

10 files changed, 27 insertions, 50 deletions

diff --git a/src/mongo/db/commands/cpuprofile.cpp b/src/mongo/db/commands/cpuprofile.cpp
index d25b7c034cb..d279f4b51d1 100644
--- a/src/mongo/db/commands/cpuprofile.cpp
+++ b/src/mongo/db/commands/cpuprofile.cpp
@@ -153,11 +153,9 @@ namespace mongo {
                                           BSONObjBuilder &result,
                                           bool fromRepl ) {
             Lock::DBLock dbXLock(txn->lockState(), db, MODE_X);
-            WriteUnitOfWork wunit(txn);
             Client::Context ctx(txn, db);
 
             ::ProfilerStop();
-            wunit.commit();
             return true;
         }
 
diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp
index cf6832ba44b..190f37c32f7 100644
--- a/src/mongo/db/commands/mr.cpp
+++ b/src/mongo/db/commands/mr.cpp
@@ -386,7 +386,6 @@ namespace mongo {
             {
                 // copy indexes into temporary storage
                 Client::WriteContext finalCtx(_txn, _config.outputOptions.finalNamespace);
-                WriteUnitOfWork wuow(_txn);
                 Collection* const finalColl = finalCtx.getCollection();
                 if ( finalColl ) {
                     IndexCatalog::IndexIterator ii =
@@ -409,7 +408,6 @@ namespace mongo {
                         indexesToInsert.push_back( b.obj() );
                     }
                 }
-                wuow.commit();
             }
 
             {
@@ -603,10 +601,8 @@ namespace mongo {
                     Lock::DBLock lock(_txn->lockState(),
                                       nsToDatabaseSubstring(_config.outputOptions.finalNamespace),
                                       MODE_X);
-                    WriteUnitOfWork wunit(_txn);
                     BSONObj o = cursor->nextSafe();
                     Helpers::upsert( _txn, _config.outputOptions.finalNamespace , o );
-                    wunit.commit();
                     pm.hit();
                 }
                 _db.dropCollection( _config.tempNamespace );
@@ -622,7 +618,6 @@ namespace mongo {
                 auto_ptr<DBClientCursor> cursor = _db.query( _config.tempNamespace , BSONObj() );
                 while ( cursor->more() ) {
                     Lock::GlobalWrite lock(txn->lockState()); // TODO(erh) why global?
-                    WriteUnitOfWork wunit(txn);
                     BSONObj temp = cursor->nextSafe();
                     BSONObj old;
 
@@ -651,7 +646,6 @@ namespace mongo {
                     else {
                         Helpers::upsert( _txn, _config.outputOptions.finalNamespace , temp );
                     }
-                    wunit.commit();
                     pm.hit();
                 }
                 pm.finished();
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index 3b02c054866..61906bcb1e9 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -462,7 +462,6 @@ namespace mongo {
             }
 
             Lock::DBLock dbXLock(txn->lockState(), dbname, MODE_X);
-            WriteUnitOfWork wunit(txn);
             Client::Context ctx(txn, nsToDrop);
             Database* db = ctx.db();
 
@@ -480,6 +479,7 @@ namespace mongo {
             result.append( "ns", nsToDrop );
             result.append( "nIndexesWas", numIndexes );
 
+            WriteUnitOfWork wunit(txn);
             Status s = db->dropCollection( txn, nsToDrop );
 
             if ( !s.isOK() ) {
diff --git a/src/mongo/db/introspect.cpp b/src/mongo/db/introspect.cpp
index 1359abf0375..bbd5cf6c71a 100644
--- a/src/mongo/db/introspect.cpp
+++ b/src/mongo/db/introspect.cpp
@@ -126,14 +126,17 @@ namespace {
             p = b.done();
         }
 
+        WriteUnitOfWork wunit(txn);
+
         // write: not replicated
         // get or create the profiling collection
         Collection* profileCollection = getOrCreateProfileCollection(txn, db);
-        if ( profileCollection ) {
-            profileCollection->insertDocument( txn, p, false );
-            return true;
+        if ( !profileCollection ) {
+            return false;
         }
-        return false;
+        profileCollection->insertDocument( txn, p, false );
+        wunit.commit();
+        return true;
     }
 
     void profile(OperationContext* txn, const Client& c, int op, CurOp& currentOp) {
@@ -157,9 +160,7 @@ namespace {
                 }
                 Database* db = dbHolder().get(txn, dbname);
                 if (db != NULL) {
-                    // We want the profiling to happen in a different WUOW from the actual op.
                     Lock::CollectionLock clk(txn->lockState(), db->getProfilingNS(), MODE_X);
-                    WriteUnitOfWork wunit(txn);
                     Client::Context cx(txn, currentOp.getNS(), false);
                     if ( !_profile(txn, c, cx.db(), currentOp, profileBufBuilder ) && lk.get() ) {
                         if ( tryAgain ) {
@@ -170,7 +171,6 @@ namespace {
                         tryAgain = true;
                         continue;
                     }
-                    wunit.commit();
                 }
                 else {
                     mongo::log() << "note: not profiling because db went away - "
diff --git a/src/mongo/db/query/plan_yield_policy.cpp b/src/mongo/db/query/plan_yield_policy.cpp
index 7aa624d6ee9..fdf909152cc 100644
--- a/src/mongo/db/query/plan_yield_policy.cpp
+++ b/src/mongo/db/query/plan_yield_policy.cpp
@@ -42,6 +42,7 @@ namespace mongo {
           _planYielding(exec) { }
 
     bool PlanYieldPolicy::shouldYield() {
+        invariant(!_planYielding->getOpCtx()->lockState()->inAWriteUnitOfWork());
         return _elapsedTracker.intervalHasElapsed();
     }
 
diff --git a/src/mongo/db/repl/bgsync.cpp b/src/mongo/db/repl/bgsync.cpp
index 13f286e7ebf..c07c1c651f4 100644
--- a/src/mongo/db/repl/bgsync.cpp
+++ b/src/mongo/db/repl/bgsync.cpp
@@ -521,11 +521,8 @@ namespace {
     long long BackgroundSync::_readLastAppliedHash(OperationContext* txn) {
         BSONObj oplogEntry;
         try {
-            // Uses WuoW because there is no way to demarcate a read transaction boundary.
             Lock::DBLock lk(txn->lockState(), "local", MODE_X);
-            WriteUnitOfWork uow(txn);
             bool success = Helpers::getLast(txn, rsoplog, oplogEntry);
-            uow.commit();
             if (!success) {
                 // This can happen when we are to do an initial sync.  lastHash will be set
                 // after the initial sync is complete.
diff --git a/src/mongo/db/repl/minvalid.cpp b/src/mongo/db/repl/minvalid.cpp
index 9aeb9e272d4..30a22a611ea 100644
--- a/src/mongo/db/repl/minvalid.cpp
+++ b/src/mongo/db/repl/minvalid.cpp
@@ -51,25 +51,19 @@ namespace {
 
     void clearInitialSyncFlag(OperationContext* txn) {
         Lock::DBLock lk(txn->lockState(), "local", MODE_X);
-        WriteUnitOfWork wunit(txn);
         Helpers::putSingleton(txn, minvalidNS, BSON("$unset" << initialSyncFlag));
-        wunit.commit();
     }
 
     void setInitialSyncFlag(OperationContext* txn) {
         Lock::DBLock lk(txn->lockState(), "local", MODE_X);
-        WriteUnitOfWork wunit(txn);
         Helpers::putSingleton(txn, minvalidNS, BSON("$set" << initialSyncFlag));
-        wunit.commit();
     }
 
     bool getInitialSyncFlag() {
         OperationContextImpl txn;
         Lock::DBLock lk(txn.lockState(), "local", MODE_X);
-        WriteUnitOfWork uow( &txn );
         BSONObj mv;
         bool found = Helpers::getSingleton( &txn, minvalidNS, mv);
-        uow.commit();
 
         if (found) {
             return mv[initialSyncFlagString].trueValue();
@@ -79,25 +73,17 @@ namespace {
 
     void setMinValid(OperationContext* ctx, OpTime ts) {
         Lock::DBLock lk(ctx->lockState(), "local", MODE_X);
-        {
-            WriteUnitOfWork wunit(ctx);
-            Helpers::putSingleton(ctx, minvalidNS, BSON("$set" << BSON("ts" << ts)));
-            wunit.commit();
-        }
+        Helpers::putSingleton(ctx, minvalidNS, BSON("$set" << BSON("ts" << ts)));
     }
 
     OpTime getMinValid(OperationContext* txn) {
         Lock::DBLock lk(txn->lockState(), "local", MODE_S);
-        {
-            WriteUnitOfWork wunit(txn);
-            BSONObj mv;
-            bool found = Helpers::getSingleton(txn, minvalidNS, mv);
-            wunit.commit();
-            if (found) {
-                return mv["ts"]._opTime();
-            }
-            return OpTime();
+        BSONObj mv;
+        bool found = Helpers::getSingleton(txn, minvalidNS, mv);
+        if (found) {
+            return mv["ts"]._opTime();
         }
+        return OpTime();
     }
 
 }
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index cfd20791e5c..fd27a964e4d 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -121,9 +121,6 @@ namespace repl {
         */
     OpTime _logOpObjRS(OperationContext* txn, const BSONObj& op) {
         Lock::DBLock lk(txn->lockState(), "local", MODE_X);
-        // XXX soon this needs to be part of an outer WUOW not its own.
-        // We can't do this yet due to locking limitations.
-        WriteUnitOfWork wunit(txn);
 
         const OpTime ts = op["ts"]._opTime();
         long long hash = op["h"].numberLong();
@@ -140,6 +137,9 @@ namespace repl {
                         localOplogRSCollection);
             }
             Client::Context ctx(txn, rsoplog, localDB);
+            // TODO(geert): soon this needs to be part of an outer WUOW not its own.
+            // We can't do this yet due to locking limitations.
+            WriteUnitOfWork wunit(txn);
             checkOplogInsert(localOplogRSCollection->insertDocument(txn, op, false));
 
             ReplicationCoordinator* replCoord = getGlobalReplicationCoordinator();
@@ -149,6 +149,7 @@ namespace repl {
                          << myLastOptime << " newest timestamp: " << ts;
                 fassertFailedNoTrace(18905);
             }
+            wunit.commit();
             
             BackgroundSync* bgsync = BackgroundSync::get();
             // Keep this up-to-date, in case we step up to primary.
@@ -160,7 +161,6 @@ namespace repl {
         }
 
         setNewOptime(ts);
-        wunit.commit();
         return ts;
     }
 
@@ -237,7 +237,6 @@ namespace repl {
                          bool *bb,
                          bool fromMigrate ) {
         Lock::DBLock lk1(txn->lockState(), "local", MODE_X);
-        WriteUnitOfWork wunit(txn);
 
         if ( strncmp(ns, "local.", 6) == 0 ) {
             return;
@@ -299,6 +298,7 @@ namespace repl {
         }
 
         Client::Context ctx(txn, rsoplog, localDB);
+        WriteUnitOfWork wunit(txn);
         OplogDocWriter writer( partial, obj );
         checkOplogInsert( localOplogRSCollection->insertDocument( txn, &writer, false ) );
 
@@ -319,7 +319,6 @@ namespace repl {
                           bool *bb,
                           bool fromMigrate ) {
         Lock::DBLock lk(txn->lockState(), "local", MODE_X);
-        WriteUnitOfWork wunit(txn);
         static BufBuilder bufbuilder(8*1024); // todo there is likely a mutex on this constructor
 
         if ( strncmp(ns, "local.", 6) == 0 ) {
@@ -361,6 +360,7 @@ namespace repl {
         }
 
         Client::Context ctx(txn, logNS , localDB);
+        WriteUnitOfWork wunit(txn);
         OplogDocWriter writer( partial, obj );
         checkOplogInsert( localOplogMainCollection->insertDocument( txn, &writer, false ) );
 
@@ -448,7 +448,6 @@ namespace repl {
 
     void createOplog(OperationContext* txn) {
         Lock::GlobalWrite lk(txn->lockState());
-        WriteUnitOfWork uow( txn );
 
         const char * ns = "local.oplog.$main";
 
@@ -478,7 +477,6 @@ namespace repl {
 
             if ( !rs )
                 initOpTimeFromOplog(txn, ns);
-            uow.commit();
             return;
         }
 
@@ -517,6 +515,7 @@ namespace repl {
         options.cappedSize = sz;
         options.autoIndexId = CollectionOptions::NO;
 
+        WriteUnitOfWork uow( txn );
         invariant(ctx.db()->createCollection(txn, ns, options));
         if( !rs )
             logOp(txn, "n", "", BSONObj() );
diff --git a/src/mongo/db/repl/rs_initialsync.cpp b/src/mongo/db/repl/rs_initialsync.cpp
index d148e8049c5..51fd7e43b49 100644
--- a/src/mongo/db/repl/rs_initialsync.cpp
+++ b/src/mongo/db/repl/rs_initialsync.cpp
@@ -309,12 +309,12 @@ namespace {
 
         {
             AutoGetDb autodb(&txn, "local", MODE_X);
-            WriteUnitOfWork wunit(&txn);
             OpTime lastOpTimeWritten(getGlobalReplicationCoordinator()->getMyLastOptime());
             log() << "replSet set minValid=" << lastOpTimeWritten << rsLog;
 
             // Initial sync is now complete.  Flag this by setting minValid to the last thing
             // we synced.
+            WriteUnitOfWork wunit(&txn);
             setMinValid(&txn, lastOpTimeWritten);
 
             // Clear the initial sync flag.
diff --git a/src/mongo/db/repl/sync_source_feedback.cpp b/src/mongo/db/repl/sync_source_feedback.cpp
index 86460230bb2..ca15023b97e 100644
--- a/src/mongo/db/repl/sync_source_feedback.cpp
+++ b/src/mongo/db/repl/sync_source_feedback.cpp
@@ -79,7 +79,6 @@ namespace repl {
         string myname = getHostName();
         {
             Lock::DBLock dlk(txn->lockState(), "local", MODE_X);
-            WriteUnitOfWork wunit(txn);
             Client::Context ctx(txn, "local");
 
             // local.me is an identifier for a server for getLastError w:2+
@@ -87,6 +86,8 @@ namespace repl {
                 !_me.hasField("host") ||
                 _me["host"].String() != myname) {
 
+                WriteUnitOfWork wunit(txn);
+
                 // clean out local.me
                 Helpers::emptyCollection(txn, "local.me");
 
@@ -96,8 +97,9 @@ namespace repl {
                 b.append("host", myname);
                 _me = b.obj();
                 Helpers::putSingleton(txn, "local.me", _me);
+
+                wunit.commit();
             }
-            wunit.commit();
             // _me is used outside of a read lock, so we must copy it out of the mmap
             _me = _me.getOwned();
         }