diff options
author | Matt Broadstone <mbroadst@mongodb.com> | 2022-05-25 21:57:58 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-05-25 22:27:17 +0000 |
commit | 25c8241437807544cd15d520a702b092146f4ace (patch) | |
tree | 36c3de5cc890608e97810af1f87574680efc00a9 | |
parent | ad379f1338379fa96dcdf52f6c1a2f3b68f90dc7 (diff) | |
download | mongo-25c8241437807544cd15d520a702b092146f4ace.tar.gz |
SERVER-46907 Speed up reconfig replication acknowledgement
6 files changed, 168 insertions, 229 deletions
diff --git a/jstests/replsets/remove_newly_added_field_after_finishing_initial_sync.js b/jstests/replsets/remove_newly_added_field_after_finishing_initial_sync.js index 59a6d7713bf..74fa230c7f4 100644 --- a/jstests/replsets/remove_newly_added_field_after_finishing_initial_sync.js +++ b/jstests/replsets/remove_newly_added_field_after_finishing_initial_sync.js @@ -214,6 +214,7 @@ rst.nodes[2].disconnect(rst.nodes); assert.commandWorked(primaryColl.insert({a: 8}, {writeConcern: {w: "majority"}})); // Only three nodes are needed for an election (0, 1, and 3). +rst.waitForConfigReplication(rst.nodes[0], [rst.nodes[1]]); assert.commandWorked(rst.nodes[1].adminCommand({replSetStepUp: 1})); assert.eq(rst.getPrimary(), rst.nodes[1]); rst.waitForConfigReplication(rst.nodes[1], [rst.nodes[0], rst.nodes[1], rst.nodes[3]]); diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index e5b56720dc1..99203742cab 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -5785,7 +5785,14 @@ Status ReplicationCoordinatorImpl::processHeartbeatV1(const ReplSetHeartbeatArgs auto senderHost(args.getSenderHost()); const Date_t now = _replExecutor->now(); - result = _topCoord->prepareHeartbeatResponseV1(now, args, replSetName, response); + auto configChanged = _topCoord->prepareHeartbeatResponseV1(now, args, replSetName, response); + result = configChanged.getStatus(); + if (configChanged.isOK() && configChanged.getValue()) { + // If the latest heartbeat indicates that the remote node's config has changed, we want to + // update it's member data as soon as possible. Send an immediate hearbeat, and update the + // member data on processing its response. + _scheduleHeartbeatToTarget_inlock(senderHost, now, replSetName); + } if ((result.isOK() || result == ErrorCodes::InvalidReplicaSetConfig) && _selfIndex < 0) { // If this node does not belong to the configuration it knows about, send heartbeats diff --git a/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp index c9954a761c0..5203980b575 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_heartbeat_v1_test.cpp @@ -80,6 +80,26 @@ TEST(ReplSetHeartbeatArgs, AcceptsUnknownField) { ASSERT_BSONOBJ_EQ(bob2.obj(), cmdObj); } +auto makePrimaryHeartbeatResponseFrom(const ReplSetConfig& rsConfig, + const std::string& setName = "mySet") { + // Construct the heartbeat response containing the newer config. + ReplSetHeartbeatResponse hbResp; + hbResp.setSetName(setName); + hbResp.setState(MemberState::RS_PRIMARY); + hbResp.setConfigVersion(rsConfig.getConfigVersion()); + hbResp.setConfigTerm(rsConfig.getConfigTerm()); + // The smallest valid optime in PV1. + OpTime opTime(Timestamp(1, 1), 0); + hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + hbResp.setDurableOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + BSONObjBuilder responseBuilder; + responseBuilder << "ok" << 1; + hbResp.addToBSON(&responseBuilder); + // Add the raw config object. + responseBuilder << "config" << rsConfig.toBSON(); + return responseBuilder.obj(); +} + class ReplCoordHBV1Test : public ReplCoordTest { protected: explicit ReplCoordHBV1Test() : ReplCoordTest(Options{}.useMockClock(true)) {} @@ -152,20 +172,9 @@ void ReplCoordHBV1Test::processResponseFromPrimary(const ReplSetConfig& config, ASSERT_EQUALS("mySet", hbArgs.getSetName()); ASSERT_EQUALS(version, hbArgs.getConfigVersion()); ASSERT_EQUALS(term, hbArgs.getTerm()); - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(config.getConfigVersion()); - hbResp.setConfig(config); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - net->scheduleResponse( - noi, startDate + Milliseconds(200), makeResponseStatus(responseBuilder.obj())); + + auto response = makePrimaryHeartbeatResponseFrom(config); + net->scheduleResponse(noi, startDate + Milliseconds(200), makeResponseStatus(response)); assertRunUntil(startDate + Milliseconds(200)); } @@ -391,9 +400,9 @@ TEST_F(ReplCoordHBV1Test, RestartingHeartbeatsShouldOnlyCancelScheduledHeartbeat hbResp.setState(MemberState::RS_SECONDARY); hbResp.setConfigVersion(rsConfig.getConfigVersion()); // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); + OpTime opTime(Timestamp(1, 1), 0); + hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + hbResp.setDurableOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); BSONObjBuilder responseBuilder; responseBuilder << "ok" << 1; hbResp.addToBSON(&responseBuilder); @@ -601,9 +610,9 @@ public: hbResp.setConfigVersion(config.getConfigVersion()); hbResp.setConfigTerm(config.getConfigTerm()); // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); + OpTime opTime(Timestamp(1, 1), 0); + hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + hbResp.setDurableOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); BSONObjBuilder responseBuilder; responseBuilder << "ok" << 1; hbResp.addToBSON(&responseBuilder); @@ -749,21 +758,9 @@ TEST_F(ReplCoordHBV1SplitConfigTest, RejectMismatchedSetNameInHeartbeatResponse) // Schedule a heartbeat response which reports the higher (term, version) but wrong setName auto response = [&]() { - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("differentSetName"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfig( - ReplSetConfig::parse(makeConfigObj((_configVersion + 1), (_configTerm + 1)))); - hbResp.setConfigVersion(_configVersion + 1); - hbResp.setConfigTerm(_configTerm + 1); - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - return responseBuilder.obj(); + auto newConfig = + ReplSetConfig::parse(makeConfigObj((_configVersion + 1), (_configTerm + 1))); + return makePrimaryHeartbeatResponseFrom(newConfig, "differentSetName"); }(); getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); @@ -891,25 +888,9 @@ TEST_F(ReplCoordHBV1ReconfigTest, ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << makeConfigObj(initConfigVersion + 1, initConfigTerm); - auto origResObj = responseBuilder.obj(); - - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the newer config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigVersion(), initConfigVersion + 1); @@ -935,25 +916,9 @@ TEST_F(ReplCoordHBV1ReconfigTest, ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << makeConfigObj(initConfigVersion, initConfigTerm + 1); - auto origResObj = responseBuilder.obj(); - - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the new config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigTerm(), initConfigTerm + 1); @@ -988,25 +953,9 @@ TEST_F( ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << makeConfigObj((initConfigVersion - 1), (initConfigTerm + 1)); - auto origResObj = responseBuilder.obj(); - - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the newer config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigVersion(), (initConfigVersion - 1)); @@ -1034,25 +983,9 @@ TEST_F( ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << makeConfigObj((initConfigVersion + 1), UninitializedTerm); - auto origResObj = responseBuilder.obj(); - - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the newer config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigVersion(), (initConfigVersion + 1)); @@ -1079,19 +1012,7 @@ TEST_F(ReplCoordHBV1ReconfigTest, ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfig(rsConfig); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - auto origResObj = responseBuilder.obj(); + auto origResObj = makePrimaryHeartbeatResponseFrom(rsConfig); // Construct a heartbeat response object that omits the top-level 't' field and the 'term' field // from the config object. This simulates the case of receiving a heartbeat response from a 4.2 @@ -1143,28 +1064,12 @@ TEST_F(ReplCoordHBV1ReconfigTest, ConfigWithTermAndVersionChangeOnlyDoesntCallIs ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << rsConfig.toBSON(); - auto origResObj = responseBuilder.obj(); - // Make isSelf not work. getExternalState()->clearSelfHosts(); - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the newer config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigTerm(), initConfigTerm + 1); @@ -1199,25 +1104,9 @@ TEST_F(ReplCoordHBV1ReconfigTest, ConfigWithSignificantChangeDoesCallIsSelf) { ASSERT_EQUALS(initConfigTerm, hbArgs.getConfigTerm()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - // Construct the heartbeat response containing the newer config. - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfigTerm(rsConfig.getConfigTerm()); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - // Add the raw config object. - responseBuilder << "config" << rsConfig.toBSON(); - auto origResObj = responseBuilder.obj(); - - // Schedule and deliver the heartbeat response. - getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(origResObj)); + // Schedule and deliver the heartbeat response containing the newer config. + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); getNet()->runReadyNetworkOperations(); ASSERT_EQ(getReplCoord()->getConfigTerm(), initConfigTerm); @@ -1232,6 +1121,66 @@ TEST_F(ReplCoordHBV1ReconfigTest, ConfigWithSignificantChangeDoesCallIsSelf) { ASSERT_EQ(rsConfig.getMemberAt(2).getId().getData(), getReplCoord()->getMyId()); } +TEST_F(ReplCoordHBV1ReconfigTest, ScheduleImmediateHeartbeatToSpeedUpConfigCommitment) { + // Prepare a config which is newer than the installed config + ReplSetConfig rsConfig = [&]() { + auto mutableConfig = + makeRSConfigWithVersionAndTerm(initConfigVersion + 1, initConfigTerm + 1).getMutable(); + ReplSetConfigSettings settings; + settings.setHeartbeatIntervalMillis(10000); + mutableConfig.setSettings(settings); + return ReplSetConfig(std::move(mutableConfig)); + }(); + + // Receive the newer config from the primary + receiveHeartbeatFrom(rsConfig, 1, HostAndPort("h2", 1)); + { + InNetworkGuard guard(getNet()); + auto noi = getNet()->getNextReadyRequest(); + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); + getNet()->runReadyNetworkOperations(); + + // Ignore the immediate heartbeats sent to secondaries as part of the reconfig process + getNet()->blackHole(getNet()->getNextReadyRequest()); + getNet()->blackHole(getNet()->getNextReadyRequest()); + } + + // Receive a heartbeat from secondary with the updated configVersionAndTerm before the primary + // has updated its in-memory MemberData for this secondary + receiveHeartbeatFrom(rsConfig, 1, HostAndPort("h1", 1)); + + // Verify that we schedule an _immediate_ heartbeat to the node with old config + { + InNetworkGuard guard(getNet()); + ASSERT_TRUE(getNet()->hasReadyRequests()); + auto noi = getNet()->getNextReadyRequest(); + const RemoteCommandRequest& hbrequest = noi->getRequest(); + ASSERT_EQUALS(HostAndPort("h1", 1), hbrequest.target); + + ReplSetHeartbeatArgsV1 hbArgs; + ASSERT_OK(hbArgs.initialize(hbrequest.cmdObj)); + ASSERT_EQUALS("mySet", hbArgs.getSetName()); + ASSERT_EQUALS(rsConfig.getConfigVersion(), hbArgs.getConfigVersion()); + ASSERT_EQUALS(rsConfig.getConfigTerm(), hbArgs.getConfigTerm()); + + // Schedule a response to the immediate heartbeat check which returns the config + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + getNet()->scheduleResponse(noi, getNet()->now(), makeResponseStatus(response)); + getNet()->runReadyNetworkOperations(); + } + + // Now receive a heartbeat from the same secondary with the updated configVersionAndTerm after + // the primary has updated its in-memory MemberData for that node + receiveHeartbeatFrom(rsConfig, 1, HostAndPort("h1", 1)); + + // Verify that we DO NOT schedule an immediate heartbeat to the node + { + InNetworkGuard guard(getNet()); + ASSERT_FALSE(getNet()->hasReadyRequests()); + } +} + TEST_F(ReplCoordHBV1Test, RejectHeartbeatReconfigDuringElection) { auto severityGuard = unittest::MinimumLoggedSeverityGuard{ logv2::LogComponent::kReplicationHeartbeats, logv2::LogSeverity::Debug(1)}; @@ -1358,20 +1307,8 @@ TEST_F(ReplCoordHBV1Test, AwaitHelloReturnsResponseOnReconfigViaHeartbeat) { enterNetwork(); NetworkInterfaceMock::NetworkOperationIterator noi = net->getNextReadyRequest(); - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfig(rsConfig); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - net->scheduleResponse( - noi, startDate + Milliseconds(200), makeResponseStatus(responseBuilder.obj())); + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + net->scheduleResponse(noi, startDate + Milliseconds(200), makeResponseStatus(response)); assertRunUntil(startDate + Milliseconds(200)); performSyncToFinishReconfigHeartbeat(); @@ -1460,20 +1397,9 @@ TEST_F(ReplCoordHBV1Test, ASSERT_EQUALS("mySet", hbArgs.getSetName()); ASSERT_EQUALS(-2, hbArgs.getConfigVersion()); ASSERT_EQUALS(OpTime::kInitialTerm, hbArgs.getTerm()); - ReplSetHeartbeatResponse hbResp; - hbResp.setSetName("mySet"); - hbResp.setState(MemberState::RS_PRIMARY); - hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setConfig(rsConfig); - // The smallest valid optime in PV1. - OpTime opTime(Timestamp(), 0); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); - BSONObjBuilder responseBuilder; - responseBuilder << "ok" << 1; - hbResp.addToBSON(&responseBuilder); - net->scheduleResponse( - noi, startDate + Milliseconds(50), makeResponseStatus(responseBuilder.obj())); + + auto response = makePrimaryHeartbeatResponseFrom(rsConfig); + net->scheduleResponse(noi, startDate + Milliseconds(50), makeResponseStatus(response)); assertRunUntil(startDate + Milliseconds(550)); performSyncToFinishReconfigHeartbeat(); @@ -1574,8 +1500,8 @@ TEST_F(ReplCoordHBV1Test, IgnoreTheContentsOfMetadataWhenItsReplicaSetIdDoesNotM hbResp.setSetName(rsConfig.getReplSetName()); hbResp.setState(MemberState::RS_PRIMARY); hbResp.setConfigVersion(rsConfig.getConfigVersion()); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); + hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + hbResp.setDurableOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); BSONObjBuilder responseBuilder; responseBuilder << "ok" << 1; @@ -1935,23 +1861,23 @@ TEST_F(ReplCoordHBV1Test, handleHeartbeatResponseForTestEnqueuesValidHandle) { /** - * Test a concurrent stepdown and reconfig. The stepdown is triggered by a heartbeat response with - * a higher term, the reconfig is triggered either by a heartbeat with a new config, or by a user - * replSetReconfig command. + * Test a concurrent stepdown and reconfig. The stepdown is triggered by a heartbeat response + * with a higher term, the reconfig is triggered either by a heartbeat with a new config, or by + * a user replSetReconfig command. * - * In setUp, the replication coordinator is initialized so "self" is the primary of a 3-node set. - * The coordinator schedules heartbeats to the other nodes but this test doesn't respond to those - * heartbeats. Instead, it creates heartbeat responses that have no associated requests, and injects - * the responses via handleHeartbeatResponse_forTest. + * In setUp, the replication coordinator is initialized so "self" is the primary of a 3-node + * set. The coordinator schedules heartbeats to the other nodes but this test doesn't respond to + * those heartbeats. Instead, it creates heartbeat responses that have no associated requests, + * and injects the responses via handleHeartbeatResponse_forTest. * - * Each subclass of HBStepdownAndReconfigTest triggers some sequence of stepdown and reconfig steps. - * The exact sequences are nondeterministic, since we don't use failpoints or NetworkInterfaceMock - * to force a specific order. + * Each subclass of HBStepdownAndReconfigTest triggers some sequence of stepdown and reconfig + * steps. The exact sequences are nondeterministic, since we don't use failpoints or + * NetworkInterfaceMock to force a specific order. * - * Tests assert that stepdown via heartbeat completed, and the tests that send the new config via - * heartbeat assert that the new config was stored. Tests that send the new config with the - * replSetReconfig command don't check that it was stored; if the stepdown finished first then the - * replSetReconfig was rejected with a NotWritablePrimary error. + * Tests assert that stepdown via heartbeat completed, and the tests that send the new config + * via heartbeat assert that the new config was stored. Tests that send the new config with the + * replSetReconfig command don't check that it was stored; if the stepdown finished first then + * the replSetReconfig was rejected with a NotWritablePrimary error. */ class HBStepdownAndReconfigTest : public ReplCoordHBV1Test { protected: @@ -2013,7 +1939,8 @@ void HBStepdownAndReconfigTest::setUp() { // To complete a reconfig from Config 1 to Config 2 requires: // Oplog Commitment: last write in previous Config 0 is majority-committed. - // Config Replication: Config 2 gossipped by heartbeat response to majority of Config 2 members. + // Config Replication: Config 2 gossipped by heartbeat response to majority of Config 2 + // members. // // Catch up all members to the same OpTime to ensure Oplog Commitment in all tests. // In tests that require it, we ensure Config Replication with acknowledgeReconfigCommand(). @@ -2042,8 +1969,8 @@ void HBStepdownAndReconfigTest::sendHBResponse(int targetIndex, hbResp.setTerm(term); hbResp.setConfigVersion(configVersion); hbResp.setConfigTerm(configTerm); - hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t()}); - hbResp.setDurableOpTimeAndWallTime({opTime, Date_t()}); + hbResp.setAppliedOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); + hbResp.setDurableOpTimeAndWallTime({opTime, Date_t() + Seconds{1}}); if (includeConfig) { auto configDoc = MutableDocument(Document(_initialConfig)); @@ -2089,14 +2016,15 @@ Future<void> HBStepdownAndReconfigTest::startReconfigCommand() { _threadPool->schedule( [promise = std::move(promise), coord, args, opCtx = std::move(opCtx)](Status) mutable { - // Avoid the need to respond to quorum-check heartbeats sent to the other two members. - // These heartbeats are sent *before* reconfiguring, they're distinct from the oplog - // commitment and config replication checks. + // Avoid the need to respond to quorum-check heartbeats sent to the other two + // members. These heartbeats are sent *before* reconfiguring, they're distinct from + // the oplog commitment and config replication checks. FailPointEnableBlock omitConfigQuorumCheck("omitConfigQuorumCheck"); BSONObjBuilder result; auto status = Status::OK(); try { - // OK for processReplSetReconfig to return, throw NotPrimary-like error, or succeed. + // OK for processReplSetReconfig to return, throw NotPrimary-like error, or + // succeed. status = coord->processReplSetReconfig(opCtx.get(), args, &result); } catch (const DBException&) { status = exceptionToStatus(); diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp index 05aa6620ed8..7f30b7b113d 100644 --- a/src/mongo/db/repl/topology_coordinator.cpp +++ b/src/mongo/db/repl/topology_coordinator.cpp @@ -786,11 +786,12 @@ void TopologyCoordinator::prepareSyncFromResponse(const HostAndPort& target, *result = Status::OK(); } -// produce a reply to a heartbeat -Status TopologyCoordinator::prepareHeartbeatResponseV1(Date_t now, - const ReplSetHeartbeatArgsV1& args, - StringData ourSetName, - ReplSetHeartbeatResponse* response) { +// produce a reply to a heartbeat, and return whether the remote node's config has changed. +StatusWith<bool> TopologyCoordinator::prepareHeartbeatResponseV1( + Date_t now, + const ReplSetHeartbeatArgsV1& args, + StringData ourSetName, + ReplSetHeartbeatResponse* response) { // Verify that replica set names match const std::string rshb = args.getSetName(); if (ourSetName != rshb) { @@ -853,7 +854,7 @@ Status TopologyCoordinator::prepareHeartbeatResponseV1(Date_t now, if (!_rsConfig.isInitialized()) { response->setConfigVersion(-2); - return Status::OK(); + return false; } response->setElectable( @@ -874,7 +875,7 @@ Status TopologyCoordinator::prepareHeartbeatResponseV1(Date_t now, from = _getMemberIndex(args.getSenderId()); } if (from == -1) { - return Status::OK(); + return false; } invariant(from != _selfIndex); @@ -883,7 +884,7 @@ Status TopologyCoordinator::prepareHeartbeatResponseV1(Date_t now, fromNodeData.setLastHeartbeatRecv(now); // Update liveness for sending node. fromNodeData.updateLiveness(now); - return Status::OK(); + return fromNodeData.getConfigVersionAndTerm() < args.getConfigVersionAndTerm(); } int TopologyCoordinator::_getMemberIndex(int id) const { diff --git a/src/mongo/db/repl/topology_coordinator.h b/src/mongo/db/repl/topology_coordinator.h index c80daba86a9..fb9f7a196f7 100644 --- a/src/mongo/db/repl/topology_coordinator.h +++ b/src/mongo/db/repl/topology_coordinator.h @@ -366,11 +366,11 @@ public: BSONObjBuilder* response, Status* result); - // produce a reply to a V1 heartbeat - Status prepareHeartbeatResponseV1(Date_t now, - const ReplSetHeartbeatArgsV1& args, - StringData ourSetName, - ReplSetHeartbeatResponse* response); + // produce a reply to a V1 heartbeat, and return whether the remote node's config has changed. + StatusWith<bool> prepareHeartbeatResponseV1(Date_t now, + const ReplSetHeartbeatArgsV1& args, + StringData ourSetName, + ReplSetHeartbeatResponse* response); struct ReplSetStatusArgs { const Date_t now; diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp index 1d2587bb48e..2aaf9aed925 100644 --- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp +++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp @@ -2320,7 +2320,8 @@ public: void prepareHeartbeatResponseV1(const ReplSetHeartbeatArgsV1& args, ReplSetHeartbeatResponse* response, Status* result) { - *result = getTopoCoord().prepareHeartbeatResponseV1(now()++, args, "rs0", response); + *result = + getTopoCoord().prepareHeartbeatResponseV1(now()++, args, "rs0", response).getStatus(); } int initConfigVersion = 1; @@ -2402,7 +2403,8 @@ TEST_F(TopoCoordTest, SetConfigVersionToNegativeTwoInHeartbeatResponseWhenNoConf args.setSenderId(20); ReplSetHeartbeatResponse response; // prepare response and check the results - Status result = getTopoCoord().prepareHeartbeatResponseV1(now()++, args, "rs0", &response); + Status result = + getTopoCoord().prepareHeartbeatResponseV1(now()++, args, "rs0", &response).getStatus(); ASSERT_OK(result); // this change to true because we can now see a majority, unlike in the previous cases ASSERT_EQUALS("rs0", response.getReplicaSetName()); |