diff options
author | Gabriel Marks <gabriel.marks@mongodb.com> | 2022-09-23 21:05:47 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-09-23 22:18:02 +0000 |
commit | 6fe48eb397d5bdbfd953b6e98b9d7ede51940882 (patch) | |
tree | 06417fe63a3ac1c28e1e168239bfac63d8a36a89 | |
parent | cdafd3e4b159ed66d61da9fc6d15c8707a3e5b3e (diff) | |
download | mongo-6fe48eb397d5bdbfd953b6e98b9d7ede51940882.tar.gz |
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." |