SERVER-42055 Only acquire a collection IX lock to write the lastVote document

9 files changed, 81 insertions, 25 deletions

diff --git a/src/mongo/base/error_codes.err b/src/mongo/base/error_codes.err
index f8557db4479..07a3f7aecf8 100644
--- a/src/mongo/base/error_codes.err
+++ b/src/mongo/base/error_codes.err
@@ -206,6 +206,7 @@ error_code("KeyNotFound", 211)
 error_code("UpdateOperationFailed", 218)
 error_code("FTDCPathNotSet", 219)
 error_code("FTDCPathAlreadySet", 220)
+error_code("CannotCreateCollection", 242)
 error_code("ProducerConsumerQueueBatchTooLarge", 247)
 error_code("ProducerConsumerQueueEndClosed", 248)
 
diff --git a/src/mongo/db/repl/replication_coordinator_external_state.h b/src/mongo/db/repl/replication_coordinator_external_state.h
index dadaaff2d2d..8ccc0b5b88c 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state.h
@@ -195,6 +195,12 @@ public:
      */
     virtual Status storeLocalConfigDocument(OperationContext* txn, const BSONObj& config) = 0;
 
+
+    /**
+     * Creates the collection for "lastVote" documents and initializes it, or returns an error.
+     */
+    virtual Status createLocalLastVoteCollection(OperationContext* txn) = 0;
+
     /**
      * Gets the replica set lastVote document from local storage, or returns an error.
      */
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
index 597c9b47799..71891b244b8 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp
@@ -46,6 +46,7 @@
 #include "mongo/db/commands/feature_compatibility_version.h"
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/concurrency/write_conflict_exception.h"
+#include "mongo/db/db_raii.h"
 #include "mongo/db/dbdirectclient.h"
 #include "mongo/db/dbhelpers.h"
 #include "mongo/db/jsobj.h"
@@ -569,6 +570,38 @@ Status ReplicationCoordinatorExternalStateImpl::storeLocalConfigDocument(Operati
     }
 }
 
