SERVER-55465 Response from all nodes means sufficient responses have been received even if primary gave bad response in catchup takeover dry run

2 files changed, 34 insertions, 2 deletions

diff --git a/src/mongo/db/repl/vote_requester.cpp b/src/mongo/db/repl/vote_requester.cpp
index 9399acd396f..6102b6deb0b 100644
--- a/src/mongo/db/repl/vote_requester.cpp
+++ b/src/mongo/db/repl/vote_requester.cpp
@@ -153,12 +153,18 @@ bool VoteRequester::Algorithm::hasReceivedSufficientResponses() const {
         return true;
     }
 
+    // We require the primary's response during catchup takeover. An error response from the primary
+    // is not a yes, no or pending, but is still a response. Therefore, we handle the case in which
+    // we have received some response (even if error) from all nodes first.
+    if (_responsesProcessed == static_cast<int>(_targets.size())) {
+        return true;
+    }
+
     if (_primaryHost && _primaryVote == PrimaryVote::Pending) {
         return false;
     }
 
-    return _staleTerm || _votes >= _rsConfig.getMajorityVoteCount() ||
-        _responsesProcessed == static_cast<int>(_targets.size());
+    return _staleTerm || _votes >= _rsConfig.getMajorityVoteCount();
 }
 
 VoteRequester::Result VoteRequester::Algorithm::getResult() const {
diff --git a/src/mongo/db/repl/vote_requester_test.cpp b/src/mongo/db/repl/vote_requester_test.cpp
index 31ca9c5fc5e..53c810a3a46 100644
--- a/src/mongo/db/repl/vote_requester_test.cpp
+++ b/src/mongo/db/repl/vote_requester_test.cpp
@@ -560,6 +560,32 @@ TEST_F(VoteRequesterCatchupTakeoverDryRunTest, CatchupTakeoverPrimarySaysNoLoseE
     stopCapturingLogMessages();
 }
 
+TEST_F(VoteRequesterCatchupTakeoverDryRunTest,
+       CatchupTakeoverAllNodesRespondedMeansSufficientResponses) {
+    startCapturingLogMessages();
+    ASSERT_FALSE(hasReceivedSufficientResponses());
+
+    // Getting a good response from the other secondaries is insufficient.
+    processResponse(requestFrom("host2"), votedYes());
+    processResponse(requestFrom("host3"), votedYes());
+    processResponse(requestFrom("host4"), votedYes());
+    ASSERT_FALSE(hasReceivedSufficientResponses());
+
+    // Getting an error response from the primary means all targets have responded, and so we have
+    // received sufficient responses.
+    processResponse(requestFrom("host1"), badRemoteCommandResponse());
+    ASSERT_EQUALS(1, countLogLinesContaining("failed to receive response from host1:27017"));
+    ASSERT_TRUE(hasReceivedSufficientResponses());
+
+    // An error response from the primary is equivalent to it saying NO.
+    ASSERT(VoteRequester::Result::kPrimaryRespondedNo == getResult());
+
+    // Only the secondaries are counted; the primary is excluded from the responders since it gave
+    // an error response.
+    ASSERT_EQUALS(3, getNumResponders());
+    stopCapturingLogMessages();
+}
+
 }  // namespace
 }  // namespace repl
 }  // namespace mongo