SERVER-65128: Improve integration testing and address bugs in setClusterParameter

13 files changed, 302 insertions, 108 deletions

diff --git a/jstests/libs/cluster_server_parameter_utils.js b/jstests/libs/cluster_server_parameter_utils.js
index 8800ee45af7..76a78003176 100644
--- a/jstests/libs/cluster_server_parameter_utils.js
+++ b/jstests/libs/cluster_server_parameter_utils.js
@@ -15,12 +15,10 @@ const clusterParameterNames = ["testStrClusterParameter", "testIntClusterParamet
 const clusterParametersDefault = [
     {
         _id: "testStrClusterParameter",
-        clusterParameterTime: Timestamp(0, 0),
         strData: "off",
     },
     {
         _id: "testIntClusterParameter",
-        clusterParameterTime: Timestamp(0, 0),
         intData: 16,
     }
 ];
@@ -28,12 +26,10 @@ const clusterParametersDefault = [
 const clusterParametersInsert = [
     {
         _id: "testStrClusterParameter",
-        clusterParameterTime: Timestamp(10, 2),
         strData: "on",
     },
     {
         _id: "testIntClusterParameter",
-        clusterParameterTime: Timestamp(10, 2),
         intData: 17,
     }
 ];
@@ -41,12 +37,10 @@ const clusterParametersInsert = [
 const clusterParametersUpdate = [
     {
         _id: "testStrClusterParameter",
-        clusterParameterTime: Timestamp(20, 4),
         strData: "sleep",
     },
     {
         _id: "testIntClusterParameter",
-        clusterParameterTime: Timestamp(20, 4),
         intData: 18,
     }
 ];
@@ -74,27 +68,22 @@ function setupSharded(st) {
     });
 }
 
-// TO-DO SERVER-65128: replace this function with a call to setClusterParameter.
-// Upserts config.clusterParameters document with w:majority.
-function simulateSetClusterParameterReplicaSet(rst, query, update) {
-    const clusterParametersNS = rst.getPrimary().getDB('config').clusterParameters;
-    assert.commandWorked(
-        clusterParametersNS.update(query, update, {upsert: true, writeConcern: {w: "majority"}}));
-}
-
-// TO-DO SERVER-65128: replace this function with a call to setClusterParameter.
-// Upserts config.clusterParameters document with w:majority into configsvr and all shards.
-function simulateSetClusterParameterSharded(st, query, update) {
-    simulateSetClusterParameterReplicaSet(st.configRS, query, update);
+// Upserts config.clusterParameters document with w:majority via setClusterParameter.
+function runSetClusterParameter(conn, update) {
+    const paramName = update._id;
+    let updateCopy = Object.assign({}, update);
+    delete updateCopy._id;
+    delete updateCopy.clusterParameterTime;
+    const setClusterParameterDoc = {
+        [paramName]: updateCopy,
+    };
 
-    const shards = [st.rs0, st.rs1, st.rs2];
-    shards.forEach(function(shard) {
-        simulateSetClusterParameterReplicaSet(shard, query, update);
-    });
+    const adminDB = conn.getDB('admin');
+    assert.commandWorked(adminDB.runCommand({setClusterParameter: setClusterParameterDoc}));
 }
 
 // Runs getClusterParameter on a specific mongod or mongos node and returns true/false depending
-// on whether .
+// on whether the expected values were returned.
 function runGetClusterParameterNode(conn, getClusterParameterArgs, expectedClusterParameters) {
     const adminDB = conn.getDB('admin');
     const actualClusterParameters =
@@ -117,10 +106,15 @@ function runGetClusterParameterNode(conn, getClusterParameterArgs, expectedClust
             }, {});
         const sortedActualClusterParameter =
             Object.keys(actualClusterParameter).sort().reduce(function(sorted, key) {
-                sorted[key] = actualClusterParameter[key];
+                if (key !== 'clusterParameterTime') {
+                    sorted[key] = actualClusterParameter[key];
+                }
                 return sorted;
             }, {});
+
         if (bsonWoCompare(sortedExpectedClusterParameter, sortedActualClusterParameter) !== 0) {
+            print('expected: ' + tojson(sortedExpectedClusterParameter) +
+                  '\nactual: ' + tojson(sortedActualClusterParameter));
             return false;
         }
     }
@@ -148,10 +142,13 @@ function runGetClusterParameterReplicaSet(rst, getClusterParameterArgs, expected
     assert((numMatches / numTotalNodes) > 0.5);
 }
 
-// Runs getClusterParameter on mongos and each mongod in each shard replica set.
+// Runs getClusterParameter on mongos, each mongod in each shard replica set, and each mongod in
+// the config server replica set.
 function runGetClusterParameterSharded(st, getClusterParameterArgs, expectedClusterParameters) {
     runGetClusterParameterNode(st.s0, getClusterParameterArgs, expectedClusterParameters);
 
+    runGetClusterParameterReplicaSet(
+        st.configRS, getClusterParameterArgs, expectedClusterParameters);
     const shards = [st.rs0, st.rs1, st.rs2];
     shards.forEach(function(shard) {
         runGetClusterParameterReplicaSet(shard, getClusterParameterArgs, expectedClusterParameters);
@@ -159,26 +156,24 @@ function runGetClusterParameterSharded(st, getClusterParameterArgs, expectedClus
 }
 
 // Tests valid usages of getClusterParameter and verifies that the expected values are returned.
-function testValidParameters(conn) {
+function testValidClusterParameterCommands(conn) {
     if (conn instanceof ReplSetTest) {
         // Run getClusterParameter in list format and '*' and ensure it returns all default values
         // on all nodes in the replica set.
         runGetClusterParameterReplicaSet(conn, clusterParameterNames, clusterParametersDefault);
         runGetClusterParameterReplicaSet(conn, '*', clusterParametersDefault);
 
-        // For each parameter, simulate setClusterParameter and verify that getClusterParameter
+        // For each parameter, run setClusterParameter and verify that getClusterParameter
         // returns the updated value on all nodes in the replica set.
         for (let i = 0; i < clusterParameterNames.length; i++) {
-            query = {_id: clusterParameterNames[i]};
-            simulateSetClusterParameterReplicaSet(conn, query, clusterParametersInsert[i]);
+            runSetClusterParameter(conn.getPrimary(), clusterParametersInsert[i]);
             runGetClusterParameterReplicaSet(
                 conn, clusterParameterNames[i], [clusterParametersInsert[i]]);
         }
 
         // Do the above again to verify that document updates are also handled properly.
         for (let i = 0; i < clusterParameterNames.length; i++) {
-            query = {_id: clusterParameterNames[i]};
-            simulateSetClusterParameterReplicaSet(conn, query, clusterParametersUpdate[i]);
+            runSetClusterParameter(conn.getPrimary(), clusterParametersUpdate[i]);
             runGetClusterParameterReplicaSet(
                 conn, clusterParameterNames[i], [clusterParametersUpdate[i]]);
         }
@@ -196,16 +191,14 @@ function testValidParameters(conn) {
         // For each parameter, simulate setClusterParameter and verify that getClusterParameter
         // returns the updated value on all nodes in the sharded cluster.
         for (let i = 0; i < clusterParameterNames.length; i++) {
-            query = {_id: clusterParameterNames[i]};
-            simulateSetClusterParameterSharded(conn, query, clusterParametersInsert[i]);
+            runSetClusterParameter(conn.s0, clusterParametersInsert[i]);
             runGetClusterParameterSharded(
                 conn, clusterParameterNames[i], [clusterParametersInsert[i]]);
         }
 
         // Do the above again to verify that document updates are also handled properly.
         for (let i = 0; i < clusterParameterNames.length; i++) {
-            query = {_id: clusterParameterNames[i]};
-            simulateSetClusterParameterSharded(conn, query, clusterParametersUpdate[i]);
+            runSetClusterParameter(conn.s0, clusterParametersUpdate[i]);
             runGetClusterParameterSharded(
                 conn, clusterParameterNames[i], [clusterParametersUpdate[i]]);
         }
@@ -217,8 +210,8 @@ function testValidParameters(conn) {
     }
 }
 
-// Tests that invalid uses of getClusterParameter fails
-function testInvalidParametersNode(conn) {
+// Tests that invalid uses of getClusterParameter fails on a given node.
+function testInvalidGetClusterParameter(conn) {
     const adminDB = conn.getDB('admin');
     // Assert that specifying a nonexistent parameter returns an error.
     assert.commandFailedWithCode(adminDB.runCommand({getClusterParameter: "nonexistentParam"}),
@@ -230,20 +223,90 @@ function testInvalidParametersNode(conn) {
         ErrorCodes.NoSuchKey);
 }
 
-// Tests that invalid uses of getClusterParameter fail with the appropriate errors.
-function testInvalidParameters(conn) {
+// Tests that invalid uses of set/getClusterParameter fail with the appropriate errors.
+function testInvalidClusterParameterCommands(conn) {
     if (conn instanceof ReplSetTest) {
-        testInvalidParametersNode(conn.getPrimary());
+        const adminDB = conn.getPrimary().getDB('admin');
+
+        // Assert that invalid uses of getClusterParameter fail on the primary.
+        testInvalidGetClusterParameter(conn.getPrimary());
+
+        // Assert that setting a nonexistent parameter on the primary returns an error.
+        assert.commandFailedWithCode(
+            adminDB.runCommand({setClusterParameter: {nonexistentParam: {intData: 5}}}),
+            ErrorCodes.IllegalOperation);
+
+        // Assert that running setClusterParameter with a scalar value fails.
+        assert.commandFailedWithCode(
+            adminDB.runCommand({setClusterParameter: {testIntClusterParameter: 5}}),
+            ErrorCodes.IllegalOperation);
+
         conn.getSecondaries().forEach(function(secondary) {
-            testInvalidParametersNode(secondary);
+            // Assert that setClusterParameter cannot be run on a secondary.
+            const secondaryAdminDB = secondary.getDB('admin');
+            assert.commandFailedWithCode(
+                secondaryAdminDB.runCommand(
+                    {setClusterParameter: {testIntClusterParameter: {intData: 5}}}),
+                ErrorCodes.NotWritablePrimary);
+            // Assert that invalid uses of getClusterParameter fail on secondaries.
+            testInvalidGetClusterParameter(secondary);
         });
     } else {
-        testInvalidParametersNode(conn.s0);
+        const adminDB = conn.s0.getDB('admin');
+
+        // Assert that invalid uses of getClusterParameter fail on mongos.
+        testInvalidGetClusterParameter(conn.s0);
+
+        // Assert that setting a nonexistent parameter on the mongos returns an error.
+        assert.commandFailedWithCode(
+            adminDB.runCommand({setClusterParameter: {nonexistentParam: {intData: 5}}}),
+            ErrorCodes.IllegalOperation);
+
+        // Assert that running setClusterParameter with a scalar value fails.
+        assert.commandFailedWithCode(
+            adminDB.runCommand({setClusterParameter: {testIntClusterParameter: 5}}),
+            ErrorCodes.IllegalOperation);
+
         const shards = [conn.rs0, conn.rs1, conn.rs2];
         shards.forEach(function(shard) {
+            // Assert that setClusterParameter cannot be run directly on a shard primary.
+            const shardPrimaryAdmin = shard.getPrimary().getDB('admin');
+            assert.commandFailedWithCode(
+                shardPrimaryAdmin.runCommand(
+                    {setClusterParameter: {testIntClusterParameter: {intData: 5}}}),
+                ErrorCodes.NotImplemented);
+            // Assert that invalid forms of getClusterParameter fail on the shard primary.
+            testInvalidGetClusterParameter(shard.getPrimary());
             shard.getSecondaries().forEach(function(secondary) {
-                testInvalidParametersNode(secondary);
+                // Assert that setClusterParameter cannot be run on a shard secondary.
+                const shardSecondaryAdmin = secondary.getDB('admin');
+                assert.commandFailedWithCode(
+                    shardSecondaryAdmin.runCommand(
+                        {setClusterParameter: {testIntClusterParameter: {intData: 5}}}),
+                    ErrorCodes.NotWritablePrimary);
+                // Assert that invalid forms of getClusterParameter fail on shard secondaries.
+                testInvalidGetClusterParameter(secondary);
             });
         });
+
+        // Assert that setClusterParameter cannot be run directly on the configsvr primary.
+        const configRS = conn.configRS;
+        const configPrimaryAdmin = configRS.getPrimary().getDB('admin');
+        assert.commandFailedWithCode(
+            configPrimaryAdmin.runCommand(
+                {setClusterParameter: {testIntClusterParameter: {intData: 5}}}),
+            ErrorCodes.NotImplemented);
+        // Assert that invalid forms of getClusterParameter fail on the configsvr primary.
+        testInvalidGetClusterParameter(configRS.getPrimary());
+        configRS.getSecondaries().forEach(function(secondary) {
+            // Assert that setClusterParameter cannot be run on a configsvr secondary.
+            const configSecondaryAdmin = secondary.getDB('admin');
+            assert.commandFailedWithCode(
+                configSecondaryAdmin.runCommand(
+                    {setClusterParameter: {testIntClusterParameter: {intData: 5}}}),
+                ErrorCodes.NotWritablePrimary);
+            // Assert that invalid forms of getClusterParameter fail on configsvr secondaries.
+            testInvalidGetClusterParameter(secondary);
+        });
     }
 }
diff --git a/jstests/replsets/cluster_server_parameter_commands_replset.js b/jstests/replsets/cluster_server_parameter_commands_replset.js
index 5e296108349..add7533d47c 100644
--- a/jstests/replsets/cluster_server_parameter_commands_replset.js
+++ b/jstests/replsets/cluster_server_parameter_commands_replset.js
@@ -1,5 +1,5 @@
 /**
- * Checks that getClusterParameter runs as expected on replica set nodes.
+ * Checks that set/getClusterParameter runs as expected on replica set nodes.
  *
  * @tags: [
  *   # Requires all nodes to be running the latest binary.
@@ -13,27 +13,22 @@
 
 load('jstests/libs/cluster_server_parameter_utils.js');
 
-// Tests that getClusterParameter works on a non-sharded replica set.
-function runReplSetTest() {
-    const rst = new ReplSetTest({
-        nodes: 3,
-    });
-    rst.startSet();
-    rst.initiate();
+// Tests that set/getClusterParameter works on a non-sharded replica set.
+const rst = new ReplSetTest({
+    nodes: 3,
+});
+rst.startSet();
+rst.initiate();
 
-    // Setup the necessary logging level for the test.
-    setupReplicaSet(rst);
+// Setup the necessary logging level for the test.
+setupReplicaSet(rst);
 
-    // First, ensure that nonexistent parameters and unauthorized users are rejected with the
-    // appropriate error codes.
-    testInvalidParameters(rst);
+// First, ensure that incorrect usages of set/getClusterParameter fail appropriately.
+testInvalidClusterParameterCommands(rst);
 
-    // Then, ensure that getClusterParameter returns the expected values for all valid invocations
-    // of getClusterParameter.
-    testValidParameters(rst);
+// Then, ensure that set/getClusterParameter set and retrieve the expected values on the
+// majority of the nodes in the replica set.
+testValidClusterParameterCommands(rst);
 
-    rst.stopSet();
-}
-
-runReplSetTest();
+rst.stopSet();
 })();
diff --git a/jstests/replsets/set_cluster_parameter_replset.js b/jstests/replsets/set_cluster_parameter_replset.js
new file mode 100644
index 00000000000..2e3cb5440b8
--- /dev/null
+++ b/jstests/replsets/set_cluster_parameter_replset.js
@@ -0,0 +1,101 @@
+/**
+ * Checks that setClusterParameter behaves as expected during initial sync and restart.
+ *
+ * @tags: [
+ *   # Requires all nodes to be running the latest binary.
+ *   requires_fcv_60,
+ *   featureFlagClusterWideConfig,
+ *   does_not_support_stepdowns
+ *  ]
+ */
+(function() {
+'use strict';
+
+load('jstests/libs/cluster_server_parameter_utils.js');
+
+// Checks that up-to-date cluster parameters are transferred over to newly-added replica set nodes
+// as part of initial sync.
+function checkClusterParameterInitialSync(rst) {
+    // Update some parameter values.
+    let newIntParameter = {
+        _id: "testIntClusterParameter",
+        intData: 5,
+    };
+    let newStrParameter = {
+        _id: "testStrClusterParameter",
+        strData: "on",
+    };
+    runSetClusterParameter(rst.getPrimary(), newIntParameter);
+    runSetClusterParameter(rst.getPrimary(), newStrParameter);
+
+    // Check that the new values are visible on the majority of the nodes.
+    runGetClusterParameterReplicaSet(rst,
+                                     ["testIntClusterParameter", "testStrClusterParameter"],
+                                     [newIntParameter, newStrParameter]);
+
+    // Add a new node to the replica set, reinitiate the set, and wait for it to become a secondary
+    // with all data fully replicated to it.
+    const newNode = rst.add({});
+    rst.reInitiate();
+    rst.waitForState(newNode, ReplSetTest.State.SECONDARY);
+    rst.awaitReplication();
+
+    // Check that the new node has the latest cluster parameter values.
+    runGetClusterParameterNode(newNode,
+                               ["testIntClusterParameter", "testStrClusterParameter"],
+                               [newIntParameter, newStrParameter]);
+
+    // Check that setClusterParameter properly works with the reconfigured replica set.
+    newIntParameter.intData = 30;
+    newStrParameter.strData = "sleep";
+    runSetClusterParameter(rst.getPrimary(), newIntParameter);
+    runSetClusterParameter(rst.getPrimary(), newStrParameter);
+
+    // Check that the new values are visible on the majority of the nodes.
+    runGetClusterParameterReplicaSet(rst,
+                                     ["testIntClusterParameter", "testStrClusterParameter"],
+                                     [newIntParameter, newStrParameter]);
+}
+
+// Checks that restarted replica sets start with the most recent majority-written cluster parameter
+// values.
+function checkClusterParameterRestart(rst) {
+    // Update some parameter values.
+    let newIntParameter = {
+        _id: "testIntClusterParameter",
+        intData: 8,
+    };
+    let newStrParameter = {
+        _id: "testStrClusterParameter",
+        strData: "dormant",
+    };
+    runSetClusterParameter(rst.getPrimary(), newIntParameter);
+    runSetClusterParameter(rst.getPrimary(), newStrParameter);
+
+    // Check that the new values are visible on the majority of the nodes.
+    runGetClusterParameterReplicaSet(rst,
+                                     ["testIntClusterParameter", "testStrClusterParameter"],
+                                     [newIntParameter, newStrParameter]);
+
+    // Bounce restart all of the nodes.
+    rst.nodeList().forEach((_, index) => {
+        rst.restart(index);
+    });
+
+    // Check that restarted replica set still has the most recent setClusterParameter values.
+    runGetClusterParameterReplicaSet(rst,
+                                     ["testIntClusterParameter", "testStrClusterParameter"],
+                                     [newIntParameter, newStrParameter]);
+}
+
+const rst = new ReplSetTest({
+    nodes: 2,
+});
+rst.startSet();
+rst.initiate();
+
+checkClusterParameterInitialSync(rst);
+checkClusterParameterRestart(rst);
+
+rst.stopSet();
+})();
diff --git a/jstests/sharding/cluster_server_parameter_commands_sharded.js b/jstests/sharding/cluster_server_parameter_commands_sharded.js
index 9fecae646eb..3431fa0edcd 100644
--- a/jstests/sharding/cluster_server_parameter_commands_sharded.js
+++ b/jstests/sharding/cluster_server_parameter_commands_sharded.js
@@ -1,5 +1,5 @@
 /**
- * Checks that getClusterParameter runs as expected on sharded clusters.
+ * Checks that set/getClusterParameter runs as expected on sharded clusters.
  *
  * @tags: [
  *   # Requires all nodes to be running the latest binary.
@@ -13,31 +13,27 @@
 
 load('jstests/libs/cluster_server_parameter_utils.js');
 
-// Tests that getClusterParameter works on all nodes of a sharded cluster.
-function runShardedTest() {
-    const options = {
-        mongos: 1,
-        config: 1,
-        shards: 3,
-        rs: {
-            nodes: 3,
-        },
-    };
-    const st = new ShardingTest(options);
+// Tests that set/getClusterParameter works on all nodes of a sharded cluster.
+const options = {
+    mongos: 1,
+    config: 1,
+    shards: 3,
+    rs: {
+        nodes: 3,
+    },
+};
+const st = new ShardingTest(options);
 
-    // Setup the necessary logging on mongos and the shards.
-    setupSharded(st);
+// Setup the necessary logging on mongos and the shards.
+setupSharded(st);
 
-    // First, ensure that nonexistent parameters are rejected with the
-    // appropriate error codes on mongos and all shards.
-    testInvalidParameters(st);
+// First, ensure that incorrect usages of set/getClusterParameter fail appropriately on mongos
+// and cluster mongods.
+testInvalidClusterParameterCommands(st);
 
-    // Then, ensure that getClusterParameter returns the expected values for all valid invocations
-    // of getClusterParameter.
-    testValidParameters(st);
+// Then, ensure that set/getClusterParameter set and retrieve the expected values on mongos
+// and the majority of nodes on all replica sets in the cluster.
+testValidClusterParameterCommands(st);
 
-    st.stop();
-}
-
-runShardedTest();
+st.stop();
 })();
diff --git a/src/mongo/db/commands/get_cluster_parameter_command.cpp b/src/mongo/db/commands/get_cluster_parameter_command.cpp
index 76f6f91c590..87c84d18387 100644
--- a/src/mongo/db/commands/get_cluster_parameter_command.cpp
+++ b/src/mongo/db/commands/get_cluster_parameter_command.cpp
@@ -34,6 +34,7 @@
 #include "mongo/db/auth/authorization_session.h"
 #include "mongo/db/commands.h"
 #include "mongo/db/commands/cluster_server_parameter_cmds_gen.h"
+#include "mongo/db/repl/replication_coordinator.h"
 #include "mongo/idl/cluster_server_parameter_gen.h"
 #include "mongo/logv2/log.h"
 
@@ -69,6 +70,13 @@ public:
                 "featureFlagClusterWideConfig not enabled",
                 gFeatureFlagClusterWideConfig.isEnabled(serverGlobalParams.featureCompatibility));
 
+            // TODO SERVER-65249: This will eventually be made specific to the parameter being set
+            // so that some parameters will be able to use getClusterParameter even on standalones.
+            uassert(ErrorCodes::IllegalOperation,
+                    str::stream() << Request::kCommandName << " cannot be run on standalones",
+                    repl::ReplicationCoordinator::get(opCtx)->getReplicationMode() !=
+                        repl::ReplicationCoordinator::modeNone);
+
             const stdx::variant<std::string, std::vector<std::string>>& cmdBody =
                 request().getCommandParameter();
             ServerParameterSet* clusterParameters = ServerParameterSet::getClusterParameterSet();
@@ -78,7 +86,6 @@ public:
             // For each parameter, generate a BSON representation of it and retrieve its name.
             auto makeBSON = [&](ServerParameter* requestedParameter) {
                 BSONObjBuilder bob;
-                bob.append("_id"_sd, requestedParameter->name());
                 requestedParameter->append(opCtx, bob, requestedParameter->name());
                 parameterValues.push_back(bob.obj());
                 parameterNames.push_back(requestedParameter->name());
diff --git a/src/mongo/db/commands/set_cluster_parameter_command.cpp b/src/mongo/db/commands/set_cluster_parameter_command.cpp
index c4472f1ae12..ce752e33266 100644
--- a/src/mongo/db/commands/set_cluster_parameter_command.cpp
+++ b/src/mongo/db/commands/set_cluster_parameter_command.cpp
@@ -35,6 +35,7 @@
 #include "mongo/db/commands.h"
 #include "mongo/db/commands/cluster_server_parameter_cmds_gen.h"
 #include "mongo/db/commands/set_cluster_parameter_invocation.h"
+#include "mongo/db/repl/replication_coordinator.h"
 #include "mongo/idl/cluster_server_parameter_gen.h"
 #include "mongo/logv2/log.h"
 
@@ -67,11 +68,21 @@ public:
         using InvocationBase::InvocationBase;
 
         void typedRun(OperationContext* opCtx) {
-
             uassert(ErrorCodes::IllegalOperation,
                     "Cannot set cluster parameter, gFeatureFlagClusterWideConfig is not enabled",
                     gFeatureFlagClusterWideConfig.isEnabledAndIgnoreFCV());
 
+            uassert(ErrorCodes::ErrorCodes::NotImplemented,
+                    "setClusterParameter can only run on mongos in sharded clusters",
+                    (serverGlobalParams.clusterRole == ClusterRole::None));
+
+            // TODO SERVER-65249: This will eventually be made specific to the parameter being set
+            // so that some parameters will be able to use setClusterParameter even on standalones.
+            uassert(ErrorCodes::IllegalOperation,
+                    str::stream() << Request::kCommandName << " cannot be run on standalones",
+                    repl::ReplicationCoordinator::get(opCtx)->getReplicationMode() !=
+                        repl::ReplicationCoordinator::modeNone);
+
             std::unique_ptr<ServerParameterService> parameterService =
                 std::make_unique<ClusterParameterService>();
 
diff --git a/src/mongo/db/commands/set_cluster_parameter_invocation.cpp b/src/mongo/db/commands/set_cluster_parameter_invocation.cpp
index 8d29147d282..b021b800e12 100644
--- a/src/mongo/db/commands/set_cluster_parameter_invocation.cpp
+++ b/src/mongo/db/commands/set_cluster_parameter_invocation.cpp
@@ -61,11 +61,10 @@ bool SetClusterParameterInvocation::invoke(OperationContext* opCtx,
             "Cluster parameter value must be an object",
             BSONType::Object == commandElement.type());
 
-    LogicalTime clusterTime =
-        paramTime ? LogicalTime(*paramTime) : _dbService.getUpdateClusterTime(opCtx);
+    Timestamp clusterTime = paramTime ? *paramTime : _dbService.getUpdateClusterTime(opCtx);
 
     BSONObjBuilder updateBuilder;
-    updateBuilder << "_id" << parameterName << "clusterParameterTime" << clusterTime.toBSON();
+    updateBuilder << "_id" << parameterName << "clusterParameterTime" << clusterTime;
     updateBuilder.appendElements(commandElement.Obj());
 
     BSONObj query = BSON("_id" << parameterName);
@@ -78,9 +77,9 @@ bool SetClusterParameterInvocation::invoke(OperationContext* opCtx,
     return uassertStatusOK(_dbService.updateParameterOnDisk(opCtx, query, update, writeConcern));
 }
 
-LogicalTime ClusterParameterDBClientService::getUpdateClusterTime(OperationContext* opCtx) {
+Timestamp ClusterParameterDBClientService::getUpdateClusterTime(OperationContext* opCtx) {
     VectorClock::VectorTime vt = VectorClock::get(opCtx)->getTime();
-    return vt.clusterTime();
+    return vt.clusterTime().asTimestamp();
 }
 
 StatusWith<bool> ClusterParameterDBClientService::updateParameterOnDisk(
@@ -94,6 +93,9 @@ StatusWith<bool> ClusterParameterDBClientService::updateParameterOnDisk(
     set.append("$set", update);
     set.doneFast();
 
+    const auto writeConcernObj =
+        BSON(WriteConcernOptions::kWriteConcernField << writeConcern.toBSON());
+
     try {
         _dbClient.runCommand(
             NamespaceString::kConfigDb.toString(),
@@ -109,7 +111,7 @@ StatusWith<bool> ClusterParameterDBClientService::updateParameterOnDisk(
                     return entry;
                 }()});
 
-                return updateOp.toBSON(writeConcern.toBSON());
+                return updateOp.toBSON(writeConcernObj);
             }(),
             res);
     } catch (const DBException& ex) {
diff --git a/src/mongo/db/commands/set_cluster_parameter_invocation.h b/src/mongo/db/commands/set_cluster_parameter_invocation.h
index d6ff09f0d47..aeb9cc1fde7 100644
--- a/src/mongo/db/commands/set_cluster_parameter_invocation.h
+++ b/src/mongo/db/commands/set_cluster_parameter_invocation.h
@@ -52,7 +52,7 @@ public:
                                                    BSONObj query,
                                                    BSONObj update,
                                                    const WriteConcernOptions&) = 0;
-    virtual LogicalTime getUpdateClusterTime(OperationContext*) = 0;
+    virtual Timestamp getUpdateClusterTime(OperationContext*) = 0;
     virtual ~DBClientService() = default;
 };
 
@@ -63,7 +63,7 @@ public:
                                            BSONObj query,
                                            BSONObj update,
                                            const WriteConcernOptions&) override;
-    LogicalTime getUpdateClusterTime(OperationContext*) override;
+    Timestamp getUpdateClusterTime(OperationContext*) override;
 
 private:
     DBDirectClient& _dbClient;
diff --git a/src/mongo/db/commands/set_cluster_parameter_invocation_test.cpp b/src/mongo/db/commands/set_cluster_parameter_invocation_test.cpp
index 895b094d47d..e1e641c019e 100644
--- a/src/mongo/db/commands/set_cluster_parameter_invocation_test.cpp
+++ b/src/mongo/db/commands/set_cluster_parameter_invocation_test.cpp
@@ -104,9 +104,9 @@ public:
         return updateParameterOnDiskMockImpl(cmd, info);
     }
 
-    LogicalTime getUpdateClusterTime(OperationContext*) override {
+    Timestamp getUpdateClusterTime(OperationContext*) override {
         LogicalTime lt;
-        return lt;
+        return lt.asTimestamp();
     }
 
 private:
diff --git a/src/mongo/idl/server_parameter_specialized_test.cpp b/src/mongo/idl/server_parameter_specialized_test.cpp
index 611af89335f..36a28fc7bf5 100644
--- a/src/mongo/idl/server_parameter_specialized_test.cpp
+++ b/src/mongo/idl/server_parameter_specialized_test.cpp
@@ -433,9 +433,8 @@ TEST(SpecializedServerParameter, withValidate) {
 void SpecializedClusterServerParameter::append(OperationContext*,
                                                BSONObjBuilder& builder,
                                                const std::string& name) {
-    BSONObjBuilder subObjBuilder = builder.subobjStart(name);
-    _data.serialize(&subObjBuilder);
-    subObjBuilder.done();
+    builder.append("_id"_sd, name);
+    builder.appendElementsUnique(_data.toBSON());
 }
 
 Status SpecializedClusterServerParameter::set(const BSONElement& newValueElement) {
@@ -488,6 +487,7 @@ TEST(SpecializedServerParameter, clusterServerParameter) {
     data.setClusterParameterTime(updateTime);
     data.setIntData(50);
     data.setStrData("hello");
+    data.setId(kSpecializedCSPName);
     data.serialize(&builder);
     ASSERT_OK(specializedCsp->set(builder.asTempObj()));
 
@@ -500,8 +500,9 @@ TEST(SpecializedServerParameter, clusterServerParameter) {
     // Assert that the parameter can be appended to a builder.
     builder.resetToEmpty();
     specializedCsp->append(nullptr, builder, kSpecializedCSPName.toString());
-    auto obj = builder.asTempObj()["specializedCluster"_sd].Obj();
-    ASSERT_EQ(obj.nFields(), 3);
+    auto obj = builder.asTempObj();
+    ASSERT_EQ(obj.nFields(), 4);
+    ASSERT_EQ(obj["_id"_sd].String(), kSpecializedCSPName);
     ASSERT_EQ(obj["clusterParameterTime"_sd].timestamp(), updateTime.asTimestamp());
     ASSERT_EQ(obj["strData"_sd].String(), "hello");
     ASSERT_EQ(obj["intData"_sd].Int(), 50);
@@ -520,8 +521,9 @@ TEST(SpecializedServerParameter, clusterServerParameter) {
     builder.resetToEmpty();
     ASSERT_OK(specializedCsp->reset());
     specializedCsp->append(nullptr, builder, kSpecializedCSPName.toString());
-    obj = builder.asTempObj()["specializedCluster"_sd].Obj();
-    ASSERT_EQ(obj.nFields(), 3);
+    obj = builder.asTempObj();
+    ASSERT_EQ(obj.nFields(), 4);
+    ASSERT_EQ(obj["_id"_sd].String(), kSpecializedCSPName);
     ASSERT_EQ(obj["clusterParameterTime"_sd].timestamp(), LogicalTime().asTimestamp());
     ASSERT_EQ(obj["strData"_sd].String(), "default");
     ASSERT_EQ(obj["intData"_sd].Int(), 30);
diff --git a/src/mongo/idl/server_parameter_specialized_test.h b/src/mongo/idl/server_parameter_specialized_test.h
index 00047c14e90..d21d36175e1 100644
--- a/src/mongo/idl/server_parameter_specialized_test.h
+++ b/src/mongo/idl/server_parameter_specialized_test.h
@@ -82,6 +82,11 @@ public:
         _intData = intData;
     }
 
+    void setId(StringData id) {
+        stdx::lock_guard<Latch> lg(_mutex);
+        _id = id.toString();
+    }
+
     void parse(const BSONObj& updatedObj) {
         stdx::lock_guard<Latch> lg(_mutex);
         _clusterParameterTime = LogicalTime(updatedObj["clusterParameterTime"].timestamp());
@@ -91,19 +96,30 @@ public:
 
     const void serialize(BSONObjBuilder* builder) const {
         stdx::lock_guard<Latch> lg(_mutex);
+        if (_id.is_initialized()) {
+            builder->append("_id"_sd, _id.get());
+        }
         builder->append("clusterParameterTime"_sd, _clusterParameterTime.asTimestamp());
         builder->append("strData"_sd, _strData);
         builder->append("intData"_sd, _intData);
     }
 
+    BSONObj toBSON() const {
+        BSONObjBuilder builder;
+        serialize(&builder);
+        return builder.obj();
+    }
+
     void reset() {
         stdx::lock_guard<Latch> lg(_mutex);
         _clusterParameterTime = LogicalTime();
         _strData = "default";
         _intData = 30;
+        _id = boost::none;
     }
 
 private:
+    boost::optional<std::string> _id;
     LogicalTime _clusterParameterTime;
     std::string _strData;
     std::int32_t _intData;
diff --git a/src/mongo/idl/server_parameter_with_storage.h b/src/mongo/idl/server_parameter_with_storage.h
index e116dee1078..c90280ed60c 100644
--- a/src/mongo/idl/server_parameter_with_storage.h
+++ b/src/mongo/idl/server_parameter_with_storage.h
@@ -350,7 +350,8 @@ public:
         if (isRedact()) {
             b.append(name, "###");
         } else if constexpr (paramType == SPT::kClusterWide) {
-            getValue().serialize(&b);
+            b.append("_id"_sd, name);
+            b.appendElementsUnique(getValue().toBSON());
         } else {
             b.append(name, getValue());
         }
diff --git a/src/mongo/s/commands/cluster_get_cluster_parameter_cmd.cpp b/src/mongo/s/commands/cluster_get_cluster_parameter_cmd.cpp
index 7f5022ab989..71bda72dfaf 100644
--- a/src/mongo/s/commands/cluster_get_cluster_parameter_cmd.cpp
+++ b/src/mongo/s/commands/cluster_get_cluster_parameter_cmd.cpp
@@ -147,6 +147,7 @@ public:
                 defaultParameterNames.reserve(requestedParameterNames.size() -
                                               onDiskParameterNames.size());
                 std::sort(onDiskParameterNames.begin(), onDiskParameterNames.end());
+                std::sort(requestedParameterNames.begin(), requestedParameterNames.end());
                 std::set_difference(requestedParameterNames.begin(),
                                     requestedParameterNames.end(),
                                     onDiskParameterNames.begin(),
@@ -156,7 +157,6 @@ public:
                 for (const auto& defaultParameterName : defaultParameterNames) {
                     auto defaultParameter = clusterParameters->get(defaultParameterName);
                     BSONObjBuilder bob;
-                    bob.append("_id"_sd, defaultParameterName);
                     defaultParameter->append(opCtx, bob, defaultParameterName);
                     retrievedParameters.push_back(bob.obj());
                 }