SERVER-20855 Change CatalogManager::getAllShards to return the config OpTime the shards were loaded at

14 files changed, 59 insertions, 61 deletions

diff --git a/src/mongo/s/balancer_policy.cpp b/src/mongo/s/balancer_policy.cpp
index 8864a0b298b..473e13ed6c9 100644
--- a/src/mongo/s/balancer_policy.cpp
+++ b/src/mongo/s/balancer_policy.cpp
@@ -273,11 +273,11 @@ void DistributionStatus::dump() const {
 
 Status DistributionStatus::populateShardInfoMap(OperationContext* txn, ShardInfoMap* shardInfo) {
     try {
-        vector<ShardType> shards;
-        Status status = grid.catalogManager(txn)->getAllShards(txn, &shards);
-        if (!status.isOK()) {
-            return status;
+        auto shardsStatus = grid.catalogManager(txn)->getAllShards(txn);
+        if (!shardsStatus.isOK()) {
+            return shardsStatus.getStatus();
         }
+        vector<ShardType> shards = std::move(shardsStatus.getValue().value);
 
         for (const ShardType& shardData : shards) {
             std::set<std::string> dummy;
diff --git a/src/mongo/s/catalog/catalog_manager.h b/src/mongo/s/catalog/catalog_manager.h
index 2b5b7dcb094..47759ca7cfa 100644
--- a/src/mongo/s/catalog/catalog_manager.h
+++ b/src/mongo/s/catalog/catalog_manager.h
@@ -287,7 +287,7 @@ public:
      * Retrieves all shards in this sharded cluster.
      * Returns a !OK status if an error occurs.
      */
-    virtual Status getAllShards(OperationContext* txn, std::vector<ShardType>* shards) = 0;
+    virtual StatusWith<OpTimePair<std::vector<ShardType>>> getAllShards(OperationContext* txn) = 0;
 
     /**
      * Runs a user management command on the config servers, potentially synchronizing through
diff --git a/src/mongo/s/catalog/catalog_manager_mock.cpp b/src/mongo/s/catalog/catalog_manager_mock.cpp
index ce7533eceff..e58764e2d00 100644
--- a/src/mongo/s/catalog/catalog_manager_mock.cpp
+++ b/src/mongo/s/catalog/catalog_manager_mock.cpp
@@ -35,6 +35,7 @@
 #include "mongo/s/catalog/type_collection.h"
 #include "mongo/s/catalog/type_database.h"
 #include "mongo/s/catalog/type_settings.h"
+#include "mongo/s/catalog/type_shard.h"
 
 namespace mongo {
 
@@ -133,8 +134,9 @@ StatusWith<string> CatalogManagerMock::getTagForChunk(OperationContext* txn,
     return string();
 }
 
-Status CatalogManagerMock::getAllShards(OperationContext* txn, vector<ShardType>* shards) {
-    return Status::OK();
+StatusWith<OpTimePair<std::vector<ShardType>>> CatalogManagerMock::getAllShards(
+    OperationContext* txn) {
+    return {OpTimePair<std::vector<ShardType>>{{}}};
 }
 
 bool CatalogManagerMock::runUserManagementWriteCommand(OperationContext* txn,
diff --git a/src/mongo/s/catalog/catalog_manager_mock.h b/src/mongo/s/catalog/catalog_manager_mock.h
index 39a0d111545..1a76c3cdd91 100644
--- a/src/mongo/s/catalog/catalog_manager_mock.h
+++ b/src/mongo/s/catalog/catalog_manager_mock.h
@@ -104,7 +104,7 @@ public:
                                            const std::string& collectionNs,
                                            const ChunkType& chunk) override;
 
-    Status getAllShards(OperationContext* txn, std::vector<ShardType>* shards) override;
+    StatusWith<OpTimePair<std::vector<ShardType>>> getAllShards(OperationContext* txn) override;
 
     bool runUserManagementWriteCommand(OperationContext* txn,
                                        const std::string& commandName,
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.cpp b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
index 10c464f1976..f14dd5cd75a 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.cpp
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
@@ -473,14 +473,9 @@ StatusWith<std::string> ForwardingCatalogManager::getTagForChunk(OperationContex
     return retry(txn, [&] { return _actual->getTagForChunk(txn, collectionNs, chunk); });
 }
 
-Status ForwardingCatalogManager::getAllShards(OperationContext* txn,
-                                              std::vector<ShardType>* shards) {
-    invariant(shards->empty());
-    return retry(txn,
-                 [&] {
-                     shards->clear();
-                     return _actual->getAllShards(txn, shards);
-                 });
+StatusWith<OpTimePair<std::vector<ShardType>>> ForwardingCatalogManager::getAllShards(
+    OperationContext* txn) {
+    return retry(txn, [&] { return _actual->getAllShards(txn); });
 }
 
 bool ForwardingCatalogManager::runUserManagementWriteCommand(OperationContext* txn,
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.h b/src/mongo/s/catalog/forwarding_catalog_manager.h
index ed7cb2f82a1..7abed5eece4 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.h
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.h
@@ -176,7 +176,7 @@ private:
                                            const std::string& collectionNs,
                                            const ChunkType& chunk) override;
 
-    Status getAllShards(OperationContext* txn, std::vector<ShardType>* shards) override;
+    StatusWith<OpTimePair<std::vector<ShardType>>> getAllShards(OperationContext* txn) override;
 
     bool runUserManagementWriteCommand(OperationContext* txn,
                                        const std::string& commandName,
diff --git a/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp b/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
index 44c98e2fb65..9908f683c88 100644
--- a/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
+++ b/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
@@ -549,11 +549,11 @@ Status CatalogManagerLegacy::dropCollection(OperationContext* txn, const Namespa
     logChange(
         txn, txn->getClient()->clientAddress(true), "dropCollection.start", ns.ns(), BSONObj());
 
-    vector<ShardType> allShards;
-    Status status = getAllShards(txn, &allShards);
-    if (!status.isOK()) {
-        return status;
+    auto shardsStatus = getAllShards(txn);
+    if (!shardsStatus.isOK()) {
+        return shardsStatus.getStatus();
     }
+    vector<ShardType> allShards = std::move(shardsStatus.getValue().value);
 
     LOG(1) << "dropCollection " << ns << " started";
 
@@ -936,7 +936,9 @@ StatusWith<string> CatalogManagerLegacy::getTagForChunk(OperationContext* txn,
     return status.getStatus();
 }
 
-Status CatalogManagerLegacy::getAllShards(OperationContext* txn, vector<ShardType>* shards) {
+StatusWith<OpTimePair<std::vector<ShardType>>> CatalogManagerLegacy::getAllShards(
+    OperationContext* txn) {
+    std::vector<ShardType> shards;
     ScopedDbConnection conn(_configServerConnectionString, 30.0);
     std::unique_ptr<DBClientCursor> cursor(
         _safeCursor(conn->query(ShardType::ConfigNS, BSONObj())));
@@ -945,7 +947,7 @@ Status CatalogManagerLegacy::getAllShards(OperationContext* txn, vector<ShardTyp
 
         StatusWith<ShardType> shardRes = ShardType::fromBSON(shardObj);
         if (!shardRes.isOK()) {
-            shards->clear();
+            shards.clear();
             conn.done();
             return Status(ErrorCodes::FailedToParse,
                           str::stream() << "Failed to parse shard with id ("
@@ -953,11 +955,11 @@ Status CatalogManagerLegacy::getAllShards(OperationContext* txn, vector<ShardTyp
                                         << "): " << shardRes.getStatus().toString());
         }
 
-        shards->push_back(shardRes.getValue());
+        shards.push_back(shardRes.getValue());
     }
     conn.done();
 
-    return Status::OK();
+    return OpTimePair<std::vector<ShardType>>{std::move(shards)};
 }
 
 bool CatalogManagerLegacy::runUserManagementWriteCommand(OperationContext* txn,
diff --git a/src/mongo/s/catalog/legacy/catalog_manager_legacy.h b/src/mongo/s/catalog/legacy/catalog_manager_legacy.h
index 2b7f6c8548c..e95ab49935a 100644
--- a/src/mongo/s/catalog/legacy/catalog_manager_legacy.h
+++ b/src/mongo/s/catalog/legacy/catalog_manager_legacy.h
@@ -104,7 +104,7 @@ public:
                                            const std::string& collectionNs,
                                            const ChunkType& chunk) override;
 
-    Status getAllShards(OperationContext* txn, std::vector<ShardType>* shards) override;
+    StatusWith<OpTimePair<std::vector<ShardType>>> getAllShards(OperationContext* txn) override;
 
     /**
      * Grabs a distributed lock and runs the command on all config servers.
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 e4b43e8e602..360607991ae 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
@@ -443,11 +443,11 @@ Status CatalogManagerReplicaSet::dropCollection(OperationContext* txn, const Nam
     logChange(
         txn, txn->getClient()->clientAddress(true), "dropCollection.start", ns.ns(), BSONObj());
 
-    vector<ShardType> allShards;
-    Status status = getAllShards(txn, &allShards);
-    if (!status.isOK()) {
-        return status;
+    auto shardsStatus = getAllShards(txn);
+    if (!shardsStatus.isOK()) {
+        return shardsStatus.getStatus();
     }
+    vector<ShardType> allShards = std::move(shardsStatus.getValue().value);
 
     LOG(1) << "dropCollection " << ns << " started";
 
@@ -793,7 +793,9 @@ StatusWith<string> CatalogManagerReplicaSet::getTagForChunk(OperationContext* tx
     return tagsResult.getValue().getTag();
 }
 
-Status CatalogManagerReplicaSet::getAllShards(OperationContext* txn, vector<ShardType>* shards) {
+StatusWith<OpTimePair<std::vector<ShardType>>> CatalogManagerReplicaSet::getAllShards(
+    OperationContext* txn) {
+    std::vector<ShardType> shards;
     auto findStatus = _exhaustiveFindOnConfig(txn,
                                               NamespaceString(ShardType::ConfigNS),
                                               BSONObj(),     // no query filter
@@ -806,17 +808,17 @@ Status CatalogManagerReplicaSet::getAllShards(OperationContext* txn, vector<Shar
     for (const BSONObj& doc : findStatus.getValue().value) {
         auto shardRes = ShardType::fromBSON(doc);
         if (!shardRes.isOK()) {
-            shards->clear();
+            shards.clear();
             return {ErrorCodes::FailedToParse,
                     stream() << "Failed to parse shard with id ("
                              << doc[ShardType::name()].toString()
                              << "): " << shardRes.getStatus().toString()};
         }
 
-        shards->push_back(shardRes.getValue());
+        shards.push_back(shardRes.getValue());
     }
 
-    return Status::OK();
+    return OpTimePair<std::vector<ShardType>>{std::move(shards), findStatus.getValue().opTime};
 }
 
 bool CatalogManagerReplicaSet::runUserManagementWriteCommand(OperationContext* txn,
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 f5a4e424441..0f9981e16ad 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.h
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.h
@@ -101,7 +101,7 @@ public:
                                            const std::string& collectionNs,
                                            const ChunkType& chunk) override;
 
-    Status getAllShards(OperationContext* txn, std::vector<ShardType>* shards) override;
+    StatusWith<OpTimePair<std::vector<ShardType>>> getAllShards(OperationContext* txn) override;
 
     bool runUserManagementWriteCommand(OperationContext* txn,
                                        const std::string& commandName,
diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
index 44c414e239c..993fa52f12c 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
@@ -327,9 +327,8 @@ TEST_F(CatalogManagerReplSetTest, GetAllShardsValid) {
     const vector<ShardType> expectedShardsList = {s1, s2, s3};
 
     auto future = launchAsync([this] {
-        vector<ShardType> shards;
-        ASSERT_OK(catalogManager()->getAllShards(operationContext(), &shards));
-        return shards;
+        auto shards = assertGet(catalogManager()->getAllShards(operationContext()));
+        return shards.value;
     });
 
     onFindCommand([this, &s1, &s2, &s3](const RemoteCommandRequest& request) {
@@ -363,11 +362,9 @@ TEST_F(CatalogManagerReplSetTest, GetAllShardsWithInvalidShard) {
     configTargeter()->setFindHostReturnValue(HostAndPort("TestHost1"));
 
     auto future = launchAsync([this] {
-        vector<ShardType> shards;
-        Status status = catalogManager()->getAllShards(operationContext(), &shards);
+        auto status = catalogManager()->getAllShards(operationContext());
 
-        ASSERT_EQ(ErrorCodes::FailedToParse, status);
-        ASSERT_EQ(0U, shards.size());
+        ASSERT_EQ(ErrorCodes::FailedToParse, status.getStatus());
     });
 
     onFindCommand([](const RemoteCommandRequest& request) {
diff --git a/src/mongo/s/client/shard_registry.cpp b/src/mongo/s/client/shard_registry.cpp
index da709faf1f8..5e94c796942 100644
--- a/src/mongo/s/client/shard_registry.cpp
+++ b/src/mongo/s/client/shard_registry.cpp
@@ -123,12 +123,12 @@ void ShardRegistry::shutdown() {
 }
 
 void ShardRegistry::reload(OperationContext* txn) {
-    vector<ShardType> shards;
-    Status status = grid.catalogManager(txn)->getAllShards(txn, &shards);
+    auto shardsStatus = grid.catalogManager(txn)->getAllShards(txn);
     uassert(13632,
             str::stream() << "could not get updated shard list from config server due to "
-                          << status.toString(),
-            status.isOK());
+                          << shardsStatus.getStatus().toString(),
+            shardsStatus.isOK());
+    vector<ShardType> shards = std::move(shardsStatus.getValue().value);
 
     int numShards = shards.size();
 
diff --git a/src/mongo/s/commands/cluster_list_shards_cmd.cpp b/src/mongo/s/commands/cluster_list_shards_cmd.cpp
index dedbc136726..1e0f4e4060f 100644
--- a/src/mongo/s/commands/cluster_list_shards_cmd.cpp
+++ b/src/mongo/s/commands/cluster_list_shards_cmd.cpp
@@ -73,11 +73,12 @@ public:
                      int options,
                      std::string& errmsg,
                      BSONObjBuilder& result) {
-        std::vector<ShardType> shards;
-        Status status = grid.catalogManager(txn)->getAllShards(txn, &shards);
-        if (!status.isOK()) {
-            return appendCommandStatus(result, status);
+        auto shardsStatus = grid.catalogManager(txn)->getAllShards(txn);
+        if (!shardsStatus.isOK()) {
+            return appendCommandStatus(result, shardsStatus.getStatus());
         }
+        std::vector<ShardType> shards = std::move(shardsStatus.getValue().value);
+
         std::vector<BSONObj> shardsObj;
         for (std::vector<ShardType>::const_iterator it = shards.begin(); it != shards.end(); it++) {
             shardsObj.push_back(it->toBSON());
diff --git a/src/mongo/s/commands/cluster_user_management_commands.cpp b/src/mongo/s/commands/cluster_user_management_commands.cpp
index afb2559aa7c..cedb28b3c87 100644
--- a/src/mongo/s/commands/cluster_user_management_commands.cpp
+++ b/src/mongo/s/commands/cluster_user_management_commands.cpp
@@ -823,20 +823,19 @@ Status runUpgradeOnAllShards(OperationContext* txn, int maxSteps, const BSONObj&
     const BSONObj cmdObj = cmdObjBuilder.done();
 
     // Upgrade each shard in turn, stopping on first failure.
-    vector<ShardType> allShards;
-    Status status = grid.catalogManager(txn)->getAllShards(txn, &allShards);
-    if (!status.isOK()) {
-        return status;
-    }
     auto shardRegistry = grid.shardRegistry();
-    for (const auto& shardEntry : allShards) {
-        auto cmdResult = shardRegistry->runCommandWithNotMasterRetries(
-            txn, shardEntry.getName(), "admin", cmdObj);
+    shardRegistry->reload(txn);
+    vector<string> shardIds;
+    shardRegistry->getAllShardIds(&shardIds);
+
+    for (const auto& shardId : shardIds) {
+        auto cmdResult =
+            shardRegistry->runCommandWithNotMasterRetries(txn, shardId, "admin", cmdObj);
 
         if (!cmdResult.isOK()) {
-            return Status(status.code(),
-                          str::stream() << "Failed to run authSchemaUpgrade on shard "
-                                        << shardEntry.getName() << causedBy(status));
+            return Status(cmdResult.getStatus().code(),
+                          str::stream() << "Failed to run authSchemaUpgrade on shard " << shardId
+                                        << causedBy(cmdResult.getStatus()));
         }
     }