SERVER-68340 Fix flaky unit test again

7 files changed, 165 insertions, 6 deletions

diff --git a/src/mongo/db/s/SConscript b/src/mongo/db/s/SConscript
index 96589f2da9a..01b60cf14eb 100644
--- a/src/mongo/db/s/SConscript
+++ b/src/mongo/db/s/SConscript
@@ -747,6 +747,7 @@ env.CppUnitTest(
         'balancer/migration_test_fixture.cpp',
         'balancer/type_migration_test.cpp',
         'config_server_op_observer_test.cpp',
+        'config/configsvr_coordinator_service_test.cpp',
         'config/index_on_config_test.cpp',
         'config/initial_split_policy_test.cpp',
         'config/sharding_catalog_manager_add_shard_test.cpp',
@@ -780,6 +781,7 @@ env.CppUnitTest(
         '$BUILD_DIR/mongo/db/pipeline/document_source_mock',
         '$BUILD_DIR/mongo/db/read_write_concern_defaults_mock',
         '$BUILD_DIR/mongo/db/repl/primary_only_service',
+        '$BUILD_DIR/mongo/db/repl/primary_only_service_test_fixture',
         '$BUILD_DIR/mongo/db/repl/replication_info',
         '$BUILD_DIR/mongo/db/repl/wait_for_majority_service',
         '$BUILD_DIR/mongo/db/session/session_catalog_mongod',
@@ -788,6 +790,7 @@ env.CppUnitTest(
         '$BUILD_DIR/mongo/db/transaction/transaction_api',
         '$BUILD_DIR/mongo/util/version_impl',
         'config_server_test_fixture',
+        'sharding_commands_d',
     ],
 )
 
diff --git a/src/mongo/db/s/config/configsvr_coordinator.cpp b/src/mongo/db/s/config/configsvr_coordinator.cpp
index 0cbfecadac1..ce778da3e7f 100644
--- a/src/mongo/db/s/config/configsvr_coordinator.cpp
+++ b/src/mongo/db/s/config/configsvr_coordinator.cpp
@@ -42,6 +42,7 @@
 namespace mongo {
 
 MONGO_FAIL_POINT_DEFINE(hangBeforeRunningConfigsvrCoordinatorInstance);
+MONGO_FAIL_POINT_DEFINE(hangAndEndBeforeRunningConfigsvrCoordinatorInstance);
 
 namespace {
 
@@ -101,6 +102,11 @@ void ConfigsvrCoordinator::interrupt(Status status) noexcept {
 
 SemiFuture<void> ConfigsvrCoordinator::run(std::shared_ptr<executor::ScopedTaskExecutor> executor,
                                            const CancellationToken& token) noexcept {
+    if (hangAndEndBeforeRunningConfigsvrCoordinatorInstance.shouldFail()) {
+        hangAndEndBeforeRunningConfigsvrCoordinatorInstance.pauseWhileSet();
+        _completionPromise.emplaceValue();
+        return Status::OK();
+    }
     return ExecutorFuture<void>(**executor)
         .then([this, executor, token, anchor = shared_from_this()] {
             hangBeforeRunningConfigsvrCoordinatorInstance.pauseWhileSet();
diff --git a/src/mongo/db/s/config/configsvr_coordinator.idl b/src/mongo/db/s/config/configsvr_coordinator.idl
index 647fd8f7ad4..460994098b7 100644
--- a/src/mongo/db/s/config/configsvr_coordinator.idl
+++ b/src/mongo/db/s/config/configsvr_coordinator.idl
@@ -51,6 +51,10 @@ structs:
             coordinatorType:
                 description: "Type of the ConfigsvrCoordinator"
                 type: ConfigsvrCoordinatorType
+            subId:
+                description: "Identifier to distinguish from other instances of same type"
+                type: string
+                optional: true
 
     ConfigsvrCoordinatorSession:
         description: "Container for configsvr coordinator session used for guaranteeing remote
diff --git a/src/mongo/db/s/config/configsvr_coordinator_service_test.cpp b/src/mongo/db/s/config/configsvr_coordinator_service_test.cpp
new file mode 100644
index 00000000000..30c0fc56b1c
--- /dev/null
+++ b/src/mongo/db/s/config/configsvr_coordinator_service_test.cpp
@@ -0,0 +1,132 @@
+/**
+ *    Copyright (C) 2022-present MongoDB, Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the Server Side Public License, version 1,
+ *    as published by MongoDB, Inc.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    Server Side Public License for more details.
+ *
+ *    You should have received a copy of the Server Side Public License
+ *    along with this program. If not, see
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the Server Side Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#include "mongo/platform/basic.h"
+
+#include "mongo/db/repl/primary_only_service_test_fixture.h"
+#include "mongo/db/repl/storage_interface_mock.h"
+#include "mongo/db/s/config/configsvr_coordinator.h"
+#include "mongo/db/s/config/configsvr_coordinator_service.h"
+#include "mongo/db/s/config/set_cluster_parameter_coordinator_document_gen.h"
+#include "mongo/db/s/config/set_user_write_block_mode_coordinator_document_gen.h"
+#include "mongo/unittest/unittest.h"
+
+
+namespace mongo {
+namespace {
+
+class ConfigsvrCoordinatorServiceTest : public repl::PrimaryOnlyServiceMongoDTest {
+
+public:
+    std::unique_ptr<repl::PrimaryOnlyService> makeService(ServiceContext* serviceContext) override {
+        return std::make_unique<ConfigsvrCoordinatorService>(serviceContext);
+    }
+
+    void setUp() override {
+        repl::PrimaryOnlyServiceMongoDTest::setUp();
+
+        auto serviceContext = getServiceContext();
+        auto storageMock = std::make_unique<repl::StorageInterfaceMock>();
+        repl::StorageInterface::set(serviceContext, std::move(storageMock));
+    }
+
+    void tearDown() override {
+        _service->shutdown();
+        repl::PrimaryOnlyServiceMongoDTest::tearDown();
+    }
+};
+
+TEST_F(ConfigsvrCoordinatorServiceTest, CoordinatorsOfSameTypeCanExist) {
+    auto opCtx = cc().makeOperationContext();
+
+    auto* service = dynamic_cast<ConfigsvrCoordinatorService*>(_service);
+
+    std::vector<std::shared_ptr<ConfigsvrCoordinator>> instances;
+    {
+        // Ensure that the new coordinators we create won't actually run.
+        FailPointEnableBlock fp("hangAndEndBeforeRunningConfigsvrCoordinatorInstance");
+
+        SetClusterParameterCoordinatorDocument coordinatorDoc;
+        ConfigsvrCoordinatorId cid(ConfigsvrCoordinatorTypeEnum::kSetClusterParameter);
+        cid.setSubId("0"_sd);
+        coordinatorDoc.setConfigsvrCoordinatorMetadata({cid});
+        coordinatorDoc.setParameter(BSON("a" << 1));
+
+        SetClusterParameterCoordinatorDocument coordinatorDocSameSubId;
+        coordinatorDocSameSubId.setConfigsvrCoordinatorMetadata({cid});
+        coordinatorDocSameSubId.setParameter(BSON("b" << 2));
+
+        SetClusterParameterCoordinatorDocument coordinatorDocDiffSubId;
+        ConfigsvrCoordinatorId cid1(ConfigsvrCoordinatorTypeEnum::kSetClusterParameter);
+        cid1.setSubId("1"_sd);
+        coordinatorDocDiffSubId.setConfigsvrCoordinatorMetadata({cid1});
+        coordinatorDocDiffSubId.setParameter(BSON("a" << 1));
+
+        SetUserWriteBlockModeCoordinatorDocument coordinatorDocDiffType;
+        ConfigsvrCoordinatorId cid2(ConfigsvrCoordinatorTypeEnum::kSetUserWriteBlockMode);
+        cid2.setSubId("0"_sd);
+        coordinatorDocDiffType.setConfigsvrCoordinatorMetadata({cid2});
+        coordinatorDocDiffType.setBlock(true);
+
+        // Trying to create a second coordinator with exact same fields will just get current
+        // coordinator.
+        auto coord1 = service->getOrCreateService(opCtx.get(), coordinatorDoc.toBSON());
+        auto coord1_copy = service->getOrCreateService(opCtx.get(), coordinatorDoc.toBSON());
+        ASSERT(coord1);
+        // Note that this is pointer equality, so there is only one real instance.
+        ASSERT_EQUALS(coord1, coord1_copy);
+
+        // Trying to create a second coordinator with same type and subId but different fields will
+        // fail due to conflict.
+        ASSERT_THROWS(service->getOrCreateService(opCtx.get(), coordinatorDocSameSubId.toBSON()),
+                      AssertionException);
+
+        // We can create a second coordinator of the same type but different subId.
+        auto coord2 = service->getOrCreateService(opCtx.get(), coordinatorDocDiffSubId.toBSON());
+        ASSERT(coord2);
+        ASSERT_NOT_EQUALS(coord1, coord2);
+
+        // We can create a coordinator with different type and same (or different) subId.
+        auto coord3 = service->getOrCreateService(opCtx.get(), coordinatorDocDiffType.toBSON());
+        ASSERT(coord3);
+        ASSERT_NOT_EQUALS(coord1, coord3);
+        ASSERT_NOT_EQUALS(coord2, coord3);
+
+        // Ensure all instances start before we disable the failpoint.
+        fp->waitForTimesEntered(fp.initialTimesEntered() + 5);
+        instances = {coord1, coord2, coord3};
+    }
+
+    for (const auto& instance : instances) {
+        instance->getCompletionFuture().wait();
+    }
+}
+
+}  // namespace
+}  // namespace mongo
diff --git a/src/mongo/db/s/config/configsvr_set_cluster_parameter_command.cpp b/src/mongo/db/s/config/configsvr_set_cluster_parameter_command.cpp
index 19c8d947118..a4a451c1bb4 100644
--- a/src/mongo/db/s/config/configsvr_set_cluster_parameter_command.cpp
+++ b/src/mongo/db/s/config/configsvr_set_cluster_parameter_command.cpp
@@ -79,10 +79,14 @@ public:
                                               parameterName,
                                               request().getDbName().tenantId());
 
+                auto tenantId = request().getDbName().tenantId();
+
                 SetClusterParameterCoordinatorDocument coordinatorDoc;
-                coordinatorDoc.setConfigsvrCoordinatorMetadata(
-                    {ConfigsvrCoordinatorTypeEnum::kSetClusterParameter});
+                ConfigsvrCoordinatorId cid(ConfigsvrCoordinatorTypeEnum::kSetClusterParameter);
+                cid.setSubId(StringData(tenantId ? tenantId->toString() : ""));
+                coordinatorDoc.setConfigsvrCoordinatorMetadata({cid});
                 coordinatorDoc.setParameter(request().getCommandParameter());
+                coordinatorDoc.setTenantId(tenantId);
 
                 const auto service = ConfigsvrCoordinatorService::getService(opCtx);
                 const auto instance = service->getOrCreateService(opCtx, coordinatorDoc.toBSON());
diff --git a/src/mongo/db/s/config/set_cluster_parameter_coordinator.cpp b/src/mongo/db/s/config/set_cluster_parameter_coordinator.cpp
index 2eeaacf5ea0..95986b0df63 100644
--- a/src/mongo/db/s/config/set_cluster_parameter_coordinator.cpp
+++ b/src/mongo/db/s/config/set_cluster_parameter_coordinator.cpp
@@ -61,7 +61,8 @@ bool SetClusterParameterCoordinator::hasSameOptions(const BSONObj& otherDocBSON)
     const auto otherDoc =
         StateDoc::parse(IDLParserContext("SetClusterParameterCoordinatorDocument"), otherDocBSON);
     return SimpleBSONObjComparator::kInstance.evaluate(_doc.getParameter() ==
-                                                       otherDoc.getParameter());
+                                                       otherDoc.getParameter()) &&
+        _doc.getTenantId() == otherDoc.getTenantId();
 }
 
 boost::optional<BSONObj> SetClusterParameterCoordinator::reportForCurrentOp(
@@ -74,6 +75,10 @@ boost::optional<BSONObj> SetClusterParameterCoordinator::reportForCurrentOp(
     bob.append("type", "op");
     bob.append("desc", "SetClusterParameterCoordinator");
     bob.append("op", "command");
+    auto tenantId = _doc.getTenantId();
+    if (tenantId.is_initialized()) {
+        bob.append("tenantId", tenantId->toString());
+    }
     bob.append("currentPhase", _doc.getPhase());
     bob.append("command", cmdBob.obj());
     bob.append("active", true);
@@ -122,7 +127,7 @@ bool SetClusterParameterCoordinator::_isClusterParameterSetAtTimestamp(Operation
             opCtx,
             ReadPreferenceSetting(ReadPreference::PrimaryOnly),
             repl::ReadConcernLevel::kMajorityReadConcern,
-            NamespaceString::kClusterParametersNamespace,
+            NamespaceString::makeClusterParametersNSS(_doc.getTenantId()),
             BSON("_id" << parameterName << "clusterParameterTime"
                        << *_doc.getClusterParameterTime()),
             BSONObj(),
@@ -142,7 +147,7 @@ void SetClusterParameterCoordinator::_sendSetClusterParameterToAllShards(
     LOGV2_DEBUG(6387001, 1, "Sending setClusterParameter to shards:", "shards"_attr = shards);
 
     ShardsvrSetClusterParameter request(_doc.getParameter());
-    request.setDbName(NamespaceString::kAdminDb);
+    request.setDbName(DatabaseName(_doc.getTenantId(), NamespaceString::kAdminDb));
     request.setClusterParameterTime(*_doc.getClusterParameterTime());
     sharding_util::sendCommandToShards(
         opCtx,
@@ -156,7 +161,8 @@ void SetClusterParameterCoordinator::_commit(OperationContext* opCtx) {
     LOGV2_DEBUG(6387002, 1, "Updating configsvr cluster parameter");
 
     SetClusterParameter setClusterParameterRequest(_doc.getParameter());
-    setClusterParameterRequest.setDbName(NamespaceString::kAdminDb);
+    setClusterParameterRequest.setDbName(
+        DatabaseName(_doc.getTenantId(), NamespaceString::kAdminDb));
     std::unique_ptr<ServerParameterService> parameterService =
         std::make_unique<ClusterParameterService>();
     DBDirectClient client(opCtx);
diff --git a/src/mongo/db/s/config/set_cluster_parameter_coordinator_document.idl b/src/mongo/db/s/config/set_cluster_parameter_coordinator_document.idl
index f8c7d7878bf..c6f740d17fc 100644
--- a/src/mongo/db/s/config/set_cluster_parameter_coordinator_document.idl
+++ b/src/mongo/db/s/config/set_cluster_parameter_coordinator_document.idl
@@ -58,6 +58,10 @@ structs:
             parameter:
                 type: object_owned
                 description: "Parameter to be set in the cluster"
+            tenantId:
+                type: tenant_id
+                description: "Tenant ID associated with parameter to be set"
+                optional: true
             clusterParameterTime:
                 type: timestamp
                 description: "Timestamp determined at the beginning that determines the parameter time."