SERVER-79885: Oplog fetching getMore should only set null lastKnownCommittedOpTime for exhaust cursorsrelease-7.0.1

(cherry picked from commit eeac78cd8de74ca1cffb18eb4b798b8392df6192)

3 files changed, 49 insertions, 10 deletions

diff --git a/src/mongo/client/dbclient_cursor.h b/src/mongo/client/dbclient_cursor.h
index 1f64da56766..e4a25e673d0 100644
--- a/src/mongo/client/dbclient_cursor.h
+++ b/src/mongo/client/dbclient_cursor.h
@@ -173,6 +173,10 @@ public:
         return tailable() && _findRequest->getAwaitData();
     }
 
+    bool isExhaust() const {
+        return _isExhaust;
+    }
+
     /**
      * Changes the cursor's batchSize after construction. Can change after requesting first batch.
      */
diff --git a/src/mongo/db/repl/oplog_fetcher.cpp b/src/mongo/db/repl/oplog_fetcher.cpp
index d4687970b8d..eb6d92abecc 100644
--- a/src/mongo/db/repl/oplog_fetcher.cpp
+++ b/src/mongo/db/repl/oplog_fetcher.cpp
@@ -712,8 +712,21 @@ StatusWith<OplogFetcher::Documents> OplogFetcher::_getNextBatch() {
             auto lastCommittedWithCurrentTerm =
                 _dataReplicatorExternalState->getCurrentTermAndLastCommittedOpTime();
             if (lastCommittedWithCurrentTerm.value != OpTime::kUninitializedTerm) {
-                _cursor->setCurrentTermAndLastCommittedOpTime(lastCommittedWithCurrentTerm.value,
-                                                              lastCommittedWithCurrentTerm.opTime);
+                if (!_cursor->isExhaust() && lastCommittedWithCurrentTerm.opTime.isNull()) {
+                    // For non-exhaust cursors, only set the lastKnownCommittedOpTime when it is not
+                    // a null opTime. This is to avoid sending null opTime again and again and
+                    // triggering oplog empty batches every single time in case we can't advance our
+                    // commit point (e.g. during initial sync).
+                    _cursor->setCurrentTermAndLastCommittedOpTime(
+                        lastCommittedWithCurrentTerm.value, boost::none);
+                } else {
+                    // For exhaust cursors, it is safe to set a null lastKnownCommittedOpTime in the
+                    // initial getMore because the sync source will update the exhaust cursor's
+                    // lastKnownCommittedOpTime to the commit point sent in the last response after
+                    // each oplog batch.
+                    _cursor->setCurrentTermAndLastCommittedOpTime(
+                        lastCommittedWithCurrentTerm.value, lastCommittedWithCurrentTerm.opTime);
+                }
             }
             _cursor->more();
         }
