diff options
author | Frederic Vitzikam <frederic.vitzikam@mongodb.com> | 2022-02-03 23:37:50 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-02-04 00:30:30 +0000 |
commit | 780455ded3bbffeccadab9ab88ebe8f44a6d64f2 (patch) | |
tree | 2b74134b1ee08e0d1e4dce774c21c815bfbc1d2e | |
parent | f65c3ec8d02162327c80f11fbb1758096a75cbe8 (diff) | |
download | mongo-780455ded3bbffeccadab9ab88ebe8f44a6d64f2.tar.gz |
SERVER-60696 Add a server parameter to disallow multiple arbiters
-rw-r--r-- | jstests/replsets/arbiters_not_included_in_w2_wc.js | 6 | ||||
-rw-r--r-- | jstests/replsets/disable_multiple_arbiters.js | 55 | ||||
-rw-r--r-- | jstests/replsets/libs/rollback_test_deluxe.js | 2 | ||||
-rw-r--r-- | jstests/replsets/read_committed_after_rollback.js | 4 | ||||
-rw-r--r-- | jstests/replsets/read_majority_two_arbs.js | 4 | ||||
-rw-r--r-- | jstests/replsets/rollback_all_op_types.js | 2 | ||||
-rw-r--r-- | jstests/replsets/rollback_collmods.js | 2 | ||||
-rw-r--r-- | jstests/replsets/rollback_with_socket_error_then_steady_state.js | 17 | ||||
-rw-r--r-- | jstests/replsets/zero_vote_arbiter.js | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_server_parameters.idl | 14 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_config.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_config_checks_test.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/repl/repl_set_config_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp | 48 | ||||
-rw-r--r-- | src/mongo/db/repl/topology_coordinator_v1_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/shell/servers.js | 1 |
16 files changed, 159 insertions, 14 deletions
diff --git a/jstests/replsets/arbiters_not_included_in_w2_wc.js b/jstests/replsets/arbiters_not_included_in_w2_wc.js index ca391185e30..b0555add966 100644 --- a/jstests/replsets/arbiters_not_included_in_w2_wc.js +++ b/jstests/replsets/arbiters_not_included_in_w2_wc.js @@ -7,6 +7,8 @@ * A w:2 write would thus require only one additional member to fully satisfy the write concern * after committing. This test therefore shuts down the both secondaries and verifies that neither * of the arbiters gets picked in its place and the w:2 write times out instead. + * + * @tags: [requires_fcv_53] */ (function() { @@ -16,7 +18,7 @@ const name = "arbiters_not_included_in_w2_wc"; const rst = new ReplSetTest({name: name, nodes: 5}); const nodes = rst.nodeList(); -rst.startSet(); +rst.startSet({setParameter: {allowMultipleArbiters: true}}); rst.initiate({ "_id": name, "members": [ @@ -49,4 +51,4 @@ assert.commandFailedWithCode(testColl.insert({"b": 2}, {writeConcern: {w: 2, wti ErrorCodes.WriteConcernFailed); rst.stopSet(); -})();
\ No newline at end of file +})(); diff --git a/jstests/replsets/disable_multiple_arbiters.js b/jstests/replsets/disable_multiple_arbiters.js new file mode 100644 index 00000000000..aa57a04beba --- /dev/null +++ b/jstests/replsets/disable_multiple_arbiters.js @@ -0,0 +1,55 @@ +/** + * Verifies that multiple arbiters cannot be added without setting allowMultipleArbiters=true. + * + * @tags: [requires_fcv_53] + */ + +function multiple_arbiters(multiple_arbiters_allowed) { + "use strict"; + + jsTestLog("multiple_arbiters(" + multiple_arbiters_allowed + ")"); + + const name = "disable_multiple_arbiters"; + const rst = new ReplSetTest({name: name, nodes: 4}); + const nodes = rst.nodeList(); + + let config = {}; + if (multiple_arbiters_allowed) { + config["setParameter"] = {allowMultipleArbiters: true}; + } + rst.startSet(config); + rst.initiate({ + "_id": name, + "members": [ + {"_id": 0, "host": nodes[0]}, + {"_id": 1, "host": nodes[1]}, + {"_id": 2, "host": nodes[2]}, + {"_id": 3, "host": nodes[3], "arbiterOnly": true} + ] + }); + + const arbiterConn = rst.add(config); + const admin = rst.getPrimary().getDB("admin"); + const conf = admin.runCommand({replSetGetConfig: 1}).config; + conf.members.push({_id: 4, host: arbiterConn.host, arbiterOnly: true}); + conf.version++; + + jsTestLog('Add second arbiter'); + const response = admin.runCommand({replSetReconfig: conf}); + + if (multiple_arbiters_allowed) { + assert.commandWorked(response); + } else { + assert.commandFailedWithCode(response, ErrorCodes.NewReplicaSetConfigurationIncompatible); + } + + if (!multiple_arbiters_allowed) { + // Remove the node since it was not successfully added to the config, so we should not run + // validation checks on it when we shut down the replica set. + rst.remove(4); + } + rst.stopSet(); +} + +multiple_arbiters(true); +multiple_arbiters(false); diff --git a/jstests/replsets/libs/rollback_test_deluxe.js b/jstests/replsets/libs/rollback_test_deluxe.js index 2c9382e9908..858226ef662 100644 --- a/jstests/replsets/libs/rollback_test_deluxe.js +++ b/jstests/replsets/libs/rollback_test_deluxe.js @@ -152,7 +152,7 @@ function RollbackTestDeluxe(name = "FiveNodeDoubleRollbackTest", replSet) { } let replSet = new ReplSetTest({name, nodes: 5, useBridge: true, nodeOptions: nodeOptions}); - replSet.startSet(); + replSet.startSet({setParameter: {allowMultipleArbiters: true}}); const nodes = replSet.nodeList(); replSet.initiate({ diff --git a/jstests/replsets/read_committed_after_rollback.js b/jstests/replsets/read_committed_after_rollback.js index bca0bf2289b..239ee284536 100644 --- a/jstests/replsets/read_committed_after_rollback.js +++ b/jstests/replsets/read_committed_after_rollback.js @@ -3,7 +3,7 @@ * snapshots be dropped during rollback, therefore committed reads will block until a new committed * snapshot is available. * - * @tags: [requires_majority_read_concern] + * @tags: [requires_majority_read_concern, requires_fcv_53] */ (function() { @@ -27,7 +27,7 @@ function doDirtyRead(coll) { var name = "read_committed_after_rollback"; var replTest = new ReplSetTest( {name: name, nodes: 5, useBridge: true, nodeOptions: {enableMajorityReadConcern: ''}}); -replTest.startSet(); +replTest.startSet({setParameter: {allowMultipleArbiters: true}}); var nodes = replTest.nodeList(); var config = { diff --git a/jstests/replsets/read_majority_two_arbs.js b/jstests/replsets/read_majority_two_arbs.js index 1d79f4f9b95..77a7f3018e9 100644 --- a/jstests/replsets/read_majority_two_arbs.js +++ b/jstests/replsets/read_majority_two_arbs.js @@ -2,7 +2,7 @@ * Tests that writeConcern 'majority' writes succeed and are visible in a replica set that has one * data-bearing node and two arbiters. * - * @tags: [requires_majority_read_concern] + * @tags: [requires_majority_read_concern, requires_fcv_53] */ (function() { "use strict"; @@ -16,7 +16,7 @@ var name = "read_majority_two_arbs"; var replTest = new ReplSetTest({name: name, nodes: 3, nodeOptions: {enableMajorityReadConcern: ''}}); -replTest.startSet(); +replTest.startSet({setParameter: {allowMultipleArbiters: true}}); var nodes = replTest.nodeList(); var config = { "_id": name, diff --git a/jstests/replsets/rollback_all_op_types.js b/jstests/replsets/rollback_all_op_types.js index 5eddbe528e4..a2ae13b7d9a 100644 --- a/jstests/replsets/rollback_all_op_types.js +++ b/jstests/replsets/rollback_all_op_types.js @@ -7,6 +7,8 @@ * will mock many system components, and sometimes will mock behaviors that don't necessarily match * true system behavior i.e. mocking an oplog entry with an incorrect format. So, this integration * test provides an additional verification of rollback's correctness within a real replica set. + * + * @tags: [requires_fcv_53] */ (function() { diff --git a/jstests/replsets/rollback_collmods.js b/jstests/replsets/rollback_collmods.js index 7c96235f33c..75c9af51d96 100644 --- a/jstests/replsets/rollback_collmods.js +++ b/jstests/replsets/rollback_collmods.js @@ -1,6 +1,8 @@ /** * Tests that collMod commands during every stage of rollback are tracked correctly. * This especially targets collection validators that begin partially or fully uninitialized. + * + * @tags: [requires_fcv_53] */ (function() { diff --git a/jstests/replsets/rollback_with_socket_error_then_steady_state.js b/jstests/replsets/rollback_with_socket_error_then_steady_state.js index 86ad6ef094a..b8a4f327d5e 100644 --- a/jstests/replsets/rollback_with_socket_error_then_steady_state.js +++ b/jstests/replsets/rollback_with_socket_error_then_steady_state.js @@ -1,8 +1,13 @@ -// This test causes node 2 to enter rollback and then fail with a SocketException before updating -// MinValid or altering durable state in any way. It will then choose a sync source from which it -// is able to stitch the oplog and therefore doesn't need to roll back. Prior to SERVER-27282, the -// node would be "stuck" with state=ROLLBACK while it was doing steady-state replication, with no -// way to reach SECONDARY without restarting the process. +/** + * This test causes node 2 to enter rollback and then fail with a SocketException before updating + * MinValid or altering durable state in any way. It will then choose a sync source from which it + * is able to stitch the oplog and therefore doesn't need to roll back. Prior to SERVER-27282, the + * node would be "stuck" with state=ROLLBACK while it was doing steady-state replication, with no + * way to reach SECONDARY without restarting the process. + * + * @tags: [requires_fcv_53] + */ + (function() { 'use strict'; @@ -26,7 +31,7 @@ var rst = new ReplSetTest({ ], useBridge: true }); -var nodes = rst.startSet(); +var nodes = rst.startSet({setParameter: {allowMultipleArbiters: true}}); rst.initiate(); // The default WC is majority and stopServerReplication could prevent satisfying any majority diff --git a/jstests/replsets/zero_vote_arbiter.js b/jstests/replsets/zero_vote_arbiter.js index 750cec6ffac..c1020760fbb 100644 --- a/jstests/replsets/zero_vote_arbiter.js +++ b/jstests/replsets/zero_vote_arbiter.js @@ -39,7 +39,7 @@ TestData.skipCheckDBHashes = true; */ (function reconfigArbiterZeroVotes() { var replTest = new ReplSetTest({nodes: 4}); - replTest.startSet(); + replTest.startSet({setParameter: {allowMultipleArbiters: true}}); var config = replTest.getReplSetConfig(); config.members[2].arbiterOnly = true; config.members[3].arbiterOnly = true; diff --git a/src/mongo/db/repl/repl_server_parameters.idl b/src/mongo/db/repl/repl_server_parameters.idl index e5881777e75..3f60c41728d 100644 --- a/src/mongo/db/repl/repl_server_parameters.idl +++ b/src/mongo/db/repl/repl_server_parameters.idl @@ -617,6 +617,20 @@ server_parameters: validator: gte: 0 + allowMultipleArbiters: + description: >- + Allow multiple arbiters. Default is false as it can put data at risk by allowing a + replica set to accept writes without a sufficient number of secondaries being available + for data replication. + For example, a PSSAA replica set (primary, 2 secondaries, 2 arbiters) would still be + available for writes after the two secondaries fail. + In that case, only one copy of the data, on the primary, would be actively updated. + The replica set would have a majority of nodes available for election purposes, + but no active replication until at least one healthy secondary is available. + set_at: startup + cpp_vartype: bool + cpp_varname: allowMultipleArbiters + default: false feature_flags: featureFlagRetryableFindAndModify: diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp index 9ed67568c77..86368756c7f 100644 --- a/src/mongo/db/repl/repl_set_config.cpp +++ b/src/mongo/db/repl/repl_set_config.cpp @@ -418,6 +418,12 @@ Status ReplSetConfig::_validate(bool allowSplitHorizonIP) const { "their config"); } + if (!allowMultipleArbiters && arbiterCount > 1) { + return Status(ErrorCodes::BadValue, + "Multiple arbiters are not allowed unless all nodes were started with " + "with --setParameter 'allowMultipleArbiters=true'"); + } + if (!_connectionString.isValid()) { return Status(ErrorCodes::BadValue, "ReplSetConfig represented an invalid replica set ConnectionString"); 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 03f9965053c..21cb645cf10 100644 --- a/src/mongo/db/repl/repl_set_config_checks_test.cpp +++ b/src/mongo/db/repl/repl_set_config_checks_test.cpp @@ -40,6 +40,7 @@ #include "mongo/db/server_options.h" #include "mongo/db/service_context.h" #include "mongo/db/service_context_test_fixture.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/unittest/death_test.h" #include "mongo/unittest/ensure_fcv.h" #include "mongo/unittest/unittest.h" @@ -1075,6 +1076,7 @@ TEST_F(ServiceContextTest, ValidateForReconfig_SimultaneousAddAndRemoveOfArbiter } TEST_F(ServiceContextTest, ValidateForReconfig_MultiNodeAdditionOfArbitersDisallowed) { + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; BSONArray oldMembers = BSON_ARRAY(m1 << m2); BSONArray newMembers = BSON_ARRAY(m1 << m2 << m3_Arbiter << m4_Arbiter); // add two arbiters. ASSERT_EQUALS(ErrorCodes::InvalidReplicaSetConfig, diff --git a/src/mongo/db/repl/repl_set_config_test.cpp b/src/mongo/db/repl/repl_set_config_test.cpp index f93aa2e962a..285de453107 100644 --- a/src/mongo/db/repl/repl_set_config_test.cpp +++ b/src/mongo/db/repl/repl_set_config_test.cpp @@ -37,6 +37,7 @@ #include "mongo/db/repl/repl_set_config.h" #include "mongo/db/server_options.h" #include "mongo/db/serverless/shard_split_utils.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/unittest/unittest.h" #include "mongo/util/scopeguard.h" @@ -189,6 +190,7 @@ TEST(ReplSetConfig, MajorityCalculationThreeVotersNoArbiters) { } TEST(ReplSetConfig, MajorityCalculationNearlyHalfArbiters) { + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; ReplSetConfig config( ReplSetConfig::parse(BSON("_id" << "mySet" @@ -1987,6 +1989,8 @@ TEST(ReplSetConfig, ConfigVersionAndTermToString) { ASSERT_EQ(ConfigVersionAndTerm(1, -1).toString(), "{version: 1, term: -1}"); } TEST(ReplSetConfig, IsImplicitDefaultWriteConcernMajority) { + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; + ReplSetConfig config(ReplSetConfig::parse(createConfigDocWithArbiters(1, 0))); ASSERT_OK(config.validate()); ASSERT(config.isImplicitDefaultWriteConcernMajority()); 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 781c10f0df0..7d7ca3ec455 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_reconfig_test.cpp @@ -42,6 +42,7 @@ #include "mongo/db/repl/replication_coordinator_impl.h" #include "mongo/db/repl/replication_coordinator_test_fixture.h" #include "mongo/executor/network_interface_mock.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/logv2/log.h" #include "mongo/stdx/future.h" #include "mongo/unittest/log_test.h" @@ -162,6 +163,7 @@ TEST_F(ReplCoordTest, NodeReturnsNotPrimaryErrorWhenReconfigCmdReceivedWhileInDr TEST_F(ReplCoordTest, NodeReturnsInvalidReplicaSetConfigWhenReconfigReceivedWithInvalidConfig) { // start up, become primary, receive uninitializable config + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; assertStartSuccess(BSON("_id" << "mySet" << "version" << 2 << "members" @@ -1002,6 +1004,7 @@ TEST_F(ReplCoordTest, ReconfigThatChangesIDWCW1ToWMajWithCWWCSetPasses) { } TEST_F(ReplCoordTest, ReconfigThatKeepsIDWCAtW1WithoutCWWCSetPasses) { + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; assertStartSuccess(BSON("_id" << "mySet" << "version" << 2 << "members" @@ -1194,6 +1197,42 @@ public: respondToAllHeartbeats(); } + void multipleArbiterTest(bool allowMultipleArbiters) { + LOGV2(60696, "multipleArbiterTest", "allowMultipleArbiters"_attr = allowMultipleArbiters); + init(); + auto configVersion = 2; + assertStartSuccess(configWithMembers(configVersion, + 0, + BSON_ARRAY(member(1, "n1:1") + << member(2, "n2:1") + << BSON("_id" << 3 << "host" + << "n3:1" + << "arbiterOnly" << true))), + HostAndPort("n1", 1)); + + auto opCtx = makeOperationContext(); + BSONObjBuilder result; + ReplSetReconfigArgs args; + args.force = true; + args.newConfigObj = configWithMembers(2, + 0, + BSON_ARRAY(member(1, "n1:1") + << member(2, "n2:1") + << BSON("_id" << 3 << "host" + << "n3:1" + << "arbiterOnly" << true) + << BSON("_id" << 4 << "host" + << "n4:1" + << "arbiterOnly" << true))); + if (allowMultipleArbiters) { + ASSERT_EQUALS(ErrorCodes::OK, + getReplCoord()->processReplSetReconfig(opCtx.get(), args, &result)); + } else { + ASSERT_EQUALS(ErrorCodes::NewReplicaSetConfigurationIncompatible, + getReplCoord()->processReplSetReconfig(opCtx.get(), args, &result)); + } + } + void respondToNHeartbeats(int n) { LOGV2(24262, "Responding to {n} heartbeats", "n"_attr = n); enterNetwork(); @@ -2320,6 +2359,15 @@ TEST_F(ReplCoordReconfigTest, ForceReconfigShouldThrowIfArbiterNodesHaveNewlyAdd getReplCoord()->processReplSetReconfig(opCtx.get(), args, &result)); } +TEST_F(ReplCoordReconfigTest, MultipleArbitersShouldFailWithoutServerParameter) { + multipleArbiterTest(false); +} + +TEST_F(ReplCoordReconfigTest, MultipleArbitersShouldSucceedWithServerParameter) { + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; + multipleArbiterTest(true); +} + TEST_F(ReplCoordTest, StepUpReconfigConcurrentWithHeartbeatReconfig) { auto severityGuard = unittest::MinimumLoggedSeverityGuard{logv2::LogComponent::kReplication, logv2::LogSeverity::Debug(2)}; diff --git a/src/mongo/db/repl/topology_coordinator_v1_test.cpp b/src/mongo/db/repl/topology_coordinator_v1_test.cpp index a80c45e6108..c4aa8622a37 100644 --- a/src/mongo/db/repl/topology_coordinator_v1_test.cpp +++ b/src/mongo/db/repl/topology_coordinator_v1_test.cpp @@ -45,6 +45,7 @@ #include "mongo/db/server_options.h" #include "mongo/db/service_context.h" #include "mongo/executor/task_executor.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/logv2/log.h" #include "mongo/rpc/metadata/oplog_query_metadata.h" #include "mongo/rpc/metadata/repl_set_metadata.h" @@ -2025,6 +2026,7 @@ TEST_F(TopoCoordTest, ReplSetGetStatus) { TEST_F(TopoCoordTest, ReplSetGetStatusWriteMajorityDifferentFromMajorityVoteCount) { // This tests that writeMajorityCount differs from majorityVoteCount in replSetGetStatus when // the number of non-arbiter voters is less than majorityVoteCount. + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; Date_t startupTime = Date_t::fromMillisSinceEpoch(100); Date_t heartbeatTime = Date_t::fromMillisSinceEpoch(5000); Seconds uptimeSecs(10); @@ -2067,6 +2069,7 @@ TEST_F(TopoCoordTest, ReplSetGetStatusVotingMembersCountAndWritableVotingMembers // This test verifies that `votingMembersCount` and `writableVotingMembersCount` in // replSetGetStatus are set correctly when arbiters and non-voting nodes are included in the // replica set. + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; updateConfig(BSON("_id" << "mySet" << "version" << 1 << "members" @@ -6321,6 +6324,7 @@ TEST_F(TopoCoordTest, ArbiterNotIncludedInW3WriteInPSSAReplSet) { TEST_F(TopoCoordTest, ArbitersNotIncludedInW2WriteInPSSAAReplSet) { // In a PSSAA set, a w:2 write should only be acknowledged if at least one of the secondaries // can satisfy it. + RAIIServerParameterControllerForTest controller{"allowMultipleArbiters", true}; updateConfig(BSON("_id" << "rs0" << "version" << 2 << "members" diff --git a/src/mongo/shell/servers.js b/src/mongo/shell/servers.js index b40131a6425..737437cff7b 100644 --- a/src/mongo/shell/servers.js +++ b/src/mongo/shell/servers.js @@ -704,6 +704,7 @@ MongoRunner.mongodOptions = function(opts = {}) { _removeSetParameterIfBeforeVersion(opts, "enableReconfigRollbackCommittedWritesCheck", "5.0.0"); _removeSetParameterIfBeforeVersion(opts, "featureFlagRetryableFindAndModify", "5.0.0"); _removeSetParameterIfBeforeVersion(opts, "internalQueryForceClassicEngine", "5.1.0"); + _removeSetParameterIfBeforeVersion(opts, "allowMultipleArbiters", "5.3.0"); if (!opts.logFile && opts.useLogFiles) { opts.logFile = opts.dbpath + "/mongod.log"; |