From 64fcdabe8c7cc38188f31fd606379f935c94555a Mon Sep 17 00:00:00 2001 From: Judah Schvimer Date: Mon, 8 Jun 2020 23:20:37 +0000 Subject: SERVER-46541 enable automatic reconfigs for initial sync semantics by default --- src/mongo/db/repl/member_config_test.cpp | 40 ----- src/mongo/db/repl/repl_server_parameters.idl | 2 +- src/mongo/db/repl/repl_set_commands.cpp | 18 +- src/mongo/db/repl/repl_set_config_checks_test.cpp | 54 ------ src/mongo/db/repl/repl_set_config_test.cpp | 35 ---- src/mongo/db/repl/replication_coordinator.h | 6 +- src/mongo/db/repl/replication_coordinator_impl.cpp | 9 +- src/mongo/db/repl/replication_coordinator_impl.h | 3 +- .../replication_coordinator_impl_reconfig_test.cpp | 113 +++--------- src/mongo/db/repl/replication_coordinator_mock.cpp | 3 +- src/mongo/db/repl/replication_coordinator_mock.h | 4 +- src/mongo/db/repl/replication_coordinator_noop.cpp | 3 +- src/mongo/db/repl/replication_coordinator_noop.h | 4 +- .../db/repl/replication_coordinator_test_fixture.h | 4 +- .../embedded/replication_coordinator_embedded.cpp | 3 +- .../embedded/replication_coordinator_embedded.h | 4 +- src/mongo/shell/assert.js | 5 +- src/mongo/shell/replsettest.js | 197 +++++++++++++++++---- 18 files changed, 233 insertions(+), 274 deletions(-) (limited to 'src/mongo') diff --git a/src/mongo/db/repl/member_config_test.cpp b/src/mongo/db/repl/member_config_test.cpp index 8b52a238b16..00c9fb36765 100644 --- a/src/mongo/db/repl/member_config_test.cpp +++ b/src/mongo/db/repl/member_config_test.cpp @@ -235,11 +235,6 @@ void setNewlyAdded_ForTest(MemberConfig* mc, boost::optional newlyAdded) { namespace { TEST(MemberConfig, ParseAndSetNewlyAddedField) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; { MemberConfig mc(BSON("_id" << 0 << "host" @@ -261,11 +256,6 @@ TEST(MemberConfig, ParseAndSetNewlyAddedField) { } TEST(MemberConfig, NewlyAddedSetToFalseShouldThrow) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; ASSERT_THROWS(MemberConfig(BSON("_id" << 0 << "host" << "h" @@ -275,11 +265,6 @@ TEST(MemberConfig, NewlyAddedSetToFalseShouldThrow) { } TEST(MemberConfig, VotingNodeWithNewlyAddedFieldShouldStillHaveVoteAfterToBSON) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; // Create a member with 'newlyAdded: true'. @@ -306,11 +291,6 @@ TEST(MemberConfig, VotingNodeWithNewlyAddedFieldShouldStillHaveVoteAfterToBSON) } TEST(MemberConfig, NonVotingNodesWithNewlyAddedFieldShouldStillHaveZeroVotesAfterToBSON) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; MemberConfig mc(BSON("_id" << 0 << "host" @@ -374,11 +354,6 @@ TEST(MemberConfig, VotingNodesShouldStillHaveVoteAfterToBSON) { } TEST(MemberConfig, NodeWithNewlyAddedFieldShouldStillHavePriorityAfterToBSON) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; // Create a member with 'newlyAdded: true' and 'priority: 3'. @@ -403,11 +378,6 @@ TEST(MemberConfig, NodeWithNewlyAddedFieldShouldStillHavePriorityAfterToBSON) { } TEST(MemberConfig, PriorityZeroNodeWithNewlyAddedFieldShouldStillHaveZeroPriorityAfterToBSON) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; // Create a member with 'newlyAdded: true' and 'priority: 0'. @@ -484,11 +454,6 @@ TEST(MemberConfig, NodeShouldStillHavePriorityAfterToBSON) { } TEST(MemberConfig, CanOmitNewlyAddedFieldInBSONViaToBSONWithoutNewlyAdded) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; // Create a member with 'newlyAdded: true' and 'priority: 0'. @@ -514,11 +479,6 @@ TEST(MemberConfig, CanOmitNewlyAddedFieldInBSONViaToBSONWithoutNewlyAdded) { } TEST(MemberConfig, ArbiterCannotHaveNewlyAddedFieldSet) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetTagConfig tagConfig; // Verify that an exception is thrown when we try to create an arbiter with 'newlyAdded' field. diff --git a/src/mongo/db/repl/repl_server_parameters.idl b/src/mongo/db/repl/repl_server_parameters.idl index 8e345c8fc92..f1b801ee1f4 100644 --- a/src/mongo/db/repl/repl_server_parameters.idl +++ b/src/mongo/db/repl/repl_server_parameters.idl @@ -276,7 +276,7 @@ server_parameters: test_only: true cpp_vartype: bool cpp_varname: enableAutomaticReconfig - default: false + default: true oplogApplicationEnforcesSteadyStateConstraints: description: >- diff --git a/src/mongo/db/repl/repl_set_commands.cpp b/src/mongo/db/repl/repl_set_commands.cpp index de4411d1488..22a42f2da65 100644 --- a/src/mongo/db/repl/repl_set_commands.cpp +++ b/src/mongo/db/repl/repl_set_commands.cpp @@ -77,7 +77,7 @@ namespace repl { using std::string; using std::stringstream; -static const std::string kReplSetReconfigNss = "local.replset.reconfig"; +constexpr StringData kInternalIncludeNewlyAddedFieldName = "$_internalIncludeNewlyAdded"_sd; class ReplExecutorSSM : public ServerStatusMetric { public: @@ -206,7 +206,21 @@ public: bool wantCommitmentStatus; uassertStatusOK(bsonExtractBooleanFieldWithDefault( cmdObj, "commitmentStatus", false, &wantCommitmentStatus)); - ReplicationCoordinator::get(opCtx)->processReplSetGetConfig(&result, wantCommitmentStatus); + + if (cmdObj[kInternalIncludeNewlyAddedFieldName]) { + uassert(ErrorCodes::InvalidOptions, + "The '$_internalIncludeNewlyAdded' option is only supported when testing" + " commands are enabled", + getTestCommandsEnabled()); + } + + bool includeNewlyAdded; + uassertStatusOK(bsonExtractBooleanFieldWithDefault( + cmdObj, kInternalIncludeNewlyAddedFieldName, false, &includeNewlyAdded)); + + ReplicationCoordinator::get(opCtx)->processReplSetGetConfig( + &result, wantCommitmentStatus, includeNewlyAdded); + return true; } diff --git a/src/mongo/db/repl/repl_set_config_checks_test.cpp b/src/mongo/db/repl/repl_set_config_checks_test.cpp index 6a98744db44..ae90eb20f83 100644 --- a/src/mongo/db/repl/repl_set_config_checks_test.cpp +++ b/src/mongo/db/repl/repl_set_config_checks_test.cpp @@ -233,10 +233,6 @@ TEST_F(ServiceContextTest, ValidateConfigForInitiate_ArbiterPriorityMustBeZeroOr } TEST_F(ServiceContextTest, ValidateConfigForInitiate_NewlyAddedFieldNotAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); ReplSetConfig firstNewlyAdded; ReplSetConfig lastNewlyAdded; OID newReplSetId = OID::gen(); @@ -1076,22 +1072,12 @@ TEST_F(ServiceContextTest, ValidateForReconfig_MultiNodeAdditionOfArbitersDisall } TEST_F(ServiceContextTest, ValidateForReconfig_SingleNodeAdditionOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2); BSONArray newMembers = BSON_ARRAY(m1 << m2 << m3_NewlyAdded); // add 1 'newlyAdded' node. ASSERT_OK(validateMemberReconfig(oldMembers, newMembers, m1)); } TEST_F(ServiceContextTest, ValidateForReconfig_MultiNodeAdditionOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2); BSONArray newMembers = BSON_ARRAY(m1 << m2 << m3_NewlyAdded << m4_NewlyAdded); // add 2 'newlyAdded' nodes. @@ -1099,22 +1085,12 @@ TEST_F(ServiceContextTest, ValidateForReconfig_MultiNodeAdditionOfNewlyAddedAllo } TEST_F(ServiceContextTest, ValidateForReconfig_MultiNodeRemovalOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded << m3_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1); // Remove 2 'newlyAdded' nodes. ASSERT_OK(validateMemberReconfig(oldMembers, newMembers, m1)); } TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAddAndRemoveOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1 << m3_NewlyAdded); // Remove 'newlyAdded' 2, add 'newlyAdded' 3. @@ -1122,22 +1098,12 @@ TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAddAndRemoveOfNewlyAd } TEST_F(ServiceContextTest, ValidateForReconfig_SingleAutoReconfigAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1 << m2); // Remove 'newlyAdded' 2, add voting node 2. ASSERT_OK(validateMemberReconfig(oldMembers, newMembers, m1)); } TEST_F(ServiceContextTest, ValidateForReconfig_MultiAutoReconfigDisallowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded << m3_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1 << m2 << m3); // Remove 'newlyAdded' 2 & 3, add voting node 2 & 3. @@ -1147,11 +1113,6 @@ TEST_F(ServiceContextTest, ValidateForReconfig_MultiAutoReconfigDisallowed) { TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAutoReconfigAndAdditionOfVoterNodeDisallowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1 << m2 << m3); // Remove 'newlyAdded' 2, add voting node 2 & 3. @@ -1161,11 +1122,6 @@ TEST_F(ServiceContextTest, TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAutoReconfigAndRemovalOfVoterNodeDisallowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded << m3); BSONArray newMembers = BSON_ARRAY(m1 << m2); // Remove 'newlyAdded' 2 and voter node 3, add voting node 2. @@ -1175,11 +1131,6 @@ TEST_F(ServiceContextTest, TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAutoReconfigAndAdditionOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded); BSONArray newMembers = BSON_ARRAY( m1 << m2 << m3_NewlyAdded); // Remove 'newlyAdded' 2, add voting node 2 & 'newlyAdded' 3. @@ -1188,11 +1139,6 @@ TEST_F(ServiceContextTest, TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAutoReconfigAndRemovalOfNewlyAddedAllowed) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - BSONArray oldMembers = BSON_ARRAY(m1 << m2_NewlyAdded << m3_NewlyAdded); BSONArray newMembers = BSON_ARRAY(m1 << m2); // Remove 'newlyAdded' 2 & 3, add voting node 2. ASSERT_OK(validateMemberReconfig(oldMembers, newMembers, m1)); diff --git a/src/mongo/db/repl/repl_set_config_test.cpp b/src/mongo/db/repl/repl_set_config_test.cpp index 20d7bb4270d..16618f56cbf 100644 --- a/src/mongo/db/repl/repl_set_config_test.cpp +++ b/src/mongo/db/repl/repl_set_config_test.cpp @@ -891,11 +891,6 @@ TEST(ReplSetConfig, ConfigServerField) { } TEST(ReplSetConfig, SetNewlyAddedFieldForMemberConfig) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetConfig config( ReplSetConfig::parse(BSON("_id" << "rs0" @@ -938,11 +933,6 @@ TEST(ReplSetConfig, SetNewlyAddedFieldForMemberConfig) { } TEST(ReplSetConfig, RemoveNewlyAddedFieldForMemberConfig) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetConfig config( ReplSetConfig::parse(BSON("_id" << "rs0" @@ -986,11 +976,6 @@ TEST(ReplSetConfig, RemoveNewlyAddedFieldForMemberConfig) { } TEST(ReplSetConfig, ParsingNewlyAddedSetsFieldToTrueCorrectly) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetConfig config( ReplSetConfig::parse(BSON("_id" << "rs0" @@ -1004,11 +989,6 @@ TEST(ReplSetConfig, ParsingNewlyAddedSetsFieldToTrueCorrectly) { } TEST(ReplSetConfig, ParseFailsWithNewlyAddedSetToFalse) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetConfig config; ASSERT_THROWS(ReplSetConfig::parse(BSON("_id" << "rs0" @@ -1020,11 +1000,6 @@ TEST(ReplSetConfig, ParseFailsWithNewlyAddedSetToFalse) { } TEST(ReplSetConfig, NodeWithNewlyAddedFieldHasVotesZero) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - // Create a config for a three-node set with one arbiter and one node with 'newlyAdded: true'. ReplSetConfig config( ReplSetConfig::parse(BSON("_id" @@ -1052,11 +1027,6 @@ TEST(ReplSetConfig, NodeWithNewlyAddedFieldHasVotesZero) { } TEST(ReplSetConfig, ToBSONWithoutNewlyAdded) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - // Create a config for a three-node set with one arbiter and one node with 'newlyAdded: true'. ReplSetConfig config( ReplSetConfig::parse(BSON("_id" @@ -1383,11 +1353,6 @@ TEST(ReplSetConfig, toBSONRoundTripAbilityWithHorizon) { } TEST(ReplSetConfig, toBSONRoundTripAbilityLarge) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - ReplSetConfig configA; ReplSetConfig configB; configA = ReplSetConfig::parse(BSON( diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h index 0a8547d5d98..f997fb16771 100644 --- a/src/mongo/db/repl/replication_coordinator.h +++ b/src/mongo/db/repl/replication_coordinator.h @@ -634,8 +634,12 @@ public: * * If commitmentStatus is true, adds a boolean 'commitmentStatus' field to 'result' indicating * whether the current config is committed. + * + * If includeNewlyAdded is true, does not omit 'newlyAdded' fields from the config. */ - virtual void processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus = false) = 0; + virtual void processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus = false, + bool includeNewlyAdded = false) = 0; /** * Processes the ReplSetMetadata returned from a command run against another diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 85f26da6e00..027a75ba474 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -3074,9 +3074,14 @@ WriteConcernOptions ReplicationCoordinatorImpl::_getConfigReplicationWriteConcer } void ReplicationCoordinatorImpl::processReplSetGetConfig(BSONObjBuilder* result, - bool commitmentStatus) { + bool commitmentStatus, + bool includeNewlyAdded) { stdx::lock_guard lock(_mutex); - result->append("config", _rsConfig.toBSONWithoutNewlyAdded()); + if (includeNewlyAdded) { + result->append("config", _rsConfig.toBSON()); + } else { + result->append("config", _rsConfig.toBSONWithoutNewlyAdded()); + } if (commitmentStatus) { uassert(ErrorCodes::NotMaster, diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h index 354304e2a5f..71b485b8379 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.h +++ b/src/mongo/db/repl/replication_coordinator_impl.h @@ -228,7 +228,8 @@ public: virtual ReplSetConfig getConfig() const override; virtual void processReplSetGetConfig(BSONObjBuilder* result, - bool commitmentStatus = false) override; + bool commitmentStatus = false, + bool includeNewlyAdded = false) override; virtual void processReplSetMetadata(const rpc::ReplSetMetadata& replMetadata) override; diff --git a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp index 6fc714b3304..419b28b86f4 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp @@ -998,8 +998,10 @@ TEST_F(ReplCoordReconfigTest, // Start up as a secondary. init(); assertStartSuccess( - configWithMembers( - 1, 0, BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1") << member(3, "n3:1"))), + configWithMembers(1, + 0, + BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1") << member(3, "n3:1") + << member(4, "n4:1", 0))), HostAndPort("n1", 1)); ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY)); @@ -1136,14 +1138,14 @@ TEST_F(ReplCoordReconfigTest, WaitForConfigCommitmentTimesOutIfConfigIsNotCommit // Start out in a non-initial config version. init(); auto configVersion = 2; - auto Ca_members = BSON_ARRAY(member(1, "n1:1")); + auto Ca_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1", 0)); auto Cb_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1")); // Startup, simulate application of one oplog entry and get elected. assertStartSuccess(configWithMembers(configVersion, 0, Ca_members), HostAndPort("n1", 1)); + ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY)); replCoordSetMyLastAppliedAndDurableOpTime(OpTime(Timestamp(1, 1), 0)); - const auto opCtx = makeOperationContext(); - runSingleNodeElection(opCtx.get()); + simulateSuccessfulV1Election(); ASSERT_EQ(getReplCoord()->getMemberState(), MemberState::RS_PRIMARY); ASSERT_EQ(getReplCoord()->getTerm(), 1); @@ -1154,6 +1156,7 @@ TEST_F(ReplCoordReconfigTest, WaitForConfigCommitmentTimesOutIfConfigIsNotCommit respondToAllHeartbeats(); // Do a first reconfig that should succeed since the current config is committed. + const auto opCtx = makeOperationContext(); Status status(ErrorCodes::InternalError, "Not Set"); configVersion = 3; ASSERT_OK(doSafeReconfig(opCtx.get(), configVersion, Cb_members, 1 /* quorumHbs */)); @@ -1175,14 +1178,14 @@ TEST_F(ReplCoordReconfigTest, WaitForConfigCommitmentReturnsOKIfConfigIsCommitte // Start out in a non-initial config version. init(); auto configVersion = 2; - auto Ca_members = BSON_ARRAY(member(1, "n1:1")); + auto Ca_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1", 0)); auto Cb_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1")); // Startup, simulate application of one oplog entry and get elected. assertStartSuccess(configWithMembers(configVersion, 0, Ca_members), HostAndPort("n1", 1)); + ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY)); replCoordSetMyLastAppliedAndDurableOpTime(OpTime(Timestamp(1, 1), 0)); - const auto opCtx = makeOperationContext(); - runSingleNodeElection(opCtx.get()); + simulateSuccessfulV1Election(); ASSERT_EQ(getReplCoord()->getMemberState(), MemberState::RS_PRIMARY); ASSERT_EQ(getReplCoord()->getTerm(), 1); @@ -1193,6 +1196,7 @@ TEST_F(ReplCoordReconfigTest, WaitForConfigCommitmentReturnsOKIfConfigIsCommitte respondToAllHeartbeats(); // Do a first reconfig that should succeed since the current config is committed. + const auto opCtx = makeOperationContext(); configVersion = 3; ASSERT_OK(doSafeReconfig(opCtx.get(), configVersion, Cb_members, 1 /* quorumHbs */)); @@ -1206,18 +1210,19 @@ TEST_F(ReplCoordReconfigTest, // Start out in a non-initial config version. init(); auto configVersion = 2; - auto Ca_members = BSON_ARRAY(member(1, "n1:1")); + auto Ca_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1", 0)); auto Cb_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1")); // Startup, simulate application of one oplog entry and get elected. assertStartSuccess(configWithMembers(configVersion, 0, Ca_members), HostAndPort("n1", 1)); + ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY)); replCoordSetMyLastAppliedAndDurableOpTime(OpTime(Timestamp(1, 1), 0)); - const auto opCtx = makeOperationContext(); - runSingleNodeElection(opCtx.get()); + simulateSuccessfulV1Election(); ASSERT_EQ(getReplCoord()->getMemberState(), MemberState::RS_PRIMARY); ASSERT_EQ(getReplCoord()->getTerm(), 1); // Write and commit one new oplog entry, and consume any heartbeats. + const auto opCtx = makeOperationContext(); auto commitPoint = OpTime(Timestamp(2, 1), 1); replCoordSetMyLastAppliedAndDurableOpTime(commitPoint); ASSERT_EQ(getReplCoord()->getLastCommittedOpTime(), commitPoint); @@ -1242,7 +1247,8 @@ TEST_F(ReplCoordReconfigTest, // Start out in a non-initial config version. init(); auto configVersion = 2; - auto Ca_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1") << member(3, "n3:1")); + auto Ca_members = BSON_ARRAY(member(1, "n1:1") + << member(2, "n2:1") << member(3, "n3:1") << member(4, "n4:1", 0)); auto Cb_members = BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1") << member(3, "n3:1") << member(4, "n4:1")); @@ -1402,9 +1408,11 @@ TEST_F(ReplCoordReconfigTest, StepdownShouldInterruptConfigWrite) { // Start out in a non-initial config version. init(); auto configVersion = 2; - assertStartSuccess( - configWithMembers(configVersion, 0, BSON_ARRAY(member(1, "n1:1") << member(2, "n2:1"))), - HostAndPort("n1", 1)); + assertStartSuccess(configWithMembers(configVersion, + 0, + BSON_ARRAY(member(1, "n1:1") + << member(2, "n2:1") << member(3, "n3:1", 0))), + HostAndPort("n1", 1)); ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY)); // Simulate application of one oplog entry. @@ -1490,11 +1498,6 @@ TEST_F(ReplCoordReconfigTest, StartElectionOnReconfigToSingleNode) { } TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsTrueForNewMembersInReconfig) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1520,11 +1523,6 @@ TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsTrueForNewMembersInReconfig) { } TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithVotesZero) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1554,11 +1552,6 @@ TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithVotesZero) } TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithModifiedHostName) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1583,11 +1576,6 @@ TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithModifiedHos } TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithDifferentIndexButSameID) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1611,11 +1599,6 @@ TEST_F(ReplCoordReconfigTest, NewlyAddedFieldIsNotPresentForNodesWithDifferentIn } TEST_F(ReplCoordReconfigTest, ForceReconfigDoesNotPersistNewlyAddedFieldFromOldNodes) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1674,11 +1657,6 @@ TEST_F(ReplCoordReconfigTest, ForceReconfigDoesNotPersistNewlyAddedFieldFromOldN } TEST_F(ReplCoordReconfigTest, ForceReconfigDoesNotAppendNewlyAddedFieldToNewNodes) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1705,11 +1683,6 @@ TEST_F(ReplCoordReconfigTest, ForceReconfigDoesNotAppendNewlyAddedFieldToNewNode } TEST_F(ReplCoordReconfigTest, ForceReconfigFailsWhenNewlyAddedFieldIsSetToTrue) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1730,11 +1703,6 @@ TEST_F(ReplCoordReconfigTest, ForceReconfigFailsWhenNewlyAddedFieldIsSetToTrue) } TEST_F(ReplCoordReconfigTest, ForceReconfigFailsWhenNewlyAddedFieldSetToFalse) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1755,11 +1723,6 @@ TEST_F(ReplCoordReconfigTest, ForceReconfigFailsWhenNewlyAddedFieldSetToFalse) { } TEST_F(ReplCoordReconfigTest, ParseFailedIfUserProvidesNewlyAddedFieldDuringSafeReconfig) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1789,11 +1752,6 @@ TEST_F(ReplCoordReconfigTest, ParseFailedIfUserProvidesNewlyAddedFieldDuringSafe } TEST_F(ReplCoordReconfigTest, ReconfigNeverModifiesExistingNewlyAddedFieldForMember) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1840,11 +1798,6 @@ TEST_F(ReplCoordReconfigTest, ReconfigNeverModifiesExistingNewlyAddedFieldForMem } TEST_F(ReplCoordReconfigTest, ReconfigNeverModifiesExistingNewlyAddedFieldForPreviouslyAddedNodes) { - // Set the flag to add the 'newlyAdded' field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1895,11 +1848,6 @@ TEST_F(ReplCoordReconfigTest, ReconfigNeverModifiesExistingNewlyAddedFieldForPre } TEST_F(ReplCoordReconfigTest, NodesWithNewlyAddedFieldSetAreTreatedAsVotesZero) { - // Set the flag to add the `newlyAdded` field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1928,11 +1876,6 @@ TEST_F(ReplCoordReconfigTest, NodesWithNewlyAddedFieldSetAreTreatedAsVotesZero) } TEST_F(ReplCoordReconfigTest, NodesWithNewlyAddedFieldSetHavePriorityZero) { - // Set the flag to add the `newlyAdded` field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -1997,11 +1940,6 @@ TEST_F(ReplCoordReconfigTest, NodesWithNewlyAddedFieldSetHavePriorityZero) { } TEST_F(ReplCoordReconfigTest, ArbiterNodesShouldNeverHaveNewlyAddedField) { - // Set the flag to add the `newlyAdded` field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); @@ -2031,11 +1969,6 @@ TEST_F(ReplCoordReconfigTest, ArbiterNodesShouldNeverHaveNewlyAddedField) { } TEST_F(ReplCoordReconfigTest, ForceReconfigShouldThrowIfArbiterNodesHaveNewlyAddedField) { - // Set the flag to add the `newlyAdded` field to MemberConfigs. - enableAutomaticReconfig = true; - // Set the flag back to false after this test exits. - ON_BLOCK_EXIT([] { enableAutomaticReconfig = false; }); - setUpNewlyAddedFieldTest(); auto opCtx = makeOperationContext(); diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp index a531cb41da2..be452167de0 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.cpp +++ b/src/mongo/db/repl/replication_coordinator_mock.cpp @@ -378,7 +378,8 @@ void ReplicationCoordinatorMock::setGetConfigReturnValue(ReplSetConfig returnVal } void ReplicationCoordinatorMock::processReplSetGetConfig(BSONObjBuilder* result, - bool commitmentStatus) { + bool commitmentStatus, + bool includeNewlyAdded) { // TODO } diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h index cafb302372a..ebd848e62d8 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.h +++ b/src/mongo/db/repl/replication_coordinator_mock.h @@ -195,7 +195,9 @@ public: virtual ReplSetConfig getConfig() const; - virtual void processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus = false); + virtual void processReplSetGetConfig(BSONObjBuilder* result, + bool commitmentStatus = false, + bool includeNewlyAdded = false); virtual void processReplSetMetadata(const rpc::ReplSetMetadata& replMetadata) override; diff --git a/src/mongo/db/repl/replication_coordinator_noop.cpp b/src/mongo/db/repl/replication_coordinator_noop.cpp index a85689d66e3..5d703311b4a 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.cpp +++ b/src/mongo/db/repl/replication_coordinator_noop.cpp @@ -299,7 +299,8 @@ ReplSetConfig ReplicationCoordinatorNoOp::getConfig() const { } void ReplicationCoordinatorNoOp::processReplSetGetConfig(BSONObjBuilder* result, - bool commitmentStatus) { + bool commitmentStatus, + bool includeNewlyAdded) { MONGO_UNREACHABLE; } diff --git a/src/mongo/db/repl/replication_coordinator_noop.h b/src/mongo/db/repl/replication_coordinator_noop.h index cef32c185e2..fb3a5ec64d2 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.h +++ b/src/mongo/db/repl/replication_coordinator_noop.h @@ -173,7 +173,9 @@ public: ReplSetConfig getConfig() const final; - void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false) final; + void processReplSetGetConfig(BSONObjBuilder*, + bool commitmentStatus = false, + bool includeNewlyAdded = false) final; void processReplSetMetadata(const rpc::ReplSetMetadata&) final; diff --git a/src/mongo/db/repl/replication_coordinator_test_fixture.h b/src/mongo/db/repl/replication_coordinator_test_fixture.h index e74e89bc6ee..a29fecdaf70 100644 --- a/src/mongo/db/repl/replication_coordinator_test_fixture.h +++ b/src/mongo/db/repl/replication_coordinator_test_fixture.h @@ -79,8 +79,8 @@ public: /** * Helpers to construct a config. */ - static BSONObj member(int id, std::string host) { - return BSON("_id" << id << "host" << host); + static BSONObj member(int id, std::string host, int votes = 1) { + return BSON("_id" << id << "host" << host << "votes" << votes << "priority" << votes); } static BSONObj configWithMembers(int version, long long term, BSONArray members) { diff --git a/src/mongo/embedded/replication_coordinator_embedded.cpp b/src/mongo/embedded/replication_coordinator_embedded.cpp index 9d08d1519de..fd12817508c 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.cpp +++ b/src/mongo/embedded/replication_coordinator_embedded.cpp @@ -324,7 +324,8 @@ ReplSetConfig ReplicationCoordinatorEmbedded::getConfig() const { } void ReplicationCoordinatorEmbedded::processReplSetGetConfig(BSONObjBuilder*, - bool commitmentStatus) { + bool commitmentStatus, + bool includeNewlyAdded) { UASSERT_NOT_IMPLEMENTED; } diff --git a/src/mongo/embedded/replication_coordinator_embedded.h b/src/mongo/embedded/replication_coordinator_embedded.h index 4201903d3a0..e7a2de3b9a8 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.h +++ b/src/mongo/embedded/replication_coordinator_embedded.h @@ -179,7 +179,9 @@ public: repl::ReplSetConfig getConfig() const override; - void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false) override; + void processReplSetGetConfig(BSONObjBuilder*, + bool commitmentStatus = false, + bool includeNewlyAdded = false) override; void processReplSetMetadata(const rpc::ReplSetMetadata&) override; diff --git a/src/mongo/shell/assert.js b/src/mongo/shell/assert.js index f50ba594a66..b9ba9fdc7d2 100644 --- a/src/mongo/shell/assert.js +++ b/src/mongo/shell/assert.js @@ -419,7 +419,7 @@ assert = (function() { // Used up all attempts msg = _buildAssertionMessage(msg); if (runHangAnalyzer) { - msg = msg + "The hang analyzer is automatically called in assert.retry functions. " + + msg = msg + " The hang analyzer is automatically called in assert.retry functions. " + "If you are *expecting* assert.soon to possibly fail, call assert.retry " + "with {runHangAnalyzer: false} as the fifth argument " + "(you can fill unused arguments with `undefined`)."; @@ -481,7 +481,8 @@ assert = (function() { "assert.time failed timeout " + timeout + "ms took " + diff + "ms : " + f + ", msg"; msg = _buildAssertionMessage(msg, msgPrefix); if (runHangAnalyzer) { - msg = msg + "The hang analyzer is automatically called in assert.time functions. " + + msg = msg + + " The hang analyzer is automatically called in assert.time functions. " + "If you are *expecting* assert.soon to possibly fail, call assert.time " + "with {runHangAnalyzer: false} as the fourth argument " + "(you can fill unused arguments with `undefined`)."; diff --git a/src/mongo/shell/replsettest.js b/src/mongo/shell/replsettest.js index 7fa584c6008..d9f2aa0b650 100644 --- a/src/mongo/shell/replsettest.js +++ b/src/mongo/shell/replsettest.js @@ -1146,7 +1146,7 @@ var ReplSetTest = function(opts) { }; function replSetCommandWithRetry(master, cmd) { - printjson(cmd); + print("Running command with retry: " + tojson(cmd)); const cmdName = Object.keys(cmd)[0]; const errorMsg = `${cmdName} during initiate failed`; assert.retry(() => { @@ -1155,13 +1155,99 @@ var ReplSetTest = function(opts) { [ ErrorCodes.NodeNotFound, ErrorCodes.NewReplicaSetConfigurationIncompatible, - ErrorCodes.InterruptedDueToReplStateChange + ErrorCodes.InterruptedDueToReplStateChange, + ErrorCodes.ConfigurationInProgress, + ErrorCodes.CurrentConfigNotCommittedYet ], errorMsg); return result.ok; }, errorMsg, 3, 5 * 1000); } + /** + * Wait until the config on the primary becomes committed. Callers specify the primary in case + * this must be called when two nodes are expected to be concurrently primary. + */ + this.waitForConfigReplication = function(primary, nodes) { + const nodeHosts = nodes ? tojson(nodes.map((n) => n.host)) : "all nodes"; + print("waitForConfigReplication: Waiting for the config on " + primary.host + + " to replicate to " + nodeHosts); + + let configVersion = -2; + let configTerm = -2; + assert.soon(function() { + const res = assert.commandWorked(primary.adminCommand({replSetGetStatus: 1})); + const primaryMember = res.members.find((m) => m.self); + configVersion = primaryMember.configVersion; + configTerm = primaryMember.configTerm; + function hasSameConfig(member) { + return member.configVersion === primaryMember.configVersion && + member.configTerm === primaryMember.configTerm; + } + let members = res.members; + if (nodes) { + members = res.members.filter((m) => nodes.some((node) => m.name === node.host)); + } + return members.every((m) => hasSameConfig(m)); + }); + + print("waitForConfigReplication: config on " + primary.host + + " replicated successfully to " + nodeHosts + " with version " + configVersion + + " and term " + configTerm); + }; + + /** + * Waits for all 'newlyAdded' fields to be removed, for that config to be committed, and for + * the in-memory and on-disk configs to match. + */ + this.waitForAllNewlyAddedRemovals = function(timeout) { + timeout = timeout || self.kDefaultTimeoutMS; + print("waitForAllNewlyAddedRemovals: starting for set " + self.name); + const primary = self.getPrimary(); + + // Shadow 'db' so that we can call the function on the primary without a separate shell when + // x509 auth is not needed. + let db = primary.getDB('admin'); + runFnWithAuthOnPrimary(function() { + assert.soon(function() { + const getConfigRes = assert.commandWorked(db.adminCommand({ + replSetGetConfig: 1, + commitmentStatus: true, + $_internalIncludeNewlyAdded: true + })); + const config = getConfigRes.config; + for (let i = 0; i < config.members.length; i++) { + const memberConfig = config.members[i]; + if (memberConfig.hasOwnProperty("newlyAdded")) { + assert(memberConfig["newlyAdded"] === true, config); + print("waitForAllNewlyAddedRemovals: Retrying because memberIndex " + i + + " is still 'newlyAdded'"); + return false; + } + } + if (!getConfigRes.hasOwnProperty("commitmentStatus")) { + print( + "waitForAllNewlyAddedRemovals: Skipping wait due to no commitmentStatus." + + " Assuming this is an older version."); + return true; + } + + if (!getConfigRes.commitmentStatus) { + print("waitForAllNewlyAddedRemovals: " + + "Retrying because primary's config isn't committed. " + + "Version: " + config.version + ", Term: " + config.term); + return false; + } + + return true; + }); + }, "waitForAllNewlyAddedRemovals"); + + self.waitForConfigReplication(primary); + + print("waitForAllNewlyAddedRemovals: finished for set " + this.name); + }; + /** * Runs replSetInitiate on the first node of the replica set. * Ensures that a primary is elected (not necessarily node 0). @@ -1171,7 +1257,8 @@ var ReplSetTest = function(opts) { */ this.initiateWithAnyNodeAsPrimary = function(cfg, initCmd, { doNotWaitForStableRecoveryTimestamp: doNotWaitForStableRecoveryTimestamp = false, - doNotWaitForReplication: doNotWaitForReplication = false + doNotWaitForReplication: doNotWaitForReplication = false, + doNotWaitForNewlyAddedRemovals: doNotWaitForNewlyAddedRemovals = false } = {}) { let startTime = new Date(); // Measure the execution time of this function. var master = this.nodes[0].getDB("admin"); @@ -1357,11 +1444,30 @@ var ReplSetTest = function(opts) { print("Reconfiguring replica set to add in other nodes"); for (let i = 2; i <= originalMembers.length; i++) { print("ReplSetTest adding in node " + i); - config.members = originalMembers.slice(0, i); - // Set a maxTimeMS so reconfig fails if it times out. - replSetCommandWithRetry( - master, {replSetReconfig: config, maxTimeMS: ReplSetTest.kDefaultTimeoutMS}); - config.version++; + assert.soon(function() { + const statusRes = + assert.commandWorked(master.adminCommand({replSetGetStatus: 1})); + const primaryMember = statusRes.members.find((m) => m.self); + config.version = primaryMember.configVersion + 1; + + config.members = originalMembers.slice(0, i); + cmd = {replSetReconfig: config, maxTimeMS: ReplSetTest.kDefaultTimeoutMS}; + print("Running reconfig command: " + tojsononeline(cmd)); + const reconfigRes = master.adminCommand(cmd); + const retryableReconfigCodes = [ + ErrorCodes.NodeNotFound, + ErrorCodes.NewReplicaSetConfigurationIncompatible, + ErrorCodes.InterruptedDueToReplStateChange, + ErrorCodes.ConfigurationInProgress, + ErrorCodes.CurrentConfigNotCommittedYet + ]; + if (retryableReconfigCodes.includes(reconfigRes.code)) { + print("Retrying reconfig due to " + tojsononeline(reconfigRes)); + return false; + } + assert.commandWorked(reconfigRes); + return true; + }, "reconfig for fixture set up failed", ReplSetTest.kDefaultTimeoutMS, 1000); } } @@ -1375,6 +1481,13 @@ var ReplSetTest = function(opts) { // detect initial sync completion more quickly. this.awaitSecondaryNodes( self.kDefaultTimeoutMS, null /* slaves */, 25 /* retryIntervalMS */); + + // If test commands are not enabled, we cannot wait for 'newlyAdded' removals. Tests that + // disable test commands must ensure 'newlyAdded' removals mid-test are acceptable. + if (!doNotWaitForNewlyAddedRemovals && jsTest.options().enableTestCommands) { + self.waitForAllNewlyAddedRemovals(); + } + print("ReplSetTest initiate reconfig and awaitSecondaryNodes took " + (new Date() - reconfigStart) + "ms for " + this.nodes.length + " nodes in set '" + this.name + "'"); @@ -1633,6 +1746,41 @@ var ReplSetTest = function(opts) { return masterOpTime; }; + // TODO(SERVER-14017): Remove this extra sub-shell in favor of a cleaner authentication + // solution. + function runFnWithAuthOnPrimary(fn, fnName) { + const primary = self.getPrimary(); + const primaryId = "n" + self.getNodeId(primary); + const primaryOptions = self.nodeOptions[primaryId] || {}; + const options = + (primaryOptions === {} || !self.startOptions) ? primaryOptions : self.startOptions; + const authMode = options.clusterAuthMode; + if (authMode === "x509") { + print(fnName + ": authenticating on separate shell with x509 for " + self.name); + const caFile = options.sslCAFile ? options.sslCAFile : options.tlsCAFile; + const keyFile = + options.sslPEMKeyFile ? options.sslPEMKeyFile : options.tlsCertificateKeyFile; + const subShellArgs = [ + 'mongo', + '--ssl', + '--sslCAFile=' + caFile, + '--sslPEMKeyFile=' + keyFile, + '--sslAllowInvalidHostnames', + '--authenticationDatabase=$external', + '--authenticationMechanism=MONGODB-X509', + primary.host, + '--eval', + `(${fn.toString()})();` + ]; + + const retVal = _runMongoProgram(...subShellArgs); + assert.eq(retVal, 0, 'mongo shell did not succeed with exit code 0'); + } else { + print(fnName + ": authenticating with authMode '" + authMode + "' for " + self.name); + asCluster(primary, fn, primaryOptions.keyFile); + } + } + /** * This function performs some writes and then waits for all nodes in this replica set to * establish a stable recovery timestamp. The writes are necessary to prompt storage engines to @@ -1654,7 +1802,8 @@ var ReplSetTest = function(opts) { // propagate to all members and trigger a stable checkpoint on all persisted storage engines // nodes. function advanceCommitPoint(master) { - // Shadow 'db' so that we can call 'advanceCommitPoint' directly on the primary node. + // Shadow 'db' so that we can call the function on the primary without a separate shell + // when x509 auth is not needed. let db = master.getDB('admin'); const appendOplogNoteFn = function() { assert.commandWorked(db.adminCommand({ @@ -1664,35 +1813,7 @@ var ReplSetTest = function(opts) { })); }; - // TODO(SERVER-14017): Remove this extra sub-shell in favor of a cleaner authentication - // solution. - const masterId = "n" + rst.getNodeId(master); - const masterOptions = rst.nodeOptions[masterId] || {}; - if (masterOptions.clusterAuthMode === "x509") { - print("AwaitLastStableRecoveryTimestamp: authenticating on separate shell " + - "with x509 for " + id); - const subShellArgs = [ - 'mongo', - '--ssl', - '--sslCAFile=' + masterOptions.sslCAFile, - '--sslPEMKeyFile=' + masterOptions.sslPEMKeyFile, - '--sslAllowInvalidHostnames', - '--authenticationDatabase=$external', - '--authenticationMechanism=MONGODB-X509', - master.host, - '--eval', - `(${appendOplogNoteFn.toString()})();` - ]; - - const retVal = _runMongoProgram(...subShellArgs); - assert.eq(retVal, 0, 'mongo shell did not succeed with exit code 0'); - } else { - if (masterOptions.clusterAuthMode) { - print("AwaitLastStableRecoveryTimestamp: authenticating with " + - masterOptions.clusterAuthMode + " for " + id); - } - asCluster(master, appendOplogNoteFn, masterOptions.keyFile); - } + runFnWithAuthOnPrimary(appendOplogNoteFn, "AwaitLastStableRecoveryTimestamp"); } print("AwaitLastStableRecoveryTimestamp: Beginning for " + id); -- cgit v1.2.1