From 33ac3a1cf049729ebf586938728b9aea1d9a61ad Mon Sep 17 00:00:00 2001 From: Spencer T Brody Date: Wed, 12 Aug 2015 17:41:47 -0400 Subject: [PATCH] SERVER-19914 Move ownership of config server connection string from CatalogManager to ShardRegistry --- src/mongo/s/catalog/catalog_manager.h | 1 + .../s/catalog/dist_lock_catalog_impl_test.cpp | 8 +++-- .../replset/catalog_manager_replica_set.cpp | 29 +++++++------------ .../replset/catalog_manager_replica_set.h | 13 +-------- ...talog_manager_replica_set_test_fixture.cpp | 11 ++++--- src/mongo/s/client/shard_registry.cpp | 14 ++++----- src/mongo/s/client/shard_registry.h | 12 ++++++-- src/mongo/s/sharding_initialization.cpp | 10 +++---- 8 files changed, 44 insertions(+), 54 deletions(-) diff --git a/src/mongo/s/catalog/catalog_manager.h b/src/mongo/s/catalog/catalog_manager.h index 4ca1a2bc9e329..889f0941e283e 100644 --- a/src/mongo/s/catalog/catalog_manager.h +++ b/src/mongo/s/catalog/catalog_manager.h @@ -86,6 +86,7 @@ class CatalogManager { /** * Retrieves the connection string for the catalog manager's backing server. + * TODO(spencer): Remove this method in favor of getting it from the ShardRegistry */ virtual ConnectionString connectionString() = 0; diff --git a/src/mongo/s/catalog/dist_lock_catalog_impl_test.cpp b/src/mongo/s/catalog/dist_lock_catalog_impl_test.cpp index 4ffb1782fde76..6ae0480db708d 100644 --- a/src/mongo/s/catalog/dist_lock_catalog_impl_test.cpp +++ b/src/mongo/s/catalog/dist_lock_catalog_impl_test.cpp @@ -111,8 +111,12 @@ class DistLockCatalogFixture : public mongo::unittest::Test { _networkTestEnv = stdx::make_unique(executor.get(), network); - _shardRegistry = stdx::make_unique( - stdx::make_unique(), std::move(executor), network); + ConnectionString configCS(HostAndPort("dummy:1234")); + _shardRegistry = + stdx::make_unique(stdx::make_unique(), + std::move(executor), + network, + configCS); _shardRegistry->init(&_catalogMgr); _shardRegistry->startup(); diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp b/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp index 679c69494c467..340444b43f4d1 100644 --- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp +++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp @@ -106,26 +106,17 @@ void _toBatchError(const Status& status, BatchedCommandResponse* response) { } // namespace -CatalogManagerReplicaSet::CatalogManagerReplicaSet() = default; +CatalogManagerReplicaSet::CatalogManagerReplicaSet(std::unique_ptr distLockManager) + : _distLockManager(std::move(distLockManager)) {} CatalogManagerReplicaSet::~CatalogManagerReplicaSet() = default; -Status CatalogManagerReplicaSet::init(const ConnectionString& configCS, - std::unique_ptr distLockManager) { - invariant(configCS.type() == ConnectionString::SET); - - _configServerConnectionString = configCS; - _distLockManager = std::move(distLockManager); - - return Status::OK(); -} - Status CatalogManagerReplicaSet::startup() { return Status::OK(); } ConnectionString CatalogManagerReplicaSet::connectionString() { - return _configServerConnectionString; + return grid.shardRegistry()->getConfigServerConnectionString(); } void CatalogManagerReplicaSet::shutDown() { @@ -220,13 +211,13 @@ Status CatalogManagerReplicaSet::shardCollection(OperationContext* txn, // Tell the primary mongod to refresh its data // TODO: Think the real fix here is for mongos to just // assume that all collections are sharded, when we get there - SetShardVersionRequest ssv = - SetShardVersionRequest::makeForVersioningNoPersist(_configServerConnectionString, - dbPrimaryShardId, - primaryShard->getConnString(), - NamespaceString(ns), - manager->getVersion(), - true); + SetShardVersionRequest ssv = SetShardVersionRequest::makeForVersioningNoPersist( + grid.shardRegistry()->getConfigServerConnectionString(), + dbPrimaryShardId, + primaryShard->getConnString(), + NamespaceString(ns), + manager->getVersion(), + true); auto ssvStatus = grid.shardRegistry()->runCommandWithNotMasterRetries( dbPrimaryShardId, "admin", ssv.toBSON()); diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set.h b/src/mongo/s/catalog/replset/catalog_manager_replica_set.h index c211c81e5d778..89366c0cd9bbd 100644 --- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.h +++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.h @@ -45,17 +45,9 @@ class VersionType; */ class CatalogManagerReplicaSet final : public CatalogManager { public: - CatalogManagerReplicaSet(); + explicit CatalogManagerReplicaSet(std::unique_ptr distLockManager); virtual ~CatalogManagerReplicaSet(); - /** - * Initializes the catalog manager. - * Can only be called once for the lifetime of the catalog manager. - * TODO(spencer): Take pointer to ShardRegistry rather than getting it from the global - * "grid" object. - */ - Status init(const ConnectionString& configCS, std::unique_ptr distLockManager); - Status startup() override; ConnectionString connectionString() override; @@ -191,9 +183,6 @@ class CatalogManagerReplicaSet final : public CatalogManager { stdx::mutex _mutex; - // Config server connection string - ConnectionString _configServerConnectionString; // (R) - // Distribted lock manager singleton. std::unique_ptr _distLockManager; // (R) diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test_fixture.cpp b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test_fixture.cpp index ee075e7233b63..0ff151e0325d1 100644 --- a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test_fixture.cpp +++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test_fixture.cpp @@ -94,19 +94,18 @@ void CatalogManagerReplSetTestFixture::setUp() { _networkTestEnv = stdx::make_unique(executor.get(), _mockNetwork); _executor = executor.get(); - std::unique_ptr cm(stdx::make_unique()); + std::unique_ptr cm( + stdx::make_unique(stdx::make_unique())); - ASSERT_OK(cm->init( - ConnectionString::forReplicaSet("CatalogManagerReplSetTest", - {HostAndPort{"TestHost1"}, HostAndPort{"TestHost2"}}), - stdx::make_unique())); + ConnectionString configCS = ConnectionString::forReplicaSet( + "CatalogManagerReplSetTest", {HostAndPort{"TestHost1"}, HostAndPort{"TestHost2"}}); auto configTargeter(stdx::make_unique()); _configTargeter = configTargeter.get(); _targeterFactory->addTargeterToReturn(cm->connectionString(), std::move(configTargeter)); auto shardRegistry(stdx::make_unique( - std::move(targeterFactory), std::move(executor), _mockNetwork)); + std::move(targeterFactory), std::move(executor), _mockNetwork, configCS)); shardRegistry->init(cm.get()); shardRegistry->startup(); diff --git a/src/mongo/s/client/shard_registry.cpp b/src/mongo/s/client/shard_registry.cpp index 321e6d86de3f2..61544f4a7346c 100644 --- a/src/mongo/s/client/shard_registry.cpp +++ b/src/mongo/s/client/shard_registry.cpp @@ -67,21 +67,21 @@ const Milliseconds kNotMasterRetryInterval{500}; ShardRegistry::ShardRegistry(std::unique_ptr targeterFactory, std::unique_ptr executor, - executor::NetworkInterface* network) + executor::NetworkInterface* network, + ConnectionString configServerCS) : _targeterFactory(std::move(targeterFactory)), _executor(std::move(executor)), _network(network), - _catalogManager(nullptr) {} + _configServerCS(configServerCS), + _catalogManager(nullptr) { + _addConfigShard_inlock(); +} ShardRegistry::~ShardRegistry() = default; void ShardRegistry::init(CatalogManager* catalogManager) { invariant(!_catalogManager); _catalogManager = catalogManager; - - // add config shard registry entry so know it's always there - stdx::lock_guard lk(_mutex); - _addConfigShard_inlock(); } void ShardRegistry::startup() { @@ -207,7 +207,7 @@ void ShardRegistry::toBSON(BSONObjBuilder* result) { void ShardRegistry::_addConfigShard_inlock() { ShardType configServerShard; configServerShard.setName("config"); - configServerShard.setHost(_catalogManager->connectionString().toString()); + configServerShard.setHost(_configServerCS.toString()); _addShard_inlock(configServerShard); } diff --git a/src/mongo/s/client/shard_registry.h b/src/mongo/s/client/shard_registry.h index b465b065e8e18..a7980b61f2ab1 100644 --- a/src/mongo/s/client/shard_registry.h +++ b/src/mongo/s/client/shard_registry.h @@ -85,11 +85,12 @@ class ShardRegistry { * @param commandRunner Command runner for executing commands against hosts * @param executor Asynchronous task executor to use for making calls to shards. * @param network Network interface backing executor. - * @param catalogManager Used to retrieve the list of registered shard. TODO: remove. + * @param configServerCS ConnectionString used for communicating with the config servers */ ShardRegistry(std::unique_ptr targeterFactory, std::unique_ptr executor, - executor::NetworkInterface* network); + executor::NetworkInterface* network, + ConnectionString configServerCS); ~ShardRegistry(); @@ -118,6 +119,10 @@ class ShardRegistry { return _network; } + ConnectionString getConfigServerConnectionString() const { + return _configServerCS; + } + void reload(); /** @@ -226,6 +231,9 @@ class ShardRegistry { // configuration, such as getting the current server's hostname. executor::NetworkInterface* const _network; + // Config server connection string + ConnectionString _configServerCS; + // Catalog manager from which to load the shard information. Not owned and must outlive // the shard registry object. Should be set once by a call to init() then never modified again. CatalogManager* _catalogManager; diff --git a/src/mongo/s/sharding_initialization.cpp b/src/mongo/s/sharding_initialization.cpp index eb88be7e40990..8da4e050bb3a1 100644 --- a/src/mongo/s/sharding_initialization.cpp +++ b/src/mongo/s/sharding_initialization.cpp @@ -75,7 +75,8 @@ Status initializeGlobalShardingState(const ConnectionString& configCS) { auto shardRegistry( stdx::make_unique(stdx::make_unique(), makeTaskExecutor(std::move(network)), - networkPtr)); + networkPtr, + configCS)); std::unique_ptr catalogManager; if (configCS.type() == ConnectionString::SET) { @@ -91,11 +92,8 @@ Status initializeGlobalShardingState(const ConnectionString& configCS) { ReplSetDistLockManager::kDistLockPingInterval, ReplSetDistLockManager::kDistLockExpirationTime); - auto catalogManagerReplicaSet = stdx::make_unique(); - Status status = catalogManagerReplicaSet->init(configCS, std::move(distLockManager)); - if (!status.isOK()) { - return status; - } + auto catalogManagerReplicaSet = + stdx::make_unique(std::move(distLockManager)); catalogManager = std::move(catalogManagerReplicaSet); } else {