SERVER-12516 Check for change in primary state when recovering from yield in CRUD operations.

Without this patch, a replicaset member running a long-running write operation
that yields might not notice, on yield recovery, that the node is no longer a
primary, and so no longer entitled to perform writes.

5 files changed, 52 insertions, 18 deletions

diff --git a/src/mongo/db/commands/write_commands/batch_executor.cpp b/src/mongo/db/commands/write_commands/batch_executor.cpp
index 1cff1890653..8a0d115d251 100644
--- a/src/mongo/db/commands/write_commands/batch_executor.cpp
+++ b/src/mongo/db/commands/write_commands/batch_executor.cpp
@@ -42,6 +42,7 @@
 #include "mongo/db/ops/update_lifecycle_impl.h"
 #include "mongo/db/ops/update_request.h"
 #include "mongo/db/pagefault.h"
+#include "mongo/db/repl/is_master.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/repl/replication_server_status.h"
 #include "mongo/db/repl/rs.h"
@@ -346,6 +347,17 @@ namespace mongo {
         return true;
     }
 
+    static bool checkIsMasterForCollection(const NamespaceString& ns, WriteErrorDetail** error) {
+        if (!isMasterNs(ns.ns().c_str())) {
+            WriteErrorDetail* errorDetail = *error = new WriteErrorDetail;
+            errorDetail->setErrCode(ErrorCodes::NotMaster);
+            errorDetail->setErrMessage(std::string(mongoutils::str::stream() <<
+                                                   "Not primary while writing to " << ns.ns()));
+            return false;
+        }
+        return true;
+    }
+
     static void buildUniqueIndexError( const BSONObj& keyPattern,
                                        const BSONObj& indexPattern,
                                        WriteErrorDetail* error ) {
@@ -712,7 +724,8 @@ namespace mongo {
 
                 // Check version inside of write lock
 
-                if ( checkShardVersion( &shardingState, request, &currResult.error )
+                if ( checkIsMasterForCollection( nss, &currResult.error )
+                     && checkShardVersion( &shardingState, request, &currResult.error )
                      && checkIndexConstraints( &shardingState, request, &currResult.error ) ) {
 
                     //
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index b081f1d7906..3aada48a438 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -612,10 +612,6 @@ namespace mongo {
 
         Lock::DBWrite lk(ns.ns());
 
-        // void ReplSetImpl::relinquish() uses big write lock so this is thus
-        // synchronized given our lock above.
-        uassert( 17010 ,  "not master", isMasterNs( ns.ns().c_str() ) );
-
         // if this ever moves to outside of lock, need to adjust check
         // Client::Context::_finishInit
         if ( ! broadcast && handlePossibleShardedMessage( m , 0 ) )
@@ -652,10 +648,7 @@ namespace mongo {
         while ( 1 ) {
             try {
                 Lock::DBWrite lk(ns.ns());
-                
-                // writelock is used to synchronize stepdowns w/ writes
-                uassert( 10056 ,  "not master", isMasterNs( ns.ns().c_str() ) );
-                
+
                 // if this ever moves to outside of lock, need to adjust check Client::Context::_finishInit
                 if ( ! broadcast && handlePossibleShardedMessage( m , 0 ) )
                     return;
diff --git a/src/mongo/db/ops/delete.cpp b/src/mongo/db/ops/delete.cpp
index 07f942e778f..d3794561cf7 100644
--- a/src/mongo/db/ops/delete.cpp
+++ b/src/mongo/db/ops/delete.cpp
@@ -30,10 +30,12 @@
 
 #include "mongo/db/client.h"
 #include "mongo/db/clientcursor.h"
+#include "mongo/db/curop.h"
 #include "mongo/db/catalog/database.h"
 #include "mongo/db/structure/catalog/namespace_details.h"
 #include "mongo/db/query/get_runner.h"
 #include "mongo/db/query/query_planner_common.h"
+#include "mongo/db/repl/is_master.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/catalog/collection.h"
 
@@ -70,6 +72,10 @@ namespace mongo {
 
         string nsForLogOp = ns.toString(); // XXX-ERH
 
+        uassert(ErrorCodes::NotMaster,
+                str::stream() << "Not primary while removing from " << ns,
+                !logop || isMasterNs(nsForLogOp.c_str()));
+
         long long nDeleted = 0;
 
         CanonicalQuery* cq;
@@ -96,8 +102,15 @@ namespace mongo {
 
         DiskLoc rloc;
         Runner::RunnerState state;
+        CurOp* curOp = cc().curop();
+        int oldYieldCount = curOp->numYields();
         while (Runner::RUNNER_ADVANCED == (state = runner->getNext(NULL, &rloc))) {
-
+            if (oldYieldCount != curOp->numYields()) {
+                uassert(ErrorCodes::NotMaster,
+                        str::stream() << "No longer primary while removing from " << ns,
+                        !logop || isMasterNs(nsForLogOp.c_str()));
+                oldYieldCount = curOp->numYields();
+            }
             BSONObj toDelete;
 
             // TODO: do we want to buffer docs and delete them in a group rather than
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 02309ad6eb7..333fccd4958 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -50,6 +50,7 @@
 #include "mongo/db/query/query_planner_common.h"
 #include "mongo/db/query/runner_yield_policy.h"
 #include "mongo/db/queryutil.h"
+#include "mongo/db/repl/is_master.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/storage/record.h"
 #include "mongo/db/catalog/collection.h"
@@ -359,14 +360,21 @@ namespace mongo {
             return Status::OK();
         }
 
-        Status recoverFromYield(UpdateLifecycle* lifecycle,
+        Status recoverFromYield(const UpdateRequest& request,
                                 UpdateDriver* driver,
-                                Collection* collection,
-                                const NamespaceString& nsString) {
+                                Collection* collection) {
+
+            const NamespaceString& nsString(request.getNamespaceString());
             // We yielded and recovered OK, and our cursor is still good. Details about
             // our namespace may have changed while we were yielded, so we re-acquire
             // them here. If we can't do so, escape the update loop. Otherwise, refresh
             // the driver so that it knows about what is currently indexed.
+
+            if (request.shouldCallLogOp() && !isMasterNs(nsString.ns().c_str())) {
+                return Status(ErrorCodes::NotMaster, mongoutils::str::stream() <<
+                              "Demoted from primary while performing update on " << nsString.ns());
+            }
+
             Collection* oldCollection = collection;
             collection = cc().database()->getCollection(nsString.ns());
 
@@ -398,8 +406,8 @@ namespace mongo {
                               "Update aborted due to invalid state transitions after yield -- "
                               "IndexCatalog not ok().");
 
-            if (lifecycle) {
-
+            if (request.getLifecycle()) {
+                UpdateLifecycle* lifecycle = request.getLifecycle();
                 lifecycle->setCollection(collection);
 
                 if (!lifecycle->canContinue()) {
@@ -549,6 +557,10 @@ namespace mongo {
         // Keep track of yield count so we can see if one happens on the getNext() calls below
         int oldYieldCount = curOp->numYields();
 
+        uassert(ErrorCodes::NotMaster,
+                mongoutils::str::stream() << "Not primary while updating " << nsString.ns(),
+                !request.shouldCallLogOp() || isMasterNs(nsString.ns().c_str()));
+
         while (true) {
             // See if we have a write in isolation mode
             isolationModeWriteOccured = isolated && (opDebug->nModified > 0);
@@ -569,7 +581,7 @@ namespace mongo {
             if (state != Runner::RUNNER_ADVANCED) {
                 if (state == Runner::RUNNER_EOF) {
                     if (didYield)
-                        uassertStatusOK(recoverFromYield(lifecycle, driver, collection, nsString));
+                        uassertStatusOK(recoverFromYield(request, driver, collection));
 
                     // We have reached the logical end of the loop, so do yielding recovery
                     break;
@@ -583,7 +595,7 @@ namespace mongo {
 
             // Refresh things after a yield.
             if (didYield)
-                uassertStatusOK(recoverFromYield(lifecycle, driver, collection, nsString));
+                uassertStatusOK(recoverFromYield(request, driver, collection));
 
             // We fill this with the new locs of moved doc so we don't double-update.
             // NOTE: The runner will de-dup non-moved things.
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 59ef6353d74..49f05e7a6ce 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -214,7 +214,10 @@ namespace mongo {
         const OpTime ts = OpTime::now(lk2);
         long long hashNew;
         if( theReplSet ) {
-            massert(13312, "replSet error : logOp() but not primary?", theReplSet->box.getState().primary());
+            if (!theReplSet->box.getState().primary()) {
+                log() << "replSet error : logOp() but not primary";
+                fassertFailed(0);
+            }
             hashNew = (theReplSet->lastH * 131 + ts.asLL()) * 17 + theReplSet->selfId();
         }
         else {