diff --git a/src/mongo/db/repl/oplog_fetcher_test.cpp b/src/mongo/db/repl/oplog_fetcher_test.cpp
index 66779d16be2..e79ecf039f2 100644
--- a/src/mongo/db/repl/oplog_fetcher_test.cpp
+++ b/src/mongo/db/repl/oplog_fetcher_test.cpp
@@ -200,16 +200,21 @@ void validateGetMoreCommand(Message m,
     ASSERT_EQ(cursorId, msg.body.getIntField("getMore"));
     ASSERT_EQUALS(timeout, msg.body.getIntField("maxTimeMS"));
 
-    // In unittests, lastCommittedWithCurrentTerm should always be default to valid and non-null.
+    // In unittests, lastCommittedWithCurrentTerm.value should always be a valid term.
     // The case when currentTerm is kUninitializedTerm is tested separately in
     // GetMoreQueryDoesNotContainTermIfGetCurrentTermAndLastCommittedOpTimeReturnsUninitializedTerm.
     invariant(lastCommittedWithCurrentTerm.value != OpTime::kUninitializedTerm);
-    invariant(!lastCommittedWithCurrentTerm.opTime.isNull());
     ASSERT_EQUALS(lastCommittedWithCurrentTerm.value, msg.body["term"].numberLong());
-    ASSERT_EQUALS(lastCommittedWithCurrentTerm.opTime.getTimestamp(),
-                  msg.body["lastKnownCommittedOpTime"]["ts"].timestamp());
-    ASSERT_EQUALS(lastCommittedWithCurrentTerm.opTime.getTerm(),
-                  msg.body["lastKnownCommittedOpTime"]["t"].numberLong());
+    if (!exhaustSupported && lastCommittedWithCurrentTerm.opTime.isNull()) {
+        // Test that we don't attach the lastKnownCommittedOpTime field for non-exhaust cursors when
+        // the lastCommittedOpTime is null.
+        ASSERT_FALSE(msg.body.hasField("lastKnownCommittedOpTime"));
+    } else {
+        ASSERT_EQUALS(lastCommittedWithCurrentTerm.opTime.getTimestamp(),
+                      msg.body["lastKnownCommittedOpTime"]["ts"].timestamp());
+        ASSERT_EQUALS(lastCommittedWithCurrentTerm.opTime.getTerm(),
+                      msg.body["lastKnownCommittedOpTime"]["t"].numberLong());
+    }
 
     if (exhaustSupported) {
         ASSERT_TRUE(OpMsg::isFlagSet(m, OpMsg::kExhaustSupported));
@@ -1597,6 +1602,14 @@ TEST_F(OplogFetcherTest, OplogFetcherWorksWithoutExhaust) {
     // Update lastFetched before it is updated by getting the next batch.
     lastFetched = oplogFetcher->getLastOpTimeFetched_forTest();
 
+    // Set a null lastCommittedOpTime to test that non-exhaust cursors don't attach a null
+    // lastKnownCommittedOpTime. This must be done before we issue the response to the find request
+    // so that the first getMore request (made immediately after processSingleRequestResponse) can
+    // pick this up.
+    dataReplicatorExternalState->lastCommittedOpTime = OpTime();
+    auto firstGetMoreTermAndLastCommittedOpTime =
+        dataReplicatorExternalState->getCurrentTermAndLastCommittedOpTime();
+
     // Creating the cursor will succeed. After this, the cursor will be blocked on call() for the
     // getMore command.
     auto m = processSingleRequestResponse(oplogFetcher->getDBClientConnection_forTest(),
@@ -1616,6 +1629,15 @@ TEST_F(OplogFetcherTest, OplogFetcherWorksWithoutExhaust) {
     auto fourthEntry = makeNoopOplogEntry({{Seconds(458), 0}, lastFetched.getTerm()});
     auto secondBatch = {thirdEntry, fourthEntry};
 
+
+    // Reset the lastCommittedOpTime to non-null. This must be done before we issue the response to
+    // the first getMore request so that the second getMore request (made immediately after
+    // processSingleRequestResponse) can pick this up.
+    dataReplicatorExternalState->lastCommittedOpTime = {{9999, 0},
+                                                        dataReplicatorExternalState->currentTerm};
+    auto secondGetMoreTermAndLastCommittedOpTime =
+        dataReplicatorExternalState->getCurrentTermAndLastCommittedOpTime();
+
     // moreToCome would be set to false if oplogFetcherUsesExhaust was set to false. After this,
     // the cursor will be blocked on call() for the next getMore command.
     m = processSingleRequestResponse(
@@ -1626,7 +1648,7 @@ TEST_F(OplogFetcherTest, OplogFetcherWorksWithoutExhaust) {
     validateGetMoreCommand(m,
                            cursorId,
                            durationCount<Milliseconds>(oplogFetcher->getAwaitDataTimeout_forTest()),
-                           dataReplicatorExternalState->getCurrentTermAndLastCommittedOpTime(),
+                           firstGetMoreTermAndLastCommittedOpTime,
                            false /* exhaustSupported */);
 
     // Update lastFetched since it should have been updated after getting the last batch.
@@ -1646,7 +1668,7 @@ TEST_F(OplogFetcherTest, OplogFetcherWorksWithoutExhaust) {
     validateGetMoreCommand(m,
                            cursorId,
                            durationCount<Milliseconds>(oplogFetcher->getAwaitDataTimeout_forTest()),
-                           dataReplicatorExternalState->getCurrentTermAndLastCommittedOpTime(),
+                           secondGetMoreTermAndLastCommittedOpTime,
                            false /* exhaustSupported */);
 
     // Update lastFetched since it should have been updated after getting the last batch.