SERVER-31662 Initialize rollback id to 1 and log when it changes

(cherry picked from commit 48c4e4eb48e7994a78bee8f3384df2963a1ea407)

8 files changed, 93 insertions, 37 deletions

diff --git a/src/mongo/db/repl/replication_process.cpp b/src/mongo/db/repl/replication_process.cpp
index 6b7ea3ef36e..efeb1c08faf 100644
--- a/src/mongo/db/repl/replication_process.cpp
+++ b/src/mongo/db/repl/replication_process.cpp
@@ -26,6 +26,8 @@
  *    it in the license file.
  */
 
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kReplication
+
 #include "mongo/platform/basic.h"
 
 #include "mongo/base/string_data.h"
@@ -39,6 +41,7 @@
 #include "mongo/db/repl/storage_interface.h"
 #include "mongo/db/service_context.h"
 #include "mongo/util/assert_util.h"
+#include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
 
 namespace mongo {
@@ -119,7 +122,13 @@ Status ReplicationProcess::initializeRollbackID(OperationContext* opCtx) {
     // Leave _rbid uninitialized until the next getRollbackID() to retrieve the actual value
     // from storage.
 
-    return _storageInterface->initializeRollbackID(opCtx);
+    auto initRbidSW = _storageInterface->initializeRollbackID(opCtx);
+    if (initRbidSW.isOK()) {
+        log() << "Initialized the rollback ID to " << initRbidSW.getValue();
+    } else {
+        warning() << "Failed to initialize the rollback ID: " << initRbidSW.getStatus().reason();
+    }
+    return initRbidSW.getStatus();
 }
 
 Status ReplicationProcess::incrementRollbackID(OperationContext* opCtx) {
@@ -131,9 +140,12 @@ Status ReplicationProcess::incrementRollbackID(OperationContext* opCtx) {
     // storage next time getRollbackID() is called.
     if (status.isOK()) {
         _rbid = kUninitializedRollbackId;
+        log() << "Incremented the rollback ID to " << status.getValue();
+    } else {
+        warning() << "Failed to increment the rollback ID: " << status.getStatus().reason();
     }
 
-    return status;
+    return status.getStatus();
 }
 
 StatusWith<OpTime> ReplicationProcess::getRollbackProgress(OperationContext* opCtx) {
diff --git a/src/mongo/db/repl/replication_process_test.cpp b/src/mongo/db/repl/replication_process_test.cpp
index 0203ba5b527..ed7cdf26c80 100644
--- a/src/mongo/db/repl/replication_process_test.cpp
+++ b/src/mongo/db/repl/replication_process_test.cpp
@@ -246,4 +246,21 @@ TEST_F(ReplicationProcessTest,
                   replicationProcess.clearRollbackProgress(opCtx.get()));
 }
 
+TEST_F(ReplicationProcessTest, RollbackIDIncrementsBy1) {
+    auto opCtx = makeOpCtx();
+    ReplicationProcess replicationProcess(
+        _storageInterface.get(),
+        stdx::make_unique<ReplicationConsistencyMarkersImpl>(_storageInterface.get()),
+        stdx::make_unique<ReplicationRecoveryMock>());
+
+    // 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()));
+
+    // Make sure the rollback ID is incremented by exactly 1.
+    ASSERT_OK(replicationProcess.incrementRollbackID(opCtx.get()));
+    int rbid = unittest::assertGet(replicationProcess.getRollbackID(opCtx.get()));
+    ASSERT_EQ(rbid, initRBID + 1);
+}
+
 }  // namespace
diff --git a/src/mongo/db/repl/storage_interface.h b/src/mongo/db/repl/storage_interface.h
index d6c50c28b26..a7882041c21 100644
--- a/src/mongo/db/repl/storage_interface.h
+++ b/src/mongo/db/repl/storage_interface.h
@@ -84,11 +84,26 @@ public:
     virtual ~StorageInterface() = default;
 
     /**
-     * Rollback ID is an increasing counter of how many rollbacks have occurred on this server.
+     * Rollback ID is an increasing counter of how many rollbacks have occurred on this server. It
+     * is initialized with a value of 1, and should increase by exactly 1 every time a rollback
+     * occurs.
+     */
+
+    /**
+     * Return the current value of the rollback ID.
      */
     virtual StatusWith<int> getRollbackID(OperationContext* opCtx) = 0;
-    virtual Status initializeRollbackID(OperationContext* opCtx) = 0;
-    virtual Status incrementRollbackID(OperationContext* opCtx) = 0;
+
+    /**
+     * Initialize the rollback ID to 1. Returns the value of the initialized rollback ID if
+     * successful.
+     */
+    virtual StatusWith<int> initializeRollbackID(OperationContext* opCtx) = 0;
+
+    /**
+     * Increments the current rollback ID. Returns the new value of the rollback ID if successful.
+     */
+    virtual StatusWith<int> incrementRollbackID(OperationContext* opCtx) = 0;
 
 
     // Collection creation and population for initial sync.
diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp
index 05f542ab3a8..2173d59640f 100644
--- a/src/mongo/db/repl/storage_interface_impl.cpp
+++ b/src/mongo/db/repl/storage_interface_impl.cpp
@@ -115,41 +115,50 @@ StatusWith<int> StorageInterfaceImpl::getRollbackID(OperationContext* opCtx) {
     MONGO_UNREACHABLE;
 }
 
-Status StorageInterfaceImpl::initializeRollbackID(OperationContext* opCtx) {
+StatusWith<int> StorageInterfaceImpl::initializeRollbackID(OperationContext* opCtx) {
     auto status = createCollection(opCtx, _rollbackIdNss, CollectionOptions());
     if (!status.isOK()) {
         return status;
     }
 
     RollbackID rbid;
+    int initRBID = 1;
     rbid.set_id(kRollbackIdDocumentId);
-    rbid.setRollbackId(0);
+    rbid.setRollbackId(initRBID);
 
     BSONObjBuilder bob;
     rbid.serialize(&bob);
     Timestamp noTimestamp;  // This write is not replicated.
-    return insertDocument(opCtx,
-                          _rollbackIdNss,
-                          TimestampedBSONObj{bob.done(), noTimestamp},
-                          OpTime::kUninitializedTerm);
+    status = insertDocument(opCtx,
+                            _rollbackIdNss,
+                            TimestampedBSONObj{bob.done(), noTimestamp},
+                            OpTime::kUninitializedTerm);
+    if (status.isOK()) {
+        return initRBID;
+    } else {
+        return status;
+    }
 }
 
-Status StorageInterfaceImpl::incrementRollbackID(OperationContext* opCtx) {
+StatusWith<int> StorageInterfaceImpl::incrementRollbackID(OperationContext* opCtx) {
     // This is safe because this is only called during rollback, and you can not have two
     // rollbacks at once.
-    auto rbid = getRollbackID(opCtx);
-    if (!rbid.isOK()) {
-        return rbid.getStatus();
+    auto rbidSW = getRollbackID(opCtx);
+    if (!rbidSW.isOK()) {
+        return rbidSW;
     }
 
-    // If we would go over the integer limit, reset the Rollback ID to 0.
+    // If we would go over the integer limit, reset the Rollback ID to 1.
     BSONObjBuilder updateBob;
-    if (rbid.getValue() == std::numeric_limits<int>::max()) {
+    int newRBID = -1;
+    if (rbidSW.getValue() == std::numeric_limits<int>::max()) {
+        newRBID = 1;
         BSONObjBuilder setBob(updateBob.subobjStart("$set"));
-        setBob.append(kRollbackIdFieldName, 0);
+        setBob.append(kRollbackIdFieldName, newRBID);
     } else {
         BSONObjBuilder incBob(updateBob.subobjStart("$inc"));
         incBob.append(kRollbackIdFieldName, 1);
+        newRBID = rbidSW.getValue() + 1;
     }
 
     // Since the Rollback ID is in a singleton collection, we can fix the _id field.
@@ -161,6 +170,7 @@ Status StorageInterfaceImpl::incrementRollbackID(OperationContext* opCtx) {
     // We wait until durable so that we are sure the Rollback ID is updated before rollback ends.
     if (status.isOK()) {
         opCtx->recoveryUnit()->waitUntilDurable();
+        return newRBID;
     }
     return status;
 }
diff --git a/src/mongo/db/repl/storage_interface_impl.h b/src/mongo/db/repl/storage_interface_impl.h
index 404b42bb3b2..710f50d9e10 100644
--- a/src/mongo/db/repl/storage_interface_impl.h
+++ b/src/mongo/db/repl/storage_interface_impl.h
@@ -53,8 +53,8 @@ public:
     StorageInterfaceImpl();
 
     StatusWith<int> getRollbackID(OperationContext* opCtx) override;
-    Status initializeRollbackID(OperationContext* opCtx) override;
-    Status incrementRollbackID(OperationContext* opCtx) override;
+    StatusWith<int> initializeRollbackID(OperationContext* opCtx) override;
+    StatusWith<int> incrementRollbackID(OperationContext* opCtx) override;
 
     /**
      *  Allocates a new TaskRunner for use by the passed in collection.
diff --git a/src/mongo/db/repl/storage_interface_impl_test.cpp b/src/mongo/db/repl/storage_interface_impl_test.cpp
index 9b3e9fada8b..343ee8b5720 100644
--- a/src/mongo/db/repl/storage_interface_impl_test.cpp
+++ b/src/mongo/db/repl/storage_interface_impl_test.cpp
@@ -280,26 +280,28 @@ TEST_F(StorageInterfaceImplTest, RollbackIdInitializesIncrementsAndReadsProperly
     StorageInterfaceImpl storage;
     auto opCtx = getOperationContext();
 
+    // Rollback ID should be initialized to 1.
     ASSERT_OK(storage.initializeRollbackID(opCtx));
-    _assertRollbackIDDocument(opCtx, 0);
+    _assertRollbackIDDocument(opCtx, 1);
 
     auto rbid = unittest::assertGet(storage.getRollbackID(opCtx));
-    ASSERT_EQUALS(rbid, 0);
+    ASSERT_EQUALS(rbid, 1);
 
+    // Rollback ID should increment by exactly 1 each time.
     ASSERT_OK(storage.incrementRollbackID(opCtx));
-    _assertRollbackIDDocument(opCtx, 1);
+    _assertRollbackIDDocument(opCtx, 2);
 
     rbid = unittest::assertGet(storage.getRollbackID(opCtx));
-    ASSERT_EQUALS(rbid, 1);
+    ASSERT_EQUALS(rbid, 2);
 
     ASSERT_OK(storage.incrementRollbackID(opCtx));
-    _assertRollbackIDDocument(opCtx, 2);
+    _assertRollbackIDDocument(opCtx, 3);
 
     rbid = unittest::assertGet(storage.getRollbackID(opCtx));
-    ASSERT_EQUALS(rbid, 2);
+    ASSERT_EQUALS(rbid, 3);
 }
 
-TEST_F(StorageInterfaceImplTest, IncrementRollbackIDRollsToZeroWhenExceedingMaxInt) {
+TEST_F(StorageInterfaceImplTest, IncrementRollbackIDRollsToOneWhenExceedingMaxInt) {
     StorageInterfaceImpl storage;
     auto opCtx = getOperationContext();
     NamespaceString nss(StorageInterfaceImpl::kDefaultRollbackIdNamespace);
@@ -315,16 +317,16 @@ TEST_F(StorageInterfaceImplTest, IncrementRollbackIDRollsToZeroWhenExceedingMaxI
     ASSERT_EQUALS(rbid, std::numeric_limits<int>::max());
 
     ASSERT_OK(storage.incrementRollbackID(opCtx));
-    _assertRollbackIDDocument(opCtx, 0);
+    _assertRollbackIDDocument(opCtx, 1);
 
     rbid = unittest::assertGet(storage.getRollbackID(opCtx));
-    ASSERT_EQUALS(rbid, 0);
+    ASSERT_EQUALS(rbid, 1);
 
     ASSERT_OK(storage.incrementRollbackID(opCtx));
-    _assertRollbackIDDocument(opCtx, 1);
+    _assertRollbackIDDocument(opCtx, 2);
 
     rbid = unittest::assertGet(storage.getRollbackID(opCtx));
-    ASSERT_EQUALS(rbid, 1);
+    ASSERT_EQUALS(rbid, 2);
 }
 
 TEST_F(StorageInterfaceImplTest, GetRollbackIDReturnsBadStatusIfDocumentHasBadField) {
diff --git a/src/mongo/db/repl/storage_interface_mock.cpp b/src/mongo/db/repl/storage_interface_mock.cpp
index 01add91bd00..2282c7ca9ac 100644
--- a/src/mongo/db/repl/storage_interface_mock.cpp
+++ b/src/mongo/db/repl/storage_interface_mock.cpp
@@ -47,7 +47,7 @@ StatusWith<int> StorageInterfaceMock::getRollbackID(OperationContext* opCtx) {
     return _rbid;
 }
 
-Status StorageInterfaceMock::initializeRollbackID(OperationContext* opCtx) {
+StatusWith<int> StorageInterfaceMock::initializeRollbackID(OperationContext* opCtx) {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
     if (_rbidInitialized) {
         return Status(ErrorCodes::NamespaceExists, "Rollback ID already initialized");
@@ -56,16 +56,16 @@ Status StorageInterfaceMock::initializeRollbackID(OperationContext* opCtx) {
 
     // Start the mock RBID at a very high number to differentiate it from uninitialized RBIDs.
     _rbid = 100;
-    return Status::OK();
+    return _rbid;
 }
 
-Status StorageInterfaceMock::incrementRollbackID(OperationContext* opCtx) {
+StatusWith<int> StorageInterfaceMock::incrementRollbackID(OperationContext* opCtx) {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
     if (!_rbidInitialized) {
         return Status(ErrorCodes::NamespaceNotFound, "Rollback ID not initialized");
     }
     _rbid++;
-    return Status::OK();
+    return _rbid;
 }
 
 void StorageInterfaceMock::setStableTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) {
diff --git a/src/mongo/db/repl/storage_interface_mock.h b/src/mongo/db/repl/storage_interface_mock.h
index c67f339a691..2f6fea75cdd 100644
--- a/src/mongo/db/repl/storage_interface_mock.h
+++ b/src/mongo/db/repl/storage_interface_mock.h
@@ -130,8 +130,8 @@ public:
     StorageInterfaceMock() = default;
 
     StatusWith<int> getRollbackID(OperationContext* opCtx) override;
-    Status initializeRollbackID(OperationContext* opCtx) override;
-    Status incrementRollbackID(OperationContext* opCtx) override;
+    StatusWith<int> initializeRollbackID(OperationContext* opCtx) override;
+    StatusWith<int> incrementRollbackID(OperationContext* opCtx) override;
 
     StatusWith<std::unique_ptr<CollectionBulkLoader>> createCollectionForBulkLoading(
         const NamespaceString& nss,