SERVER-36503 Skip dry run during election handoff

11 files changed, 50 insertions, 18 deletions

diff --git a/src/mongo/db/repl/repl_set_commands.cpp b/src/mongo/db/repl/repl_set_commands.cpp
index b01d30f8d4e..f73915f87e1 100644
--- a/src/mongo/db/repl/repl_set_commands.cpp
+++ b/src/mongo/db/repl/repl_set_commands.cpp
@@ -692,7 +692,8 @@ public:
 
         log() << "Received replSetStepUp request";
 
-        status = ReplicationCoordinator::get(opCtx)->stepUpIfEligible();
+        const bool skipDryRun = cmdObj["skipDryRun"].trueValue();
+        status = ReplicationCoordinator::get(opCtx)->stepUpIfEligible(skipDryRun);
 
         if (!status.isOK()) {
             log() << "replSetStepUp request failed" << causedBy(status);
diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h
index 2b75d7485fb..6cdde3041ab 100644
--- a/src/mongo/db/repl/replication_coordinator.h
+++ b/src/mongo/db/repl/replication_coordinator.h
@@ -766,7 +766,7 @@ public:
     virtual ReplSettings::IndexPrefetchConfig getIndexPrefetchConfig() const = 0;
     virtual void setIndexPrefetchConfig(const ReplSettings::IndexPrefetchConfig cfg) = 0;
 
-    virtual Status stepUpIfEligible() = 0;
+    virtual Status stepUpIfEligible(bool skipDryRun) = 0;
 
     virtual ServiceContext* getServiceContext() = 0;
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 3f7c6ae85b0..c7463d168f6 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -1827,7 +1827,8 @@ void ReplicationCoordinatorImpl::_performElectionHandoff() {
     }
 
     auto target = _rsConfig.getMemberAt(candidateIndex).getHostAndPort();
-    executor::RemoteCommandRequest request(target, "admin", BSON("replSetStepUp" << 1), nullptr);
+    executor::RemoteCommandRequest request(
+        target, "admin", BSON("replSetStepUp" << 1 << "skipDryRun" << true), nullptr);
     log() << "Handing off election to " << target;
 
     auto callbackHandleSW = _replExecutor->scheduleRemoteCommand(
@@ -3496,8 +3497,12 @@ CallbackFn ReplicationCoordinatorImpl::_wrapAsCallbackFn(const stdx::function<vo
     };
 }
 
-Status ReplicationCoordinatorImpl::stepUpIfEligible() {
-    _startElectSelfIfEligibleV1(TopologyCoordinator::StartElectionReason::kStepUpRequest);
+Status ReplicationCoordinatorImpl::stepUpIfEligible(bool skipDryRun) {
+
+    auto reason = skipDryRun ? TopologyCoordinator::StartElectionReason::kStepUpRequestSkipDryRun
+                             : TopologyCoordinator::StartElectionReason::kStepUpRequest;
+    _startElectSelfIfEligibleV1(reason);
+
     EventHandle finishEvent;
     {
         stdx::lock_guard<stdx::mutex> lk(_mutex);
diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h
index 5cb5f3c130a..ec828550170 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.h
+++ b/src/mongo/db/repl/replication_coordinator_impl.h
@@ -292,7 +292,7 @@ public:
     virtual ReplSettings::IndexPrefetchConfig getIndexPrefetchConfig() const override;
     virtual void setIndexPrefetchConfig(const ReplSettings::IndexPrefetchConfig cfg) override;
 
-    virtual Status stepUpIfEligible() override;
+    virtual Status stepUpIfEligible(bool skipDryRun) override;
 
     virtual Status abortCatchupIfNeeded() override;
 
@@ -793,7 +793,8 @@ private:
      *
      * For V1 (raft) style elections the election path is:
      *      _startElectSelfV1() or _startElectSelfV1_inlock()
-     *      _onDryRunComplete()
+     *      _processDryRunResult() (may skip)
+     *      _startRealElection_inlock()
      *      _writeLastVoteForMyElection()
      *      _startVoteRequester_inlock()
      *      _onVoteRequestComplete()
@@ -807,7 +808,13 @@ private:
      * "originalTerm" was the term during which the dry run began, if the term has since
      * changed, do not run for election.
      */
-    void _onDryRunComplete(long long originalTerm);
+    void _processDryRunResult(long long originalTerm);
+
+    /**
+     * Begins executing a real election. This is called either a successful dry run, or when the
+     * dry run was skipped (which may be specified for a ReplSetStepUp).
+     */
+    void _startRealElection_inlock(long long originalTerm);
 
     /**
      * Writes the last vote in persistent storage after completing dry run successfully.
diff --git a/src/mongo/db/repl/replication_coordinator_impl_elect_v1.cpp b/src/mongo/db/repl/replication_coordinator_impl_elect_v1.cpp
index f9d030458f2..85325168375 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_elect_v1.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_elect_v1.cpp
@@ -138,6 +138,14 @@ void ReplicationCoordinatorImpl::_startElectSelfV1_inlock(
     long long term = _topCoord->getTerm();
     int primaryIndex = -1;
 
+    if (reason == TopologyCoordinator::StartElectionReason::kStepUpRequestSkipDryRun) {
+        long long newTerm = term + 1;
+        log() << "skipping dry run and running for election in term " << newTerm;
+        _startRealElection_inlock(newTerm);
+        lossGuard.dismiss();
+        return;
+    }
+
     log() << "conducting a dry run election to see if we could be elected. current term: " << term;
     _voteRequester.reset(new VoteRequester);
 
@@ -159,12 +167,12 @@ void ReplicationCoordinatorImpl::_startElectSelfV1_inlock(
     fassert(28685, nextPhaseEvh.getStatus());
     _replExecutor
         ->onEvent(nextPhaseEvh.getValue(),
-                  [=](const executor::TaskExecutor::CallbackArgs&) { _onDryRunComplete(term); })
+                  [=](const executor::TaskExecutor::CallbackArgs&) { _processDryRunResult(term); })
         .status_with_transitional_ignore();
     lossGuard.dismiss();
 }
 
-void ReplicationCoordinatorImpl::_onDryRunComplete(long long originalTerm) {
+void ReplicationCoordinatorImpl::_processDryRunResult(long long originalTerm) {
     stdx::lock_guard<stdx::mutex> lk(_mutex);
     LoseElectionDryRunGuardV1 lossGuard(this);
 
@@ -194,9 +202,20 @@ void ReplicationCoordinatorImpl::_onDryRunComplete(long long originalTerm) {
 
     long long newTerm = originalTerm + 1;
     log() << "dry election run succeeded, running for election in term " << newTerm;
-    // Stepdown is impossible from this term update.
+
+    _startRealElection_inlock(newTerm);
+    lossGuard.dismiss();
+}
+
+void ReplicationCoordinatorImpl::_startRealElection_inlock(long long newTerm) {
+    LoseElectionDryRunGuardV1 lossGuard(this);
+
     TopologyCoordinator::UpdateTermResult updateTermResult;
     _updateTerm_inlock(newTerm, &updateTermResult);
+    // This is the only valid result from this term update. If we are here, then we are not a
+    // primary, so a stepdown is not possible. We have also not yet learned of a higher term from
+    // someone else: seeing an update in the topology coordinator mid-election requires releasing
+    // the mutex. This only happens during a dry run, which makes sure to check for term updates.
     invariant(updateTermResult == TopologyCoordinator::UpdateTermResult::kUpdatedTerm);
     // Secure our vote for ourself first
     _topCoord->voteForMyselfV1();
@@ -230,7 +249,6 @@ void ReplicationCoordinatorImpl::_writeLastVoteForMyElection(
     }();
 
     stdx::lock_guard<stdx::mutex> lk(_mutex);
-    invariant(_voteRequester);
     LoseElectionDryRunGuardV1 lossGuard(this);
     if (status == ErrorCodes::CallbackCanceled) {
         return;
@@ -254,8 +272,6 @@ void ReplicationCoordinatorImpl::_writeLastVoteForMyElection(
 }
 
 void ReplicationCoordinatorImpl::_startVoteRequester_inlock(long long newTerm) {
-    invariant(_voteRequester);
-
     const auto lastOpTime = _getMyLastAppliedOpTime_inlock();
 
     _voteRequester.reset(new VoteRequester);
diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
index 59f982a129c..4bde0bc9956 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
@@ -827,6 +827,7 @@ void ReplicationCoordinatorImpl::_startElectSelfIfEligibleV1(
                                     << "since we are not electable due to: " << status.reason();
                 break;
             case TopologyCoordinator::StartElectionReason::kStepUpRequest:
+            case TopologyCoordinator::StartElectionReason::kStepUpRequestSkipDryRun:
                 LOG_FOR_ELECTION(0) << "Not starting an election for a replSetStepUp request, "
                                     << "since we are not electable due to: " << status.reason();
                 break;
@@ -852,6 +853,7 @@ void ReplicationCoordinatorImpl::_startElectSelfIfEligibleV1(
             LOG_FOR_ELECTION(0) << "Starting an election for a priority takeover";
             break;
         case TopologyCoordinator::StartElectionReason::kStepUpRequest:
+        case TopologyCoordinator::StartElectionReason::kStepUpRequestSkipDryRun:
             LOG_FOR_ELECTION(0) << "Starting an election due to step up request";
             break;
         case TopologyCoordinator::StartElectionReason::kCatchupTakeover:
diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp
index 6e4f2e339af..4d2212ca320 100644
--- a/src/mongo/db/repl/replication_coordinator_mock.cpp
+++ b/src/mongo/db/repl/replication_coordinator_mock.cpp
@@ -455,7 +455,7 @@ ReplSettings::IndexPrefetchConfig ReplicationCoordinatorMock::getIndexPrefetchCo
 void ReplicationCoordinatorMock::setIndexPrefetchConfig(
     const ReplSettings::IndexPrefetchConfig cfg) {}
 
-Status ReplicationCoordinatorMock::stepUpIfEligible() {
+Status ReplicationCoordinatorMock::stepUpIfEligible(bool skipDryRun) {
     return Status::OK();
 }
 
diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h
index a784b5093e1..760727eb5a5 100644
--- a/src/mongo/db/repl/replication_coordinator_mock.h
+++ b/src/mongo/db/repl/replication_coordinator_mock.h
@@ -247,7 +247,7 @@ public:
     virtual ReplSettings::IndexPrefetchConfig getIndexPrefetchConfig() const override;
     virtual void setIndexPrefetchConfig(const ReplSettings::IndexPrefetchConfig cfg) override;
 
-    virtual Status stepUpIfEligible() override;
+    virtual Status stepUpIfEligible(bool skipDryRun) override;
 
     /**
      * Sets the return value for calls to getConfig.
diff --git a/src/mongo/db/repl/topology_coordinator.h b/src/mongo/db/repl/topology_coordinator.h
index bfd272e3f5c..3d23cce1332 100644
--- a/src/mongo/db/repl/topology_coordinator.h
+++ b/src/mongo/db/repl/topology_coordinator.h
@@ -624,6 +624,7 @@ public:
         kElectionTimeout,
         kPriorityTakeover,
         kStepUpRequest,
+        kStepUpRequestSkipDryRun,
         kCatchupTakeover,
         kSingleNodePromptElection
     };
diff --git a/src/mongo/embedded/replication_coordinator_embedded.cpp b/src/mongo/embedded/replication_coordinator_embedded.cpp
index 381a34574db..f32d93819a2 100644
--- a/src/mongo/embedded/replication_coordinator_embedded.cpp
+++ b/src/mongo/embedded/replication_coordinator_embedded.cpp
@@ -398,7 +398,7 @@ void ReplicationCoordinatorEmbedded::dropAllSnapshots() {
     UASSERT_NOT_IMPLEMENTED;
 }
 
-Status ReplicationCoordinatorEmbedded::stepUpIfEligible() {
+Status ReplicationCoordinatorEmbedded::stepUpIfEligible(bool skipDryRun) {
     UASSERT_NOT_IMPLEMENTED;
 }
 
diff --git a/src/mongo/embedded/replication_coordinator_embedded.h b/src/mongo/embedded/replication_coordinator_embedded.h
index 24eda7e109f..ce53bf91b0f 100644
--- a/src/mongo/embedded/replication_coordinator_embedded.h
+++ b/src/mongo/embedded/replication_coordinator_embedded.h
@@ -219,7 +219,7 @@ public:
     repl::ReplSettings::IndexPrefetchConfig getIndexPrefetchConfig() const override;
     void setIndexPrefetchConfig(const repl::ReplSettings::IndexPrefetchConfig) override;
 
-    Status stepUpIfEligible() override;
+    Status stepUpIfEligible(bool skipDryRun) override;
 
     Status abortCatchupIfNeeded() override;