SERVER-46387 Only vote for candidate with same config version and term as self.r4.3.5

(cherry picked from commit e063494b7379d8445bd23016819dcccbb3bd1dd3)

3 files changed, 74 insertions, 39 deletions

diff --git a/jstests/replsets/catchup_takeover_one_high_priority.js b/jstests/replsets/catchup_takeover_one_high_priority.js
index 39beecd3dce..d3cd2efc8aa 100644
--- a/jstests/replsets/catchup_takeover_one_high_priority.js
+++ b/jstests/replsets/catchup_takeover_one_high_priority.js
@@ -37,6 +37,7 @@ replSet.waitForState(2, ReplSetTest.State.PRIMARY, replSet.kDefaultTimeoutMS);
 jsTestLog('node 2 is now primary');
 
 replSet.awaitReplication();
+waitForConfigReplication(nodes[2]);
 
 // Stop replication and disconnect node 2 so that it cannot do a priority takeover.
 stopServerReplication(nodes[2]);
@@ -69,6 +70,9 @@ assert.commandWorked(primary.getDB(name).bar.insert({x: 100}, writeConcern));
 nodes[2].reconnect(nodes[0]);
 nodes[2].reconnect(nodes[1]);
 
+// Wait until nodes have learned the latest config.
+waitForConfigReplication(nodes[1], [nodes[1], nodes[2]]);
+
 // Step up a lagged node.
 assert.commandWorked(nodes[1].adminCommand({replSetStepUp: 1}));
 replSet.awaitNodesAgreeOnPrimary(replSet.kDefaultTimeoutMS, nodes);
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index 9e70e6409a8..70fb749b2fe 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -2985,8 +2985,8 @@ void TopologyCoordinator::processReplSetRequestVotes(const ReplSetRequestVotesAr
     if (MONGO_unlikely(voteNoInElection.shouldFail())) {
         LOGV2(21835, "failpoint voteNoInElection enabled");
         response->setVoteGranted(false);
-        response->setReason(str::stream() << "forced to vote no during dry run election due to "
-                                             "failpoint voteNoInElection set");
+        response->setReason(
+            "forced to vote no during dry run election due to failpoint voteNoInElection set");
         return;
     }
 
@@ -2994,54 +2994,48 @@ void TopologyCoordinator::processReplSetRequestVotes(const ReplSetRequestVotesAr
         LOGV2(21836, "failpoint voteYesInDryRunButNoInRealElection enabled");
         if (args.isADryRun()) {
             response->setVoteGranted(true);
-            response->setReason(str::stream() << "forced to vote yes in dry run due to failpoint "
-                                                 "voteYesInDryRunButNoInRealElection set");
+            response->setReason(
+                "forced to vote yes in dry run due to failpoint "
+                "voteYesInDryRunButNoInRealElection set");
         } else {
             response->setVoteGranted(false);
-            response->setReason(str::stream()
-                                << "forced to vote no in real election due to failpoint "
-                                   "voteYesInDryRunButNoInRealElection set");
+            response->setReason(
+                "forced to vote no in real election due to failpoint "
+                "voteYesInDryRunButNoInRealElection set");
         }
         return;
     }
 
-    if (args.getTerm() < _term) {
+    if (args.getConfigVersionAndTerm() != _rsConfig.getConfigVersionAndTerm()) {
         response->setVoteGranted(false);
-        response->setReason(str::stream() << "candidate's term ({}) is lower than mine ({})"_format(
-                                args.getTerm(), _term));
-    } else if (args.getConfigVersionAndTerm() < _rsConfig.getConfigVersionAndTerm()) {
+        response->setReason("candidate's config with {} differs from mine with {}"_format(
+            args.getConfigVersionAndTerm(), _rsConfig.getConfigVersionAndTerm()));
+    } else if (args.getTerm() < _term) {
         response->setVoteGranted(false);
-        response->setReason(str::stream()
-                            << "candidate's config with {} is older "
-                               "than mine with {}"_format(args.getConfigVersionAndTerm(),
-                                                          _rsConfig.getConfigVersionAndTerm()));
+        response->setReason(
+            "candidate's term ({}) is lower than mine ({})"_format(args.getTerm(), _term));
     } else if (args.getSetName() != _rsConfig.getReplSetName()) {
         response->setVoteGranted(false);
-        response->setReason(str::stream()
-                            << "candidate's set name ({}) differs from mine ({})"_format(
-                                   args.getSetName(), _rsConfig.getReplSetName()));
+        response->setReason("candidate's set name ({}) differs from mine ({})"_format(
+            args.getSetName(), _rsConfig.getReplSetName()));
     } else if (args.getLastAppliedOpTime() < getMyLastAppliedOpTime()) {
         response->setVoteGranted(false);
-        response->setReason(str::stream()
-                            << "candidate's data is staler than mine. candidate's last applied "
-                               "OpTime: {}, my last applied OpTime: {}"_format(
-                                   args.getLastAppliedOpTime().toString(),
-                                   getMyLastAppliedOpTime().toString()));
+        response->setReason(
+            "candidate's data is staler than mine. candidate's last applied OpTime: {}, "
+            "my last applied OpTime: {}"_format(args.getLastAppliedOpTime().toString(),
+                                                getMyLastAppliedOpTime().toString()));
     } else if (!args.isADryRun() && _lastVote.getTerm() == args.getTerm()) {
         response->setVoteGranted(false);
-        response->setReason(
-            str::stream() << "already voted for another candidate ({}) this "
-                             "term ({})"_format(_rsConfig.getMemberAt(_lastVote.getCandidateIndex())
-                                                    .getHostAndPort(),
-                                                _lastVote.getTerm()));
+        response->setReason("already voted for another candidate ({}) this term ({})"_format(
+            _rsConfig.getMemberAt(_lastVote.getCandidateIndex()).getHostAndPort(),
+            _lastVote.getTerm()));
     } else {
         int betterPrimary = _findHealthyPrimaryOfEqualOrGreaterPriority(args.getCandidateIndex());
         if (_selfConfig().isArbiter() && betterPrimary >= 0) {
             response->setVoteGranted(false);
-            response
-                ->setReason(str::stream()
-                            << "can see a healthy primary ({}) of equal or greater priority"_format(
-                                   _rsConfig.getMemberAt(betterPrimary).getHostAndPort()));
+            response->setReason(
+                "can see a healthy primary ({}) of equal or greater priority"_format(
+                    _rsConfig.getMemberAt(betterPrimary).getHostAndPort()));
         } else {
             if (!args.isADryRun()) {
                 _lastVote.setTerm(args.getTerm());
diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
index 5e14967e8c6..5817fb0fdaa 100644
--- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp
+++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
@@ -2917,6 +2917,39 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantVoteWhenReplSetNameDoesNotMatch) {
     ASSERT_FALSE(response.getVoteGranted());
 }
 
+TEST_F(TopoCoordTest, RemovedNodeDoesNotGrantVote) {
+    // Removed node sets its own index to -1.
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version" << 1 << "members"
+                      << BSON_ARRAY(BSON("_id" << 10 << "host"
+                                               << "h1")
+                                    << BSON("_id" << 20 << "host"
+                                                  << "h2")
+                                    << BSON("_id" << 30 << "host"
+                                                  << "h3"))),
+                 -1);
+    ASSERT_EQ(getTopoCoord().getMemberState(), MemberState::RS_REMOVED);
+
+    // A normal vote request.
+    ReplSetRequestVotesArgs args;
+    ASSERT_OK(
+        args.initialize(BSON("replSetRequestVotes"
+                             << 1 << "setName"
+                             << "rs0"
+                             << "dryRun" << true << "term" << 1LL << "candidateIndex" << 2LL
+                             << "configVersion" << 2LL << "configTerm" << 1LL << "lastAppliedOpTime"
+                             << BSON("ts" << Timestamp(10, 0) << "t" << 0LL))));
+    ReplSetRequestVotesResponse response;
+
+    getTopoCoord().processReplSetRequestVotes(args, &response);
+    ASSERT_FALSE(response.getVoteGranted());
+    ASSERT_EQUALS(
+        "candidate's config with {version: 2, term: 1} differs from mine with {version: 1, term: "
+        "-1}",
+        response.getReason());
+}
+
 class ConfigTermAndVersionVoteTest : public TopoCoordTest {
 public:
     auto testWithArbiter(bool useArbiter,
@@ -2960,7 +2993,7 @@ public:
 TEST_F(ConfigTermAndVersionVoteTest, DataNodeDoesNotGrantVoteWhenConfigVersionIsLower) {
     auto response = testWithArbiter(false, 1, 2);
     ASSERT_EQUALS(
-        "candidate's config with {version: 1, term: 2} is older than mine with"
+        "candidate's config with {version: 1, term: 2} differs from mine with"
         " {version: 2, term: 2}",
         response.getReason());
 }
@@ -2968,7 +3001,7 @@ TEST_F(ConfigTermAndVersionVoteTest, DataNodeDoesNotGrantVoteWhenConfigVersionIs
 TEST_F(ConfigTermAndVersionVoteTest, ArbiterDoesNotGrantVoteWhenConfigVersionIsLower) {
     auto response = testWithArbiter(true, 1, 2);
     ASSERT_EQUALS(
-        "candidate's config with {version: 1, term: 2} is older than mine with"
+        "candidate's config with {version: 1, term: 2} differs from mine with"
         " {version: 2, term: 2}",
         response.getReason());
 }
@@ -2976,7 +3009,7 @@ TEST_F(ConfigTermAndVersionVoteTest, ArbiterDoesNotGrantVoteWhenConfigVersionIsL
 TEST_F(ConfigTermAndVersionVoteTest, DataNodeDoesNotGrantVoteWhenConfigTermIsLower) {
     auto response = testWithArbiter(false, 2, 1);
     ASSERT_EQUALS(
-        "candidate's config with {version: 2, term: 1} is older than mine with"
+        "candidate's config with {version: 2, term: 1} differs from mine with"
         " {version: 2, term: 2}",
         response.getReason());
 }
@@ -2984,7 +3017,7 @@ TEST_F(ConfigTermAndVersionVoteTest, DataNodeDoesNotGrantVoteWhenConfigTermIsLow
 TEST_F(ConfigTermAndVersionVoteTest, ArbiterDoesNotGrantVoteWhenConfigTermIsLower) {
     auto response = testWithArbiter(true, 2, 1);
     ASSERT_EQUALS(
-        "candidate's config with {version: 2, term: 1} is older than mine with"
+        "candidate's config with {version: 2, term: 1} differs from mine with"
         " {version: 2, term: 2}",
         response.getReason());
 }
@@ -3152,7 +3185,7 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantDryRunVoteWhenConfigVersionIsLower) {
 
     getTopoCoord().processReplSetRequestVotes(args, &response);
     ASSERT_EQUALS(
-        "candidate's config with {version: 0, term: 1} is older than mine with {version: 1, term: "
+        "candidate's config with {version: 0, term: 1} differs from mine with {version: 1, term: "
         "1}",
         response.getReason());
     ASSERT_EQUALS(1, response.getTerm());
@@ -3207,7 +3240,7 @@ TEST_F(TopoCoordTest, NodeDoesNotGrantDryRunVoteWhenTermIsStale) {
     ASSERT_FALSE(response.getVoteGranted());
 }
 
-TEST_F(TopoCoordTest, NodeGrantsVoteWhenTermIsHigherButConfigVersionIsLower) {
+TEST_F(TopoCoordTest, NodeDoesNotGrantVoteWhenTermIsHigherButConfigVersionIsLower) {
     updateConfig(BSON("_id"
                       << "rs0"
                       << "version" << 2 << "term" << 1LL << "members"
@@ -3238,7 +3271,11 @@ TEST_F(TopoCoordTest, NodeGrantsVoteWhenTermIsHigherButConfigVersionIsLower) {
     getTopoCoord().processReplSetRequestVotes(args, &response);
     // Candidates config(t, v) is (2, 1) and our config is (1, 2). Even though the candidate's
     // config version is lower, we grant our vote because the candidate's config term is higher.
-    ASSERT_TRUE(response.getVoteGranted());
+    ASSERT_FALSE(response.getVoteGranted());
+    ASSERT_EQ(
+        "candidate's config with {version: 1, term: 2} differs from mine with {version: 2, term: "
+        "1}",
+        response.getReason());
 }
 
 TEST_F(TopoCoordTest, GrantDryRunVoteEvenWhenTermHasBeenSeen) {