SERVER-46667 Avoid invariant from invalid candidateIndex

2 files changed, 62 insertions, 21 deletions

diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 0f873be6cd5..23ea924189d 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -4747,28 +4747,17 @@ Status ReplicationCoordinatorImpl::processReplSetRequestVotes(
             return Status(ErrorCodes::ShutdownInProgress, "In the process of shutting down");
         }
 
-        _topCoord->processReplSetRequestVotes(args, response);
-    }
-
-    if (!args.isADryRun()) {
         const int candidateIndex = args.getCandidateIndex();
-        LastVote lastVote{args.getTerm(), candidateIndex};
-
-        const bool votedForCandidate = response->getVoteGranted();
-
-        if (votedForCandidate) {
-            Status status = _externalState->storeLocalLastVoteDocument(opCtx, lastVote);
-            if (!status.isOK()) {
-                LOGV2_ERROR(21428,
-                            "replSetRequestVotes failed to store LastVote document; {error}",
-                            "replSetRequestVotes failed to store LastVote document",
-                            "error"_attr = status);
-                return status;
-            }
+        if (candidateIndex < 0 || candidateIndex >= _rsConfig.getNumMembers()) {
+            return Status(ErrorCodes::BadValue,
+                          str::stream() << "Invalid candidateIndex: " << candidateIndex
+                                        << ". Must be between 0 and "
+                                        << _rsConfig.getNumMembers() - 1 << " inclusive");
         }
 
-        // If the vote was not granted to the candidate, we still want to track metrics around the
-        // node's participation in the election.
+        _topCoord->processReplSetRequestVotes(args, response);
+
+        const bool votedForCandidate = response->getVoteGranted();
         const long long electionTerm = args.getTerm();
         const Date_t lastVoteDate = _replExecutor->now();
         const int electionCandidateMemberId =
@@ -4777,7 +4766,6 @@ Status ReplicationCoordinatorImpl::processReplSetRequestVotes(
         const OpTime lastAppliedOpTime = _topCoord->getMyLastAppliedOpTime();
         const OpTime maxAppliedOpTime = _topCoord->latestKnownOpTime();
         const double priorityAtElection = _rsConfig.getMemberAt(_selfIndex).getPriority();
-
         ReplicationMetrics::get(getServiceContext())
             .setElectionParticipantMetrics(votedForCandidate,
                                            electionTerm,
@@ -4788,6 +4776,21 @@ Status ReplicationCoordinatorImpl::processReplSetRequestVotes(
                                            maxAppliedOpTime,
                                            priorityAtElection);
     }
+
+    // It's safe to store lastVote outside of _mutex. The topology coordinator grants only one
+    // vote per term, and storeLocalLastVoteDocument does nothing unless lastVote has a higher term
+    // than the previous lastVote, so threads racing to store votes from different terms will
+    // eventually store the latest vote.
+    if (!args.isADryRun() && response->getVoteGranted()) {
+        LastVote lastVote{args.getTerm(), args.getCandidateIndex()};
+        Status status = _externalState->storeLocalLastVoteDocument(opCtx, lastVote);
+        if (!status.isOK()) {
+            LOGV2_ERROR(21428,
+                        "replSetRequestVotes failed to store LastVote document",
+                        "error"_attr = status);
+            return status;
+        }
+    }
     return Status::OK();
 }
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
index 983d639ad96..c9492616562 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp
@@ -7125,7 +7125,7 @@ TEST_F(ReplCoordTest, NodeFailsVoteRequestIfItFailsToStoreLastVote) {
     ASSERT_EQUALS(lastVote.getCandidateIndex(), 0);
 }
 
-TEST_F(ReplCoordTest, NodeNodesNotGrantVoteIfInTerminalShutdown) {
+TEST_F(ReplCoordTest, NodeDoesNotGrantVoteIfInTerminalShutdown) {
     // Set up a 2-node replica set config.
     assertStartSuccess(BSON("_id"
                             << "mySet"
@@ -7407,6 +7407,44 @@ TEST_F(ReplCoordTest, CheckIfCommitQuorumHasReached) {
     }
 }
 
+TEST_F(ReplCoordTest, NodeFailsVoteRequestIfCandidateIndexIsInvalid) {
+    // Set up a 2-node replica set config.
+    assertStartSuccess(BSON("_id"
+                            << "mySet"
+                            << "version" << 2 << "members"
+                            << BSON_ARRAY(BSON("host"
+                                               << "node1:12345"
+                                               << "_id" << 0)
+                                          << BSON("host"
+                                                  << "node2:12345"
+                                                  << "_id" << 1))),
+                       HostAndPort("node1", 12345));
+    auto time = OpTimeWithTermOne(100, 1);
+    ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
+    replCoordSetMyLastAppliedOpTime(time, Date_t() + Seconds(100));
+    replCoordSetMyLastDurableOpTime(time, Date_t() + Seconds(100));
+    simulateSuccessfulV1Election();
+
+    auto opCtx = makeOperationContext();
+
+    // Invalid candidateIndex values.
+    for (auto candidateIndex : std::vector<long long>{-1LL, 2LL}) {
+        ReplSetRequestVotesArgs args;
+        ASSERT_OK(args.initialize(BSON(
+            "replSetRequestVotes" << 1 << "setName"
+                                  << "mySet"
+                                  << "term" << getReplCoord()->getTerm() << "candidateIndex"
+                                  << candidateIndex << "configVersion" << 2LL << "dryRun" << false
+                                  << "lastAppliedOpTime" << time.asOpTime().toBSON())));
+        ReplSetRequestVotesResponse response;
+        auto r = getReplCoord()->processReplSetRequestVotes(opCtx.get(), args, &response);
+
+        ASSERT_NOT_OK(r);
+        ASSERT_STRING_CONTAINS(r.reason(), "Invalid candidateIndex");
+        ASSERT_EQUALS(ErrorCodes::BadValue, r.code());
+    }
+}
+
 // TODO(schwerin): Unit test election id updating
 }  // namespace
 }  // namespace repl