+Status ReplicationCoordinatorExternalStateImpl::createLocalLastVoteCollection(
+    OperationContext* txn) {
+    auto status = _storageInterface->createCollection(
+        txn, NamespaceString(lastVoteCollectionName), CollectionOptions());
+    if (!status.isOK() && status.code() != ErrorCodes::NamespaceExists) {
+        return Status(ErrorCodes::CannotCreateCollection,
+                      str::stream() << "Failed to create local last vote collection. Ns: "
+                                    << lastVoteCollectionName
+                                    << " Error: "
+                                    << status.toString());
+    }
+
+    // Make sure there's always a last vote document.
+    try {
+        MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN {
+            AutoGetCollection coll(txn, NamespaceString(lastVoteCollectionName), MODE_X);
+            BSONObj result;
+            bool exists = Helpers::getSingleton(txn, lastVoteCollectionName, result);
+            if (!exists) {
+                LastVote lastVote{OpTime::kInitialTerm, -1};
+                Helpers::putSingleton(txn, lastVoteCollectionName, lastVote.toBSON());
+            }
+        }
+        MONGO_WRITE_CONFLICT_RETRY_LOOP_END(
+            txn, "create initial replica set lastVote", lastVoteCollectionName);
+    } catch (const DBException& ex) {
+        return ex.toStatus();
+    }
+
+    return Status::OK();
+}
+
 StatusWith<LastVote> ReplicationCoordinatorExternalStateImpl::loadLocalLastVoteDocument(
     OperationContext* txn) {
     try {
@@ -595,30 +628,29 @@ Status ReplicationCoordinatorExternalStateImpl::storeLocalLastVoteDocument(
     try {
         MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN {
             ScopedTransaction transaction(txn, MODE_IX);
-            Lock::DBLock dbWriteLock(txn->lockState(), lastVoteDatabaseName, MODE_X);
+            AutoGetCollection coll(txn, NamespaceString(lastVoteCollectionName), MODE_IX);
+            WriteUnitOfWork wunit(txn);
 
-            // If there is no last vote document, we want to store one. Otherwise, we only want to
-            // replace it if the new last vote document would have a higher term. We both check
-            // the term of the current last vote document and insert the new document under the
-            // DBLock to synchronize the two operations.
+            // We only want to replace the last vote document if the new last vote document
+            // would have a higher term. We check the term of the current last vote document and
+            // insert the new document in a WriteUnitOfWork to synchronize the two operations.
+            // We have already ensured at startup time that there is an old document.
             BSONObj result;
             bool exists = Helpers::getSingleton(txn, lastVoteCollectionName, result);
-            if (!exists) {
+            fassert(51241, exists);
+            StatusWith<LastVote> oldLastVoteDoc = LastVote::readFromLastVote(result);
+            if (!oldLastVoteDoc.isOK()) {
+                return oldLastVoteDoc.getStatus();
+            }
+            if (lastVote.getTerm() > oldLastVoteDoc.getValue().getTerm()) {
                 Helpers::putSingleton(txn, lastVoteCollectionName, lastVoteObj);
-            } else {
-                StatusWith<LastVote> oldLastVoteDoc = LastVote::readFromLastVote(result);
-                if (!oldLastVoteDoc.isOK()) {
-                    return oldLastVoteDoc.getStatus();
-                }
-                if (lastVote.getTerm() > oldLastVoteDoc.getValue().getTerm()) {
-                    Helpers::putSingleton(txn, lastVoteCollectionName, lastVoteObj);
-                }
             }
+            wunit.commit();
+            return Status::OK();
         }
         MONGO_WRITE_CONFLICT_RETRY_LOOP_END(
             txn, "save replica set lastVote", lastVoteCollectionName);
         txn->recoveryUnit()->waitUntilDurable();
-        return Status::OK();
     } catch (const DBException& ex) {
         return ex.toStatus();
     }
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.h b/src/mongo/db/repl/replication_coordinator_external_state_impl.h
index ff2fa102982..b8dd2ad5b99 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_impl.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.h
@@ -81,6 +81,7 @@ public:
     OpTime onTransitionToPrimary(OperationContext* txn, bool isV1ElectionProtocol) override;
     virtual void forwardSlaveProgress();
     virtual OID ensureMe(OperationContext* txn);
+    virtual Status createLocalLastVoteCollection(OperationContext* txn);
     virtual bool isSelf(const HostAndPort& host, ServiceContext* ctx);
     virtual StatusWith<BSONObj> loadLocalConfigDocument(OperationContext* txn);
     virtual Status storeLocalConfigDocument(OperationContext* txn, const BSONObj& config);
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp b/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
index cef211451a6..83ea03456bc 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
+++ b/src/mongo/db/repl/replication_coordinator_external_state_mock.cpp
@@ -152,6 +152,14 @@ void ReplicationCoordinatorExternalStateMock::setLocalConfigDocument(
     _localRsConfigDocument = localConfigDocument;
 }
 
+Status ReplicationCoordinatorExternalStateMock::createLocalLastVoteCollection(
+    OperationContext* txn) {
+    if (!_localRsLastVoteDocument.isOK()) {
+        setLocalLastVoteDocument(LastVote{OpTime::kInitialTerm, -1});
+    }
+    return Status::OK();
+}
+
 StatusWith<LastVote> ReplicationCoordinatorExternalStateMock::loadLocalLastVoteDocument(
     OperationContext* txn) {
     return _localRsLastVoteDocument;
diff --git a/src/mongo/db/repl/replication_coordinator_external_state_mock.h b/src/mongo/db/repl/replication_coordinator_external_state_mock.h
index 7120433bd73..efcf7d221d7 100644
--- a/src/mongo/db/repl/replication_coordinator_external_state_mock.h
+++ b/src/mongo/db/repl/replication_coordinator_external_state_mock.h
@@ -124,6 +124,11 @@ public:
     void setLocalConfigDocument(const StatusWith<BSONObj>& localConfigDocument);
 
     /**
+     * Initializes the return value for subsequent calls to loadLocalLastVoteDocument().
+     */
+    virtual Status createLocalLastVoteCollection(OperationContext* txn);
+
+    /**
      * Sets the return value for subsequent calls to loadLocalLastVoteDocument().
      */
     void setLocalLastVoteDocument(const StatusWith<LastVote>& localLastVoteDocument);
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 99c4f9b48ae..f532032a309 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -436,16 +436,17 @@ void ReplicationCoordinatorImpl::appendConnectionStats(executor::ConnectionPoolS
 }
 
 bool ReplicationCoordinatorImpl::_startLoadLocalConfig(OperationContext* txn) {
+    fassert(51240, _externalState->createLocalLastVoteCollection(txn));
     StatusWith<LastVote> lastVote = _externalState->loadLocalLastVoteDocument(txn);
     if (!lastVote.isOK()) {
-        if (lastVote.getStatus() == ErrorCodes::NoMatchingDocument) {
-            log() << "Did not find local voted for document at startup.";
-        } else {
-            severe() << "Error loading local voted for document at startup; "
-                     << lastVote.getStatus();
-            fassertFailedNoTrace(40367);
-        }
-    } else {
+        severe() << "Error loading local voted for document at startup; " << lastVote.getStatus();
+        fassertFailedNoTrace(40367);
+    }
+    if (lastVote.getValue().getTerm() == OpTime::kInitialTerm) {
+        // This log line is checked in unit tests.
+        log() << "Did not find local initialized voted for document at startup.";
+    }
+    {
         LockGuard topoLock(_topoMutex);
         _topCoord->loadLastVote(lastVote.getValue());
     }
diff --git a/src/mongo/dbtests/SConscript b/src/mongo/dbtests/SConscript
index 64fff21bc79..df68ecd3edd 100644
--- a/src/mongo/dbtests/SConscript
+++ b/src/mongo/dbtests/SConscript
@@ -120,6 +120,7 @@ dbtest = env.Program(
         "$BUILD_DIR/mongo/db/query/query_test_service_context",
         "$BUILD_DIR/mongo/db/repl/repl_coordinator_global",
         "$BUILD_DIR/mongo/db/repl/replmocks",
+        "$BUILD_DIR/mongo/db/repl/storage_interface_impl",
         "$BUILD_DIR/mongo/db/serveronly",
         "$BUILD_DIR/mongo/db/storage/paths",
         "$BUILD_DIR/mongo/util/concurrency/rwlock",
diff --git a/src/mongo/dbtests/replica_set_tests.cpp b/src/mongo/dbtests/replica_set_tests.cpp
index f6cb5b11e21..427e160ca53 100644
--- a/src/mongo/dbtests/replica_set_tests.cpp
+++ b/src/mongo/dbtests/replica_set_tests.cpp
@@ -32,7 +32,7 @@
 #include "mongo/db/dbdirectclient.h"
 #include "mongo/db/repl/last_vote.h"
 #include "mongo/db/repl/replication_coordinator_external_state_impl.h"
-#include "mongo/db/repl/storage_interface_mock.h"
+#include "mongo/db/repl/storage_interface_impl.h"
 #include "mongo/db/service_context.h"
 #include "mongo/unittest/unittest.h"
 
@@ -47,9 +47,10 @@ class ReplicaSetTest : public mongo::unittest::Test {
 protected:
     void setUp() {
         auto txn = makeOpCtx();
-        _storageInterface = stdx::make_unique<repl::StorageInterfaceMock>();
+        _storageInterface = stdx::make_unique<repl::StorageInterfaceImpl>();
         _replCoordExternalState.reset(
             new repl::ReplicationCoordinatorExternalStateImpl(_storageInterface.get()));
+        ASSERT_OK(_replCoordExternalState->createLocalLastVoteCollection(txn.get()));
     }
 
     void tearDown() {