SERVER-39621 Change sync source when primary steps down and chaining is disabled

(cherry picked from commit 2ffaa9d4efefffc7045b6b47d9380299b28dfd7a)
(cherry picked from commit 03db775aaf4e3167894092aa4bdfbb980b06c703)

4 files changed, 139 insertions, 0 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 5c8bc72d7ab..b876158d261 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -110,6 +110,8 @@ all:
     test_file: jstests/replsets/not_primary_errors_returned_during_rollback_if_helloOk.js
   - ticket: SERVER-55725
     test_file: jstests/sharding/time_zone_info_mongos.js
+  - ticket: SERVER-39621
+    test_file: jstests/replsets/step_down_chaining_disabled.js
 
 # Tests that should only be excluded from particular suites should be listed under that suite.
 suites:
diff --git a/jstests/replsets/step_down_chaining_disabled.js b/jstests/replsets/step_down_chaining_disabled.js
new file mode 100644
index 00000000000..b99f18ce4ab
--- /dev/null
+++ b/jstests/replsets/step_down_chaining_disabled.js
@@ -0,0 +1,33 @@
+/**
+ * Tests that if chaining is disabled, electing a new primary will cause nodes to start syncing from
+ * the new primary.
+ */
+
+(function() {
+"use strict";
+
+const replSet = new ReplSetTest({
+    nodes: 3,
+    settings: {chainingAllowed: false},
+    // We will turn on the noop writer after newPrimary is elected to ensure that newPrimary will
+    // eventually be an eligible sync source for the secondary. We don't turn it on at the start of
+    // the test because the noop writer could cause newPrimary to lose the election.
+    nodeOptions: {setParameter: {writePeriodicNoops: false, periodicNoopIntervalSecs: 1}}
+});
+replSet.startSet();
+replSet.initiateWithHighElectionTimeout();
+
+const oldPrimary = replSet.getPrimary();
+const [newPrimary, secondary] = replSet.getSecondaries();
+
+replSet.awaitSyncSource(secondary, oldPrimary);
+
+replSet.stepUp(newPrimary);
+
+// Enable periodic noops so that the secondary can sync from newPrimary.
+assert.commandWorked(newPrimary.adminCommand({setParameter: 1, writePeriodicNoops: true}));
+
+replSet.awaitSyncSource(secondary, newPrimary);
+
+replSet.stopSet();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index d1e2a03f005..6a02ad97fd1 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -2683,6 +2683,18 @@ bool TopologyCoordinator::shouldChangeSyncSource(
         return false;
     }
 
+    // Change sync source if chaining is disabled, we are not syncing from the primary, and we know
+    // who the new primary is. We do not consider chaining disabled if we are the primary, since
+    // we are in catchup mode.
+    auto chainingDisabled = !_rsConfig.isChainingAllowed() && _currentPrimaryIndex != _selfIndex;
+    auto foundNewPrimary = _currentPrimaryIndex != -1 && _currentPrimaryIndex != currentSourceIndex;
+    if (primaryIndex != currentSourceIndex && chainingDisabled && foundNewPrimary) {
+        auto newPrimary = _rsConfig.getMemberAt(_currentPrimaryIndex).getHostAndPort();
+        log() << "Choosing new sync source: " << currentSource << " because chaining is disabled "
+              << "and we are aware of a new primary: " << newPrimary;
+        return true;
+    }
+
     // Change sync source if they are not ahead of us, and don't have a sync source,
     // unless they are primary.
     const OpTime myLastOpTime = getMyLastAppliedOpTime();
diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
index e2ce2fd9314..dc403532871 100644
--- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp
+++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
@@ -3963,6 +3963,98 @@ TEST_F(HeartbeatResponseTestV1, ShouldChangeSyncSourceWhenFresherMemberExists) {
     ASSERT_EQUALS(1, countLogLinesContaining("Choosing new sync source"));
 }
 
+TEST_F(HeartbeatResponseTestV1,
+       ShouldntChangeSyncSourceWhenNotSyncingFromPrimaryAndChainingDisabledButNoNewPrimary) {
+    // In this test, the TopologyCoordinator should not tell us to change sync sources away from
+    // "host2" since we are not aware of who the new primary is.
+
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version" << 5 << "members"
+                      << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                               << "host1:27017")
+                                    << BSON("_id" << 1 << "host"
+                                                  << "host2:27017")
+                                    << BSON("_id" << 2 << "host"
+                                                  << "host3:27017"))
+                      << "protocolVersion" << 1 << "settings"
+                      << BSON("heartbeatTimeoutSecs" << 5 << "chainingAllowed" << false)),
+                 0);
+
+    OpTime staleOpTime = OpTime(Timestamp(4, 0), 0);
+    OpTime freshOpTime = OpTime(Timestamp(5, 0), 0);
+
+    ASSERT_FALSE(getTopoCoord().shouldChangeSyncSource(
+        HostAndPort("host2"),
+        makeReplSetMetadata(),
+        makeOplogQueryMetadata(freshOpTime, -1 /* primaryIndex */, 2 /* syncSourceIndex */),
+        now()));
+}
+
+TEST_F(HeartbeatResponseTestV1,
+       ShouldChangeSyncSourceWhenNotSyncingFromPrimaryChainingDisabledAndFoundNewPrimary) {
+    // In this test, the TopologyCoordinator should tell us to change sync sources away from
+    // "host2" since "host3" is the new primary and chaining is disabled.
+
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version" << 5 << "members"
+                      << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                               << "host1:27017")
+                                    << BSON("_id" << 1 << "host"
+                                                  << "host2:27017")
+                                    << BSON("_id" << 2 << "host"
+                                                  << "host3:27017"))
+                      << "protocolVersion" << 1 << "settings"
+                      << BSON("heartbeatTimeoutSecs" << 5 << "chainingAllowed" << false)),
+                 0);
+
+    OpTime election = OpTime(Timestamp(1, 0), 0);
+    OpTime staleOpTime = OpTime(Timestamp(4, 0), 0);
+    OpTime freshOpTime = OpTime(Timestamp(5, 0), 0);
+
+    // Set that host3 is the new primary.
+    HeartbeatResponseAction nextAction = receiveUpHeartbeat(
+        HostAndPort("host3"), "rs0", MemberState::RS_PRIMARY, election, freshOpTime);
+    ASSERT_NO_ACTION(nextAction.getAction());
+
+    startCapturingLogMessages();
+    ASSERT(getTopoCoord().shouldChangeSyncSource(
+        HostAndPort("host2"),
+        makeReplSetMetadata(),
+        makeOplogQueryMetadata(freshOpTime, -1 /* primaryIndex */, 2 /* syncSourceIndex */),
+        now()));
+    stopCapturingLogMessages();
+    ASSERT_EQUALS(1, countLogLinesContaining("Choosing new sync source"));
+}
+
+TEST_F(HeartbeatResponseTestV1, ShouldntChangeSyncSourceWhenChainingDisabledAndWeArePrimary) {
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version" << 5 << "members"
+                      << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                               << "host1:27017")
+                                    << BSON("_id" << 1 << "host"
+                                                  << "host2:27017")
+                                    << BSON("_id" << 2 << "host"
+                                                  << "host3:27017"))
+                      << "protocolVersion" << 1 << "settings"
+                      << BSON("heartbeatTimeoutSecs" << 5 << "chainingAllowed" << false)),
+                 0);
+
+    OpTime staleOpTime = OpTime(Timestamp(4, 0), 0);
+    OpTime freshOpTime = OpTime(Timestamp(5, 0), 0);
+
+    // Set that we are primary.
+    getTopoCoord().setPrimaryIndex(0);
+
+    ASSERT_FALSE(getTopoCoord().shouldChangeSyncSource(
+        HostAndPort("host2"),
+        makeReplSetMetadata(),
+        makeOplogQueryMetadata(freshOpTime, -1 /* primaryIndex */, 2 /* syncSourceIndex */),
+        now()));
+}
+
 TEST_F(HeartbeatResponseTestV1, ShouldNotChangeSyncSourceWhenMemberHasYetToHeartbeatUs) {
     // In this test, the TopologyCoordinator should not tell us to change sync sources away from
     // "host2" since we do not use the member's heartbeatdata in pv1.