SERVER-25353 Clean shutdown shouldn't leave secondaries needing to apply a batchr3.2.9-rc0

3 files changed, 32 insertions, 13 deletions

diff --git a/jstests/replsets/clean_shutdown_oplog_state.js b/jstests/replsets/clean_shutdown_oplog_state.js
index 81c1c9cda5e..d5cd338a52b 100644
--- a/jstests/replsets/clean_shutdown_oplog_state.js
+++ b/jstests/replsets/clean_shutdown_oplog_state.js
@@ -4,6 +4,8 @@
 // WARNING: this test does not always fail deterministically. It is possible for the bug to be
 // present without this test failing. In particular if the rst.stop(1) doesn't execute mid-batch,
 // it isn't actually testing this bug. However, if the test fails there is definitely a bug.
+//
+// @tags: [requires_persistence]
 (function() {
     "use strict";
 
@@ -56,11 +58,16 @@
     assert.neq(null, conn, "secondary failed to start");
 
     // Following a clean shutdown of a 3.2 node, the oplog must exactly match the applied
-    // operations.
+    // operations. Additionally, the begin field must not be in the minValid document and the ts
+    // must match the top of the oplog (SERVER-25353).
     var oplogDoc = conn.getCollection('local.oplog.rs').find().sort({$natural: -1}).limit(1)[0];
     var collDoc = conn.getCollection('test.coll').find().sort({_id: -1}).limit(1)[0];
-    printjson({oplogDoc: oplogDoc, collDoc: collDoc});
+    var minValidDoc =
+        conn.getCollection('local.replset.minvalid').find().sort({$natural: -1}).limit(1)[0];
+    printjson({oplogDoc: oplogDoc, collDoc: collDoc, minValidDoc: minValidDoc});
     assert.eq(collDoc._id, oplogDoc.o._id);
+    assert(!('begin' in minValidDoc), 'begin in minValidDoc');
+    assert.eq(minValidDoc.ts, oplogDoc.ts);
 
     rst.stopSet();
 })();
diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp
index 632c3498d56..50517c21bf2 100644
--- a/src/mongo/db/repl/sync_tail.cpp
+++ b/src/mongo/db/repl/sync_tail.cpp
@@ -54,7 +54,6 @@
 #include "mongo/db/operation_context_impl.h"
 #include "mongo/db/prefetch.h"
 #include "mongo/db/repl/bgsync.h"
-#include "mongo/db/repl/minvalid.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/repl/oplogreader.h"
 #include "mongo/db/repl/repl_client_info.h"
@@ -529,7 +528,9 @@ void fillWriterVectors(OperationContext* txn,
 
 // Applies a batch of oplog entries, by using a set of threads to apply the operations and then
 // writes the oplog entries to the local oplog.
-OpTime SyncTail::multiApply(OperationContext* txn, const OpQueue& ops) {
+OpTime SyncTail::multiApply(OperationContext* txn,
+                            const OpQueue& ops,
+                            boost::optional<BatchBoundaries> boundaries) {
     invariant(_applyFunc);
 
     if (getGlobalServiceContext()->getGlobalStorageEngine()->isMmapV1()) {
@@ -560,6 +561,10 @@ OpTime SyncTail::multiApply(OperationContext* txn, const OpQueue& ops) {
         fassertFailed(28527);
     }
 
+    if (boundaries) {
+        setMinValid(txn, *boundaries);  // Mark us as in the middle of a batch.
+    }
+
     applyOps(writerVectors, &_writerPool, _applyFunc, this);
 
     OpTime lastOpTime;
@@ -575,6 +580,11 @@ OpTime SyncTail::multiApply(OperationContext* txn, const OpQueue& ops) {
 
     // Due to SERVER-24933 we can't enter inShutdown while holding the PBWM lock.
     invariant(!inShutdownStrict());
+
+    if (boundaries) {
+        setMinValid(txn, boundaries->end, DurableRequirement::None);  // Mark batch as complete.
+    }
+
     return lastOpTime;
 }
 
@@ -789,7 +799,7 @@ void SyncTail::oplogApplication() {
         // (last) failed batch, whichever is larger.
         // This will cause this node to go into RECOVERING state
         // if we should crash and restart before updating finishing.
-        const OpTime start(getLastSetTimestamp(), OpTime::kUninitializedTerm);
+        minValidBoundaries.start = OpTime(getLastSetTimestamp(), OpTime::kUninitializedTerm);
 
 
         // Take the max of the first endOptime (if we recovered) and the end of our batch.
@@ -805,25 +815,22 @@ void SyncTail::oplogApplication() {
         // restart
         // batch apply, 20-25, end = max(25, 40) = 40
         // batch apply, 25-45, end = 45
-        const OpTime end(std::max(originalEndOpTime, lastOpTime));
+        minValidBoundaries.end = std::max(originalEndOpTime, lastOpTime);
 
-        // This write will not journal/checkpoint.
-        setMinValid(&txn, {start, end});
 
-        lastWriteOpTime = multiApply(&txn, ops);
+        lastWriteOpTime = multiApply(&txn, ops, minValidBoundaries);
         if (lastWriteOpTime.isNull()) {
             // fassert if oplog application failed for any reasons other than shutdown.
             error() << "Failed to apply " << ops.getDeque().size()
-                    << " operations - batch start:" << start << " end:" << end;
+                    << " operations - batch start:" << minValidBoundaries.start
+                    << " end:" << minValidBoundaries.end;
             fassert(34360, inShutdownStrict());
             // Return without setting minvalid in the case of shutdown.
             return;
         }
 
         setNewTimestamp(lastWriteOpTime.getTimestamp());
-        setMinValid(&txn, end, DurableRequirement::None);
         minValidBoundaries.start = {};
-        minValidBoundaries.end = end;
         finalizer->record(lastWriteOpTime);
     }
 }
diff --git a/src/mongo/db/repl/sync_tail.h b/src/mongo/db/repl/sync_tail.h
index 8f331cc8cf6..c6c62b66276 100644
--- a/src/mongo/db/repl/sync_tail.h
+++ b/src/mongo/db/repl/sync_tail.h
@@ -28,10 +28,12 @@
 
 #pragma once
 
+#include <boost/optional.hpp>
 #include <deque>
 
 #include "mongo/base/status.h"
 #include "mongo/bson/bsonobj.h"
+#include "mongo/db/repl/minvalid.h"
 #include "mongo/db/storage/mmap_v1/dur.h"
 #include "mongo/stdx/functional.h"
 #include "mongo/util/concurrency/old_thread_pool.h"
@@ -174,8 +176,11 @@ protected:
     static const unsigned int replBatchLimitOperations = 5000;
 
     // Apply a batch of operations, using multiple threads.
+    // If boundries is supplied, will update minValid document at begin and end of batch.
     // Returns the last OpTime applied during the apply batch, ops.end["ts"] basically.
-    OpTime multiApply(OperationContext* txn, const OpQueue& ops);
+    OpTime multiApply(OperationContext* txn,
+                      const OpQueue& ops,
+                      boost::optional<BatchBoundaries> boundaries = {});
 
 private:
     class OpQueueBatcher;