SERVER-46303 Omit configTerm -1 from heartbeats

5 files changed, 44 insertions, 13 deletions

diff --git a/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp b/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp
index 57f85aa58f9..05423e1b538 100644
--- a/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp
+++ b/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp
@@ -159,25 +159,27 @@ void ReplSetHeartbeatArgsV1::setCheckEmpty() {
     _checkEmpty = true;
 }
 
-BSONObj ReplSetHeartbeatArgsV1::toBSON(bool omitConfigTerm) const {
+BSONObj ReplSetHeartbeatArgsV1::toBSON() const {
     invariant(isInitialized());
     BSONObjBuilder builder;
-    addToBSON(&builder, omitConfigTerm);
+    addToBSON(&builder);
     return builder.obj();
 }
 
-void ReplSetHeartbeatArgsV1::addToBSON(BSONObjBuilder* builder, bool omitConfigTerm) const {
+void ReplSetHeartbeatArgsV1::addToBSON(BSONObjBuilder* builder) const {
     builder->append(kSetNameFieldName, _setName);
     if (_checkEmpty) {
         builder->append(kCheckEmptyFieldName, _checkEmpty);
     }
     builder->appendIntOrLL(kConfigVersionFieldName, _configVersion);
-    // Only attach the term field if we are fully upgraded to 4.4, since 4.2 nodes won't be able to
-    // parse it.
+    // The configTerm field is new in 4.4 and cannot be parsed by MongoDB 4.2. Therefore omit it if
+    // we have a 4.2-style replica set config with no "term". This permits us to downgrade by first
+    // removing the replica set config's term, then downgrading to 4.2.
+    // TODO (SERVER-46288): Don't check FCV.
     if (serverGlobalParams.featureCompatibility.isVersionInitialized() &&
         serverGlobalParams.featureCompatibility.getVersion() ==
             ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo44 &&
-        !omitConfigTerm) {
+        _configTerm != OpTime::kUninitializedTerm) {
         builder->appendIntOrLL(kConfigTermFieldName, _configTerm);
     }
     if (_hasHeartbeatVersion) {
diff --git a/src/mongo/db/repl/repl_set_heartbeat_args_v1.h b/src/mongo/db/repl/repl_set_heartbeat_args_v1.h
index f183a22072a..9b15f89750d 100644
--- a/src/mongo/db/repl/repl_set_heartbeat_args_v1.h
+++ b/src/mongo/db/repl/repl_set_heartbeat_args_v1.h
@@ -152,9 +152,9 @@ public:
      * Should only be called if the mandatory fields have been set.
      * Optional fields are only included if they have been set.
      */
-    BSONObj toBSON(bool omitConfigTerm = false) const;
+    BSONObj toBSON() const;
 
-    void addToBSON(BSONObjBuilder* builder, bool omitConfigTerm) const;
+    void addToBSON(BSONObjBuilder* builder) const;
 
 private:
     // look at the body of the isInitialized() function to see which fields are mandatory
diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
index 4a8c1ce7721..3961da51162 100644
--- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat.cpp
@@ -105,10 +105,7 @@ void ReplicationCoordinatorImpl::_doMemberHeartbeat(executor::TaskExecutor::Call
     Milliseconds timeout(0);
     const std::pair<ReplSetHeartbeatArgsV1, Milliseconds> hbRequest =
         _topCoord->prepareHeartbeatRequestV1(now, _settings.ourSetName(), target);
-    // Omit config term from all heartbeat requests sent by arbiter, since it cannot be parsed by
-    // 4.2 nodes and arbiters do not respect FCV setting.
-    bool omitConfigTerm = _getMemberState_inlock().arbiter();
-    heartbeatObj = hbRequest.first.toBSON(omitConfigTerm);
+    heartbeatObj = hbRequest.first.toBSON();
     timeout = hbRequest.second;
 
     const RemoteCommandRequest request(
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index c48c7df9595..5e307efaaaa 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -754,7 +754,10 @@ std::pair<ReplSetHeartbeatArgsV1, Milliseconds> TopologyCoordinator::prepareHear
     if (_rsConfig.isInitialized()) {
         hbArgs.setSetName(_rsConfig.getReplSetName());
         hbArgs.setConfigVersion(_rsConfig.getConfigVersion());
-        hbArgs.setConfigTerm(_rsConfig.getConfigTerm());
+        if (_rsConfig.getConfigTerm() != OpTime::kUninitializedTerm) {
+            hbArgs.setConfigTerm(_rsConfig.getConfigTerm());
+        }
+
         if (_selfIndex >= 0) {
             const MemberConfig& me = _selfConfig();
             hbArgs.setSenderId(me.getId().getData());
diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
index a578d50689d..a2fa3adcbd4 100644
--- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp
+++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp
@@ -2246,6 +2246,35 @@ TEST_F(PrepareHeartbeatResponseV1Test,
     ASSERT_EQUALS(HostAndPort("h2"), response.getSyncingTo());
 }
 
+TEST_F(TopoCoordTest, OmitUninitializedConfigTermFromHeartbeat) {
+    // A MongoDB 4.2 replica set config with no term.
+    updateConfig(BSON("_id"
+                      << "rs0"
+                      << "version" << 1 << "members"
+                      << BSON_ARRAY(BSON("_id" << 10 << "host"
+                                               << "hself")
+                                    << BSON("_id" << 20 << "host"
+                                                  << "h2"))),
+                 0);
+
+    // A member with this config omits configTerm from heartbeat requests.
+    auto [request, timeout] =
+        getTopoCoord().prepareHeartbeatRequestV1(Date_t(), "rs0", HostAndPort("h2", 27017));
+    (void)timeout;  // Unused.
+
+    ASSERT_EQUALS(request.getConfigTerm(), -1);
+    ASSERT_EQUALS(request.getConfigVersion(), 1);
+    ASSERT_FALSE(request.toBSON().hasField("configTerm"_sd));
+
+    // A member with this config omits configTerm from heartbeat responses.
+    ReplSetHeartbeatArgsV1 args;
+    args.setSetName("rs0");
+    args.setSenderId(20);
+    ReplSetHeartbeatResponse response;
+    ASSERT_OK(getTopoCoord().prepareHeartbeatResponseV1(now()++, args, "rs0", &response));
+    ASSERT_FALSE(response.toBSON().hasField("configTerm"_sd));
+}
+
 TEST_F(TopoCoordTest, BecomeCandidateWhenBecomingSecondaryInSingleNodeSet) {
     ASSERT_TRUE(TopologyCoordinator::Role::kFollower == getTopoCoord().getRole());
     ASSERT_EQUALS(MemberState::RS_STARTUP, getTopoCoord().getMemberState().s);