SERVER-60696 Add a server parameter to disallow multiple arbiters

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";