SERVER-33383 Prevent internal replSetStepDown from racing with stepdown via heartbeat

4 files changed, 27 insertions, 2 deletions

diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 899fd3ea5fe..b2a5947e34e 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -1619,6 +1619,8 @@ Status ReplicationCoordinatorImpl::stepDown(OperationContext* opCtx,
     status = _topCoord->prepareForStepDownAttempt();
     if (!status.isOK()) {
         // This will cause us to fail if we're already in the process of stepping down.
+        // It is also possible to get here even if we're done stepping down via another path,
+        // and this will also elicit a failure from this call.
         return status;
     }
 
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index 686f7008dd3..749d007d7eb 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -1910,6 +1910,11 @@ Status TopologyCoordinator::prepareForStepDownAttempt() {
         return Status{ErrorCodes::ConflictingOperationInProgress,
                       "This node is already in the process of stepping down"};
     }
+
+    if (_leaderMode == LeaderMode::kNotLeader) {
+        return Status{ErrorCodes::NotMaster, "This node is not a primary."};
+    }
+
     _setLeaderMode(LeaderMode::kAttemptingStepDown);
     return Status::OK();
 }
diff --git a/src/mongo/db/repl/topology_coordinator.h b/src/mongo/db/repl/topology_coordinator.h
index 3cb30571e86..b23f0967922 100644
--- a/src/mongo/db/repl/topology_coordinator.h
+++ b/src/mongo/db/repl/topology_coordinator.h
@@ -604,8 +604,9 @@ public:
      * Readies the TopologyCoordinator for an attempt to stepdown that may fail.  This is used
      * when we receive a stepdown command (which can fail if not enough secondaries are caught up)
      * to ensure that we never process more than one stepdown request at a time.
-     * Returns OK if it is safe to continue with the stepdown attempt, or returns
-     * ConflictingOperationInProgess if this node is already processing a stepdown request of any
+     * Returns OK if it is safe to continue with the stepdown attempt, or returns:
+     * - NotMaster if this node is not a leader.
+     * - ConflictingOperationInProgess if this node is already processing a stepdown request of any
      * kind.
      */
     Status prepareForStepDownAttempt();
diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
index e403b021400..010910cac91 100644
--- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp
+++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
@@ -1767,6 +1767,23 @@ TEST_F(TopoCoordTest, HeartbeatFrequencyShouldBeHalfElectionTimeoutWhenArbiter)
     ASSERT_EQUALS(expected, action.getNextHeartbeatStartDate());
 }
 
+TEST_F(TopoCoordTest, PrepareStepDownAttemptFailsIfNotLeader) {
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version"
+                      << 5
+                      << "members"
+                      << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                               << "host1:27017"))
+                      << "protocolVersion"
+                      << 1),
+                 0);
+    getTopoCoord().changeMemberState_forTest(MemberState::RS_SECONDARY);
+    Status expectedStatus(ErrorCodes::NotMaster, "This node is not a primary. ");
+
+    ASSERT_EQUALS(expectedStatus, getTopoCoord().prepareForStepDownAttempt());
+}
+
 class PrepareHeartbeatResponseV1Test : public TopoCoordTest {
 public:
     virtual void setUp() {