SERVER-32776 ReplicationProcess::getRollbackID() no longer reads from storage to refresh cached rollback id

8 files changed, 21 insertions, 40 deletions

diff --git a/src/mongo/db/repl/repl_set_commands.cpp b/src/mongo/db/repl/repl_set_commands.cpp
index f436cbf881c..1aa71ccfca7 100644
--- a/src/mongo/db/repl/repl_set_commands.cpp
+++ b/src/mongo/db/repl/repl_set_commands.cpp
@@ -171,12 +171,7 @@ public:
         if (!status.isOK())
             return CommandHelpers::appendCommandStatus(result, status);
 
-        auto rbid = ReplicationProcess::get(opCtx)->getRollbackID(opCtx);
-
-        // We should always have a Rollback ID since it is created at startup.
-        fassertStatusOK(40426, rbid.getStatus());
-
-        result.append("rbid", rbid.getValue());
+        result.append("rbid", ReplicationProcess::get(opCtx)->getRollbackID());
         return CommandHelpers::appendCommandStatus(result, Status::OK());
     }
 } cmdReplSetRBID;
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index b13a6af2181..7c512259830 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -3194,7 +3194,7 @@ void ReplicationCoordinatorImpl::prepareReplMetadata(OperationContext* opCtx,
     // Avoid retrieving Rollback ID if we do not need it for _prepareOplogQueryMetadata_inlock().
     int rbid = -1;
     if (hasOplogQueryMetadata) {
-        rbid = fassertStatusOK(40427, _replicationProcess->getRollbackID(opCtx));
+        rbid = _replicationProcess->getRollbackID();
         invariant(-1 != rbid);
     }
 
diff --git a/src/mongo/db/repl/replication_info.cpp b/src/mongo/db/repl/replication_info.cpp
index 72c73baa863..076c7e04a7c 100644
--- a/src/mongo/db/repl/replication_info.cpp
+++ b/src/mongo/db/repl/replication_info.cpp
@@ -172,9 +172,9 @@ public:
         BSONObjBuilder result;
         appendReplicationInfo(opCtx, result, level);
 
-        auto rbid = ReplicationProcess::get(opCtx)->getRollbackID(opCtx);
-        if (rbid.isOK()) {
-            result.append("rbid", rbid.getValue());
+        auto rbid = ReplicationProcess::get(opCtx)->getRollbackID();
+        if (ReplicationProcess::kUninitializedRollbackId != rbid) {
+            result.append("rbid", rbid);
         }
 
         return result.obj();
diff --git a/src/mongo/db/repl/replication_process.cpp b/src/mongo/db/repl/replication_process.cpp
index 3e4b9cd7669..7f091a990a6 100644
--- a/src/mongo/db/repl/replication_process.cpp
+++ b/src/mongo/db/repl/replication_process.cpp
@@ -53,7 +53,6 @@ namespace {
 const auto getReplicationProcess =
     ServiceContext::declareDecoration<std::unique_ptr<ReplicationProcess>>();
 
-const auto kRollbackNamespacePrefix = "local.system.rollback."_sd;
 }  // namespace
 
 ReplicationProcess* ReplicationProcess::get(ServiceContext* service) {
@@ -101,23 +100,13 @@ Status ReplicationProcess::refreshRollbackID(OperationContext* opCtx) {
     return Status::OK();
 }
 
-StatusWith<int> ReplicationProcess::getRollbackID(OperationContext* opCtx) {
+int ReplicationProcess::getRollbackID() const {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
-
-    if (kUninitializedRollbackId != _rbid) {
-        return _rbid;
-    }
-
-    // The _rbid, which caches the rollback ID persisted in the local.system.rollback.id collection,
-    // may be uninitialized because this is the first time we are retrieving the rollback ID.
-    auto rbidResult = _storageInterface->getRollbackID(opCtx);
-    if (!rbidResult.isOK()) {
-        return rbidResult;
+    if (kUninitializedRollbackId == _rbid) {
+        // This may happen when serverStatus is called by an internal client before we have a chance
+        // to read the rollback ID from storage.
+        warning() << "Rollback ID is not initialized yet.";
     }
-    log() << "Rollback ID is " << rbidResult.getValue();
-    _rbid = rbidResult.getValue();
-
-    invariant(kUninitializedRollbackId != _rbid);
     return _rbid;
 }
 
diff --git a/src/mongo/db/repl/replication_process.h b/src/mongo/db/repl/replication_process.h
index ae155fe09d5..916b300d416 100644
--- a/src/mongo/db/repl/replication_process.h
+++ b/src/mongo/db/repl/replication_process.h
@@ -79,7 +79,7 @@ public:
      * Rollback ID is an increasing counter of how many rollbacks have occurred on this server.
      */
     Status refreshRollbackID(OperationContext* opCtx);
-    StatusWith<int> getRollbackID(OperationContext* opCtx);
+    int getRollbackID() const;
     Status initializeRollbackID(OperationContext* opCtx);
     Status incrementRollbackID(OperationContext* opCtx);
 
@@ -102,7 +102,7 @@ private:
     // (M)  Reads and writes guarded by _mutex.
 
     // Guards access to member variables.
-    stdx::mutex _mutex;
+    mutable stdx::mutex _mutex;
 
     // Used to access the storage layer.
     StorageInterface* const _storageInterface;  // (R)
diff --git a/src/mongo/db/repl/replication_process_test.cpp b/src/mongo/db/repl/replication_process_test.cpp
index e031cd7551f..edf9a6645f5 100644
--- a/src/mongo/db/repl/replication_process_test.cpp
+++ b/src/mongo/db/repl/replication_process_test.cpp
@@ -97,11 +97,11 @@ TEST_F(ReplicationProcessTest, RollbackIDIncrementsBy1) {
 
     // We make no assumptions about the initial value of the rollback ID.
     ASSERT_OK(replicationProcess.initializeRollbackID(opCtx.get()));
-    int initRBID = unittest::assertGet(replicationProcess.getRollbackID(opCtx.get()));
+    int initRBID = replicationProcess.getRollbackID();
 
     // Make sure the rollback ID is incremented by exactly 1.
     ASSERT_OK(replicationProcess.incrementRollbackID(opCtx.get()));
-    int rbid = unittest::assertGet(replicationProcess.getRollbackID(opCtx.get()));
+    int rbid = replicationProcess.getRollbackID();
     ASSERT_EQ(rbid, initRBID + 1);
 }
 
@@ -117,16 +117,16 @@ TEST_F(ReplicationProcessTest, RefreshRollbackIDResetsCachedValueFromStorage) {
 
     // We make no assumptions about the initial value of the rollback ID.
     ASSERT_OK(replicationProcess.initializeRollbackID(opCtx.get()));
-    int initRBID = unittest::assertGet(replicationProcess.getRollbackID(opCtx.get()));
+    int initRBID = replicationProcess.getRollbackID();
 
     // Increment rollback ID on disk. Cached value should different from storage.
     int storageRBID = unittest::assertGet(_storageInterface->incrementRollbackID(opCtx.get()));
     ASSERT_EQUALS(storageRBID, initRBID + 1);
-    ASSERT_EQUALS(initRBID, unittest::assertGet(replicationProcess.getRollbackID(opCtx.get())));
+    ASSERT_EQUALS(initRBID, replicationProcess.getRollbackID());
 
     // Refresh cached value and check cached value against storage again.
     ASSERT_OK(replicationProcess.refreshRollbackID(opCtx.get()));
-    ASSERT_EQUALS(storageRBID, unittest::assertGet(replicationProcess.getRollbackID(opCtx.get())));
+    ASSERT_EQUALS(storageRBID, replicationProcess.getRollbackID());
 }
 
 }  // namespace
diff --git a/src/mongo/db/repl/rollback_impl_test.cpp b/src/mongo/db/repl/rollback_impl_test.cpp
index 8ac13bb02eb..5b23f7278f3 100644
--- a/src/mongo/db/repl/rollback_impl_test.cpp
+++ b/src/mongo/db/repl/rollback_impl_test.cpp
@@ -266,13 +266,13 @@ TEST_F(RollbackImplTest, RollbackIncrementsRollbackID) {
     _localOplog->setOperations({op});
 
     // Get the initial rollback id.
-    int initRollbackId = unittest::assertGet(_replicationProcess->getRollbackID(_opCtx.get()));
+    int initRollbackId = _replicationProcess->getRollbackID();
 
     // Run rollback.
     ASSERT_OK(_rollback->runRollback(_opCtx.get()));
 
     // Check that the rollback id was incremented.
-    int newRollbackId = unittest::assertGet(_replicationProcess->getRollbackID(_opCtx.get()));
+    int newRollbackId = _replicationProcess->getRollbackID();
     ASSERT_EQUALS(initRollbackId + 1, newRollbackId);
 }
 
diff --git a/src/mongo/db/s/session_catalog_migration_source.cpp b/src/mongo/db/s/session_catalog_migration_source.cpp
index e99913c1eeb..c3b96f0302f 100644
--- a/src/mongo/db/s/session_catalog_migration_source.cpp
+++ b/src/mongo/db/s/session_catalog_migration_source.cpp
@@ -113,9 +113,7 @@ repl::OplogEntry makeSentinelOplogEntry(OperationSessionInfo sessionInfo) {
 
 SessionCatalogMigrationSource::SessionCatalogMigrationSource(OperationContext* opCtx,
                                                              NamespaceString ns)
-    : _ns(std::move(ns)),
-      _rollbackIdAtInit(
-          uassertStatusOK(repl::ReplicationProcess::get(opCtx)->getRollbackID(opCtx))) {
+    : _ns(std::move(ns)), _rollbackIdAtInit(repl::ReplicationProcess::get(opCtx)->getRollbackID()) {
     // Sort is not needed for correctness. This is just for making it easier to write deterministic
     // tests.
     Query query;
@@ -325,8 +323,7 @@ repl::OplogEntry SessionCatalogMigrationSource::SessionOplogIterator::getNext(
         if (excep.code() == ErrorCodes::IncompleteTransactionHistory) {
             // Note: no need to check if in replicaSet mode because having an iterator implies
             // oplog exists.
-            auto rollbackId =
-                uassertStatusOK(repl::ReplicationProcess::get(opCtx)->getRollbackID(opCtx));
+            auto rollbackId = repl::ReplicationProcess::get(opCtx)->getRollbackID();
 
             uassert(40656,
                     str::stream() << "rollback detected, rollbackId was " << _initialRollbackId