SERVER-19914 Move ownership of config server connection string from C…

…atalogManager to ShardRegistry
adamchel · Aug 13, 2015 · 33ac3a1 · 33ac3a1
1 parent 0978210
commit 33ac3a1
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 54 deletions.
diff --git 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
@@ -111,8 +111,12 @@ class DistLockCatalogFixture : public mongo::unittest::Test {
 
         _networkTestEnv = stdx::make_unique<NetworkTestEnv>(executor.get(), network);
 
-        _shardRegistry = stdx::make_unique<ShardRegistry>(
-            stdx::make_unique<RemoteCommandTargeterFactoryMock>(), std::move(executor), network);
+        ConnectionString configCS(HostAndPort("dummy:1234"));
+        _shardRegistry =
+            stdx::make_unique<ShardRegistry>(stdx::make_unique<RemoteCommandTargeterFactoryMock>(),
+                                             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
@@ -106,26 +106,17 @@ void _toBatchError(const Status& status, BatchedCommandResponse* response) {
 
 }  // namespace
 
-CatalogManagerReplicaSet::CatalogManagerReplicaSet() = default;
+CatalogManagerReplicaSet::CatalogManagerReplicaSet(std::unique_ptr<DistLockManager> distLockManager)
+    : _distLockManager(std::move(distLockManager)) {}
 
 CatalogManagerReplicaSet::~CatalogManagerReplicaSet() = default;
 
-Status CatalogManagerReplicaSet::init(const ConnectionString& configCS,
-                                      std::unique_ptr<DistLockManager> 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
@@ -45,17 +45,9 @@ class VersionType;
  */
 class CatalogManagerReplicaSet final : public CatalogManager {
 public:
-    CatalogManagerReplicaSet();
+    explicit CatalogManagerReplicaSet(std::unique_ptr<DistLockManager> 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> 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> _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
@@ -94,19 +94,18 @@ void CatalogManagerReplSetTestFixture::setUp() {
     _networkTestEnv = stdx::make_unique<NetworkTestEnv>(executor.get(), _mockNetwork);
     _executor = executor.get();
 
-    std::unique_ptr<CatalogManagerReplicaSet> cm(stdx::make_unique<CatalogManagerReplicaSet>());
+    std::unique_ptr<CatalogManagerReplicaSet> cm(
+        stdx::make_unique<CatalogManagerReplicaSet>(stdx::make_unique<DistLockManagerMock>()));
 
-    ASSERT_OK(cm->init(
-        ConnectionString::forReplicaSet("CatalogManagerReplSetTest",
-                                        {HostAndPort{"TestHost1"}, HostAndPort{"TestHost2"}}),
-        stdx::make_unique<DistLockManagerMock>()));
+    ConnectionString configCS = ConnectionString::forReplicaSet(
+        "CatalogManagerReplSetTest", {HostAndPort{"TestHost1"}, HostAndPort{"TestHost2"}});
 
     auto configTargeter(stdx::make_unique<RemoteCommandTargeterMock>());
     _configTargeter = configTargeter.get();
     _targeterFactory->addTargeterToReturn(cm->connectionString(), std::move(configTargeter));
 
     auto shardRegistry(stdx::make_unique<ShardRegistry>(
-        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
@@ -67,21 +67,21 @@ const Milliseconds kNotMasterRetryInterval{500};
 
 ShardRegistry::ShardRegistry(std::unique_ptr<RemoteCommandTargeterFactory> targeterFactory,
                              std::unique_ptr<executor::TaskExecutor> 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<stdx::mutex> 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
@@ -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<RemoteCommandTargeterFactory> targeterFactory,
                   std::unique_ptr<executor::TaskExecutor> 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
@@ -75,7 +75,8 @@ Status initializeGlobalShardingState(const ConnectionString& configCS) {
     auto shardRegistry(
         stdx::make_unique<ShardRegistry>(stdx::make_unique<RemoteCommandTargeterFactoryImpl>(),
                                          makeTaskExecutor(std::move(network)),
-                                         networkPtr));
+                                         networkPtr,
+                                         configCS));
 
     std::unique_ptr<CatalogManager> catalogManager;
     if (configCS.type() == ConnectionString::SET) {
@@ -91,11 +92,8 @@ Status initializeGlobalShardingState(const ConnectionString& configCS) {
             ReplSetDistLockManager::kDistLockPingInterval,
             ReplSetDistLockManager::kDistLockExpirationTime);
 
-        auto catalogManagerReplicaSet = stdx::make_unique<CatalogManagerReplicaSet>();
-        Status status = catalogManagerReplicaSet->init(configCS, std::move(distLockManager));
-        if (!status.isOK()) {
-            return status;
-        }
+        auto catalogManagerReplicaSet =
+            stdx::make_unique<CatalogManagerReplicaSet>(std::move(distLockManager));
 
         catalogManager = std::move(catalogManagerReplicaSet);
     } else {
-Original file line number
+Diff line change
@@ Expand Up / @@ -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;
@@ Expand Down @@