SERVER-23351 ensure applier aquires new minvalid value after rollback

(cherry picked from commit 42cfa52297f1fc6d9fae8bb3bdf236389c800225)

2 files changed, 12 insertions, 15 deletions

diff --git a/src/mongo/db/repl/bgsync.cpp b/src/mongo/db/repl/bgsync.cpp
index 6c69a0a3269..e34c4f73335 100644
--- a/src/mongo/db/repl/bgsync.cpp
+++ b/src/mongo/db/repl/bgsync.cpp
@@ -698,7 +698,7 @@ void BackgroundSync::_rollback(OperationContext* txn,
     if (status.isOK()) {
         // When the syncTail thread sees there is no new data by adding something to the buffer.
         _signalNoNewDataForApplier();
-        // Wait until the buffer is emtpy.
+        // Wait until the buffer is empty.
         // This is an indication that syncTail has removed the sentinal marker from the buffer
         // and reset its local lastAppliedOpTime via the replCoord.
         while (!_buffer.empty()) {
@@ -707,6 +707,15 @@ void BackgroundSync::_rollback(OperationContext* txn,
                 return;
             }
         }
+
+        // It is now safe to clear the ROLLBACK state, which may result in the applier thread
+        // transitioning to SECONDARY.  This is safe because the applier thread has now reloaded
+        // the new rollback minValid from the database.
+        if (!_replCoord->setFollowerMode(MemberState::RS_RECOVERING)) {
+            warning() << "Failed to transition into " << MemberState(MemberState::RS_RECOVERING)
+                      << "; expected to be in state " << MemberState(MemberState::RS_ROLLBACK)
+                      << " but found self in " << _replCoord->getMemberState();
+        }
         return;
     }
     if (ErrorCodes::UnrecoverableRollbackError == status.code()) {
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index 076507b4106..d52032cb6a4 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -879,24 +879,12 @@ Status _syncRollback(OperationContext* txn,
     } catch (...) {
         replCoord->incrementRollbackID();
 
-        if (!replCoord->setFollowerMode(MemberState::RS_RECOVERING)) {
-            warning() << "Failed to transition into " << MemberState(MemberState::RS_RECOVERING)
-                      << "; expected to be in state " << MemberState(MemberState::RS_ROLLBACK)
-                      << " but found self in " << replCoord->getMemberState();
-        }
-
         throw;
     }
     replCoord->incrementRollbackID();
 
-    // success - leave "ROLLBACK" state
-    // can go to SECONDARY once minvalid is achieved
-    if (!replCoord->setFollowerMode(MemberState::RS_RECOVERING)) {
-        warning() << "Failed to transition into " << MemberState(MemberState::RS_RECOVERING)
-                  << "; expected to be in state " << MemberState(MemberState::RS_ROLLBACK)
-                  << " but found self in " << replCoord->getMemberState();
-    }
-
+    // Success; leave "ROLLBACK" state intact until applier thread has reloaded the new minValid.
+    // Otherwise, the applier could transition the node to SECONDARY with an out-of-date minValid.
     return Status::OK();
 }