SERVER-22937 Retry operations run through the ShardRegistry wherever possible

15 files changed, 123 insertions, 166 deletions

diff --git a/src/mongo/s/balance.cpp b/src/mongo/s/balance.cpp
index 48d36bf3130..98bdb890c5d 100644
--- a/src/mongo/s/balance.cpp
+++ b/src/mongo/s/balance.cpp
@@ -279,7 +279,7 @@ bool Balancer::_checkOIDs(OperationContext* txn) {
             continue;
         }
 
-        BSONObj f = uassertStatusOK(grid.shardRegistry()->runCommandOnShard(
+        BSONObj f = uassertStatusOK(grid.shardRegistry()->runIdempotentCommandOnShard(
             txn,
             s,
             ReadPreferenceSetting{ReadPreference::PrimaryOnly},
@@ -293,7 +293,7 @@ bool Balancer::_checkOIDs(OperationContext* txn) {
                 log() << "error: 2 machines have " << x << " as oid machine piece: " << shardId
                       << " and " << oids[x];
 
-                uassertStatusOK(grid.shardRegistry()->runCommandOnShard(
+                uassertStatusOK(grid.shardRegistry()->runIdempotentCommandOnShard(
                     txn,
                     s,
                     ReadPreferenceSetting{ReadPreference::PrimaryOnly},
@@ -302,7 +302,7 @@ bool Balancer::_checkOIDs(OperationContext* txn) {
 
                 const auto otherShard = grid.shardRegistry()->getShard(txn, oids[x]);
                 if (otherShard) {
-                    uassertStatusOK(grid.shardRegistry()->runCommandOnShard(
+                    uassertStatusOK(grid.shardRegistry()->runIdempotentCommandOnShard(
                         txn,
                         otherShard,
                         ReadPreferenceSetting{ReadPreference::PrimaryOnly},
diff --git a/src/mongo/s/balancer_policy.cpp b/src/mongo/s/balancer_policy.cpp
index 473e13ed6c9..f8ff4c49f65 100644
--- a/src/mongo/s/balancer_policy.cpp
+++ b/src/mongo/s/balancer_policy.cpp
@@ -69,12 +69,12 @@ namespace {
 std::string retrieveShardMongoDVersion(OperationContext* txn,
                                        ShardId shardId,
                                        ShardRegistry* shardRegistry) {
-    BSONObj serverStatus = uassertStatusOK(
-        shardRegistry->runCommandOnShard(txn,
-                                         shardId,
-                                         ReadPreferenceSetting{ReadPreference::PrimaryOnly},
-                                         "admin",
-                                         BSON("serverStatus" << 1)));
+    BSONObj serverStatus = uassertStatusOK(shardRegistry->runIdempotentCommandOnShard(
+        txn,
+        shardId,
+        ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+        "admin",
+        BSON("serverStatus" << 1)));
     BSONElement versionElement = serverStatus["version"];
     if (versionElement.type() != String) {
         uassertStatusOK({ErrorCodes::NoSuchKey, "version field not found in serverStatus"});
diff --git a/src/mongo/s/catalog/catalog_manager_common.cpp b/src/mongo/s/catalog/catalog_manager_common.cpp
index cc4d8d8106b..a7e81687631 100644
--- a/src/mongo/s/catalog/catalog_manager_common.cpp
+++ b/src/mongo/s/catalog/catalog_manager_common.cpp
@@ -111,7 +111,7 @@ StatusWith<ShardType> validateHostAsShard(OperationContext* txn,
     const ReadPreferenceSetting readPref{ReadPreference::PrimaryOnly};
 
     // Is it mongos?
-    auto cmdStatus = shardRegistry->runCommandForAddShard(
+    auto cmdStatus = shardRegistry->runIdempotentCommandForAddShard(
         txn, shardConn, readPref, "admin", BSON("isdbgrid" << 1));
     if (!cmdStatus.isOK()) {
         return cmdStatus.getStatus();
@@ -123,7 +123,7 @@ StatusWith<ShardType> validateHostAsShard(OperationContext* txn,
     }
 
     // Is it a replica set?
-    cmdStatus = shardRegistry->runCommandForAddShard(
+    cmdStatus = shardRegistry->runIdempotentCommandForAddShard(
         txn, shardConn, readPref, "admin", BSON("isMaster" << 1));
     if (!cmdStatus.isOK()) {
         return cmdStatus.getStatus();
@@ -157,7 +157,7 @@ StatusWith<ShardType> validateHostAsShard(OperationContext* txn,
     }
 
     // Is it a mongos config server?
-    cmdStatus = shardRegistry->runCommandForAddShard(
+    cmdStatus = shardRegistry->runIdempotentCommandForAddShard(
         txn, shardConn, readPref, "admin", BSON("replSetGetStatus" << 1));
     if (!cmdStatus.isOK()) {
         return cmdStatus.getStatus();
@@ -260,12 +260,12 @@ StatusWith<std::vector<std::string>> getDBNamesListFromShard(
         shardRegistry->createConnection(connectionString).release()};
     invariant(shardConn);
 
-    auto cmdStatus =
-        shardRegistry->runCommandForAddShard(txn,
-                                             shardConn,
-                                             ReadPreferenceSetting{ReadPreference::PrimaryOnly},
-                                             "admin",
-                                             BSON("listDatabases" << 1));
+    auto cmdStatus = shardRegistry->runIdempotentCommandForAddShard(
+        txn,
+        shardConn,
+        ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+        "admin",
+        BSON("listDatabases" << 1));
     if (!cmdStatus.isOK()) {
         return cmdStatus.getStatus();
     }
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 9179b0f3c0a..ccefd01d7f0 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
@@ -220,8 +220,12 @@ Status CatalogManagerReplicaSet::shardCollection(OperationContext* txn,
         manager->getVersion(),
         true);
 
-    auto ssvStatus = grid.shardRegistry()->runCommandWithNotMasterRetries(
-        txn, dbPrimaryShardId, "admin", ssv.toBSON());
+    auto ssvStatus = grid.shardRegistry()->runIdempotentCommandOnShard(
+        txn,
+        dbPrimaryShardId,
+        ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+        "admin",
+        ssv.toBSON());
     if (!ssvStatus.isOK()) {
         warning() << "could not update initial version of " << ns << " on shard primary "
                   << dbPrimaryShardId << ssvStatus.getStatus();
@@ -491,8 +495,12 @@ Status CatalogManagerReplicaSet::dropCollection(OperationContext* txn, const Nam
     auto* shardRegistry = grid.shardRegistry();
 
     for (const auto& shardEntry : allShards) {
-        auto dropResult = shardRegistry->runCommandWithNotMasterRetries(
-            txn, shardEntry.getName(), ns.db().toString(), BSON("drop" << ns.coll()));
+        auto dropResult = shardRegistry->runIdempotentCommandOnShard(
+            txn,
+            shardEntry.getName(),
+            ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+            ns.db().toString(),
+            BSON("drop" << ns.coll()));
 
         if (!dropResult.isOK()) {
             return dropResult.getStatus();
@@ -556,8 +564,12 @@ Status CatalogManagerReplicaSet::dropCollection(OperationContext* txn, const Nam
             ChunkVersion::DROPPED(),
             true);
 
-        auto ssvResult = shardRegistry->runCommandWithNotMasterRetries(
-            txn, shardEntry.getName(), "admin", ssv.toBSON());
+        auto ssvResult = shardRegistry->runIdempotentCommandOnShard(
+            txn,
+            shardEntry.getName(),
+            ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+            "admin",
+            ssv.toBSON());
 
         if (!ssvResult.isOK()) {
             return ssvResult.getStatus();
@@ -568,8 +580,12 @@ Status CatalogManagerReplicaSet::dropCollection(OperationContext* txn, const Nam
             return ssvStatus;
         }
 
-        auto unsetShardingStatus = shardRegistry->runCommandWithNotMasterRetries(
-            txn, shardEntry.getName(), "admin", BSON("unsetSharding" << 1));
+        auto unsetShardingStatus = shardRegistry->runIdempotentCommandOnShard(
+            txn,
+            shardEntry.getName(),
+            ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+            "admin",
+            BSON("unsetSharding" << 1));
 
         if (!unsetShardingStatus.isOK()) {
             return unsetShardingStatus.getStatus();
@@ -851,7 +867,8 @@ bool CatalogManagerReplicaSet::runReadCommandForTest(OperationContext* txn,
     cmdBuilder.appendElements(cmdObj);
     _appendReadConcern(&cmdBuilder);
 
-    auto resultStatus = _runReadCommand(txn, dbname, cmdBuilder.done(), kConfigReadSelector);
+    auto resultStatus = grid.shardRegistry()->runIdempotentCommandOnConfig(
+        txn, kConfigReadSelector, dbname, cmdBuilder.done());
     if (resultStatus.isOK()) {
         result->appendElements(resultStatus.getValue());
         return Command::getStatusFromCommandResult(resultStatus.getValue()).isOK();
@@ -864,7 +881,8 @@ bool CatalogManagerReplicaSet::runUserManagementReadCommand(OperationContext* tx
                                                             const std::string& dbname,
                                                             const BSONObj& cmdObj,
                                                             BSONObjBuilder* result) {
-    auto resultStatus = _runReadCommand(txn, dbname, cmdObj, kConfigPrimaryPreferredSelector);
+    auto resultStatus = grid.shardRegistry()->runIdempotentCommandOnConfig(
+        txn, kConfigPrimaryPreferredSelector, dbname, cmdObj);
     if (resultStatus.isOK()) {
         result->appendElements(resultStatus.getValue());
         return Command::getStatusFromCommandResult(resultStatus.getValue()).isOK();
@@ -1235,8 +1253,8 @@ StatusWith<long long> CatalogManagerReplicaSet::_runCountCommandOnConfig(Operati
     countBuilder.append("query", query);
     _appendReadConcern(&countBuilder);
 
-    auto resultStatus =
-        _runReadCommand(txn, ns.db().toString(), countBuilder.done(), kConfigReadSelector);
+    auto resultStatus = grid.shardRegistry()->runIdempotentCommandOnConfig(
+        txn, kConfigReadSelector, ns.db().toString(), countBuilder.done());
     if (!resultStatus.isOK()) {
         return resultStatus.getStatus();
     }
@@ -1387,32 +1405,10 @@ void CatalogManagerReplicaSet::_appendReadConcern(BSONObjBuilder* builder) {
     readConcern.appendInfo(builder);
 }
 
-StatusWith<BSONObj> CatalogManagerReplicaSet::_runReadCommand(
-    OperationContext* txn,
-    const std::string& dbname,
-    const BSONObj& cmdObj,
-    const ReadPreferenceSetting& readPref) {
-    for (int retry = 1; retry <= kMaxReadRetry; ++retry) {
-        auto response = grid.shardRegistry()->runCommandOnConfig(txn, readPref, dbname, cmdObj);
-        if (response.isOK()) {
-            return response;
-        }
-
-        if (ShardRegistry::kAllRetriableErrors.count(response.getStatus().code()) &&
-            retry < kMaxReadRetry) {
-            continue;
-        }
-
-        return response.getStatus();
-    }
-
-    MONGO_UNREACHABLE;
-}
-
 Status CatalogManagerReplicaSet::appendInfoForConfigServerDatabases(OperationContext* txn,
                                                                     BSONArrayBuilder* builder) {
-    auto resultStatus =
-        _runReadCommand(txn, "admin", BSON("listDatabases" << 1), kConfigPrimaryPreferredSelector);
+    auto resultStatus = grid.shardRegistry()->runIdempotentCommandOnConfig(
+        txn, kConfigPrimaryPreferredSelector, "admin", BSON("listDatabases" << 1));
 
     if (!resultStatus.isOK()) {
         return resultStatus.getStatus();
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 53a30764490..bd5cccc80c7 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.h
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.h
@@ -164,15 +164,6 @@ private:
                                          int cappedSize) override;
 
     /**
-     * Helper method for running a read command against the config server. Automatically retries on
-     * NotMaster and network errors, so these will never be returned.
-     */
-    StatusWith<BSONObj> _runReadCommand(OperationContext* txn,
-                                        const std::string& dbname,
-                                        const BSONObj& cmdObj,
-                                        const ReadPreferenceSetting& readPref);
-
-    /**
      * Executes the specified batch write command on the current config server's primary and retries
      * on the specified set of errors using the default retry policy.
      */
diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set_add_shard_test.cpp b/src/mongo/s/catalog/replset/catalog_manager_replica_set_add_shard_test.cpp
index cdfede01e91..0d8cbfa7aae 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set_add_shard_test.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set_add_shard_test.cpp
@@ -485,10 +485,12 @@ TEST_F(AddShardTest, UnreachableHost) {
         ASSERT_EQUALS("host unreachable", status.getStatus().reason());
     });
 
-    onCommandForAddShard([](const RemoteCommandRequest& request) {
-        ASSERT_EQ(request.target, HostAndPort("StandaloneHost:12345"));
-        return StatusWith<BSONObj>{ErrorCodes::HostUnreachable, "host unreachable"};
-    });
+    for (int i = 0; i < 3; i++) {  // ShardRegistry will retry 3 times
+        onCommandForAddShard([](const RemoteCommandRequest& request) {
+            ASSERT_EQ(request.target, HostAndPort("StandaloneHost:12345"));
+            return StatusWith<BSONObj>{ErrorCodes::HostUnreachable, "host unreachable"};
+        });
+    }
 
     future.timed_get(kFutureTimeout);
 }
diff --git a/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp b/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp
index a617ef1a670..e1eebf80f26 100644
--- a/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp
+++ b/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp
@@ -342,7 +342,7 @@ Status DistLockCatalogImpl::unlockAll(OperationContext* txn, const std::string&
 
 StatusWith<DistLockCatalog::ServerInfo> DistLockCatalogImpl::getServerInfo(OperationContext* txn) {
     auto resultStatus =
-        _client->runCommandOnConfig(txn, kReadPref, "admin", BSON("serverStatus" << 1));
+        _client->runIdempotentCommandOnConfig(txn, kReadPref, "admin", BSON("serverStatus" << 1));
 
     if (!resultStatus.isOK()) {
         return resultStatus.getStatus();
diff --git a/src/mongo/s/chunk.cpp b/src/mongo/s/chunk.cpp
index 1a637cf1a96..97a0f5ec988 100644
--- a/src/mongo/s/chunk.cpp
+++ b/src/mongo/s/chunk.cpp
@@ -313,7 +313,7 @@ void Chunk::pickSplitVector(OperationContext* txn,
 
     BSONObj cmdObj = cmd.obj();
 
-    auto result = grid.shardRegistry()->runCommandOnShard(
+    auto result = grid.shardRegistry()->runIdempotentCommandOnShard(
         txn,
         getShardId(),
         ReadPreferenceSetting{ReadPreference::PrimaryPreferred},
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index 3bca3bfe94c..fcb21b5002c 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -358,7 +358,7 @@ void ChunkManager::calcInitSplitsAndShards(OperationContext* txn,
         // discover split points
         const auto primaryShard = grid.shardRegistry()->getShard(txn, primaryShardId);
         const NamespaceString nss{getns()};
-        auto result = grid.shardRegistry()->runCommandOnShard(
+        auto result = grid.shardRegistry()->runIdempotentCommandOnShard(
             txn,
             primaryShard,
             ReadPreferenceSetting{ReadPreference::PrimaryPreferred},
diff --git a/src/mongo/s/client/shard_registry.cpp b/src/mongo/s/client/shard_registry.cpp
index f28b13a8f85..cb47665bad7 100644
--- a/src/mongo/s/client/shard_registry.cpp
+++ b/src/mongo/s/client/shard_registry.cpp
@@ -643,11 +643,12 @@ StatusWith<ShardRegistry::QueryResponse> ShardRegistry::exhaustiveFindOnConfig(
     MONGO_UNREACHABLE;
 }
 
-StatusWith<BSONObj> ShardRegistry::runCommandOnShard(OperationContext* txn,
-                                                     const std::shared_ptr<Shard>& shard,
-                                                     const ReadPreferenceSetting& readPref,
-                                                     const std::string& dbName,
-                                                     const BSONObj& cmdObj) {
+StatusWith<BSONObj> ShardRegistry::runIdempotentCommandOnShard(
+    OperationContext* txn,
+    const std::shared_ptr<Shard>& shard,
+    const ReadPreferenceSetting& readPref,
+    const std::string& dbName,
+    const BSONObj& cmdObj) {
     auto response = _runCommandWithRetries(txn,
                                            _executorPool->getFixedExecutor(),
                                            shard,
@@ -657,7 +658,7 @@ StatusWith<BSONObj> ShardRegistry::runCommandOnShard(OperationContext* txn,
                                            readPref.pref == ReadPreference::PrimaryOnly
                                                ? rpc::makeEmptyMetadata()
                                                : kSecondaryOkMetadata,
-                                           kNotMasterErrors);
+                                           kAllRetriableErrors);
     if (!response.isOK()) {
         return response.getStatus();
     }
@@ -665,24 +666,26 @@ StatusWith<BSONObj> ShardRegistry::runCommandOnShard(OperationContext* txn,
     return response.getValue().response;
 }
 
-StatusWith<BSONObj> ShardRegistry::runCommandOnShard(OperationContext* txn,
-                                                     ShardId shardId,
-                                                     const ReadPreferenceSetting& readPref,
-                                                     const std::string& dbName,
-                                                     const BSONObj& cmdObj) {
+StatusWith<BSONObj> ShardRegistry::runIdempotentCommandOnShard(
+    OperationContext* txn,
+    ShardId shardId,
+    const ReadPreferenceSetting& readPref,
+    const std::string& dbName,
+    const BSONObj& cmdObj) {
     auto shard = getShard(txn, shardId);
     if (!shard) {
         return {ErrorCodes::ShardNotFound, str::stream() << "shard " << shardId << " not found"};
     }
-    return runCommandOnShard(txn, shard, readPref, dbName, cmdObj);
+    return runIdempotentCommandOnShard(txn, shard, readPref, dbName, cmdObj);
 }
 
 
-StatusWith<BSONObj> ShardRegistry::runCommandForAddShard(OperationContext* txn,
-                                                         const std::shared_ptr<Shard>& shard,
-                                                         const ReadPreferenceSetting& readPref,
-                                                         const std::string& dbName,
-                                                         const BSONObj& cmdObj) {
+StatusWith<BSONObj> ShardRegistry::runIdempotentCommandForAddShard(
+    OperationContext* txn,
+    const std::shared_ptr<Shard>& shard,
+    const ReadPreferenceSetting& readPref,
+    const std::string& dbName,
+    const BSONObj& cmdObj) {
     auto status = _runCommandWithRetries(txn,
                                          _executorForAddShard.get(),
                                          shard,
@@ -692,7 +695,7 @@ StatusWith<BSONObj> ShardRegistry::runCommandForAddShard(OperationContext* txn,
                                          readPref.pref == ReadPreference::PrimaryOnly
                                              ? rpc::makeEmptyMetadata()
                                              : kSecondaryOkMetadata,
-                                         kNotMasterErrors);
+                                         kAllRetriableErrors);
     if (!status.isOK()) {
         return status.getStatus();
     }
@@ -700,10 +703,11 @@ StatusWith<BSONObj> ShardRegistry::runCommandForAddShard(OperationContext* txn,
     return status.getValue().response;
 }
 
-StatusWith<BSONObj> ShardRegistry::runCommandOnConfig(OperationContext* txn,
-                                                      const ReadPreferenceSetting& readPref,
-                                                      const std::string& dbName,
-                                                      const BSONObj& cmdObj) {
+StatusWith<BSONObj> ShardRegistry::runIdempotentCommandOnConfig(
+    OperationContext* txn,
+    const ReadPreferenceSetting& readPref,
+    const std::string& dbName,
+    const BSONObj& cmdObj) {
     auto response = _runCommandWithRetries(
         txn,
         _executorPool->getFixedExecutor(),
@@ -712,7 +716,7 @@ StatusWith<BSONObj> ShardRegistry::runCommandOnConfig(OperationContext* txn,
         dbName,
         cmdObj,
         readPref.pref == ReadPreference::PrimaryOnly ? kReplMetadata : kReplSecondaryOkMetadata,
-        kNotMasterErrors);
+        kAllRetriableErrors);
 
     if (!response.isOK()) {
         return response.getStatus();
@@ -721,28 +725,6 @@ StatusWith<BSONObj> ShardRegistry::runCommandOnConfig(OperationContext* txn,
     return response.getValue().response;
 }
 
-StatusWith<BSONObj> ShardRegistry::runCommandWithNotMasterRetries(OperationContext* txn,
-                                                                  const ShardId& shardId,
-                                                                  const std::string& dbname,
-                                                                  const BSONObj& cmdObj) {
-    auto shard = getShard(txn, shardId);
-    invariant(!shard->isConfig());
-
-    auto response = _runCommandWithRetries(txn,
-                                           _executorPool->getFixedExecutor(),
-                                           shard,
-                                           ReadPreferenceSetting{ReadPreference::PrimaryOnly},
-                                           dbname,
-                                           cmdObj,
-                                           rpc::makeEmptyMetadata(),
-                                           kNotMasterErrors);
-    if (!response.isOK()) {
-        return response.getStatus();
-    }
-
-    return response.getValue().response;
-}
-
 StatusWith<BSONObj> ShardRegistry::runCommandOnConfigWithRetries(
     OperationContext* txn,
     const std::string& dbname,
diff --git a/src/mongo/s/client/shard_registry.h b/src/mongo/s/client/shard_registry.h
index 88002fe2a0a..c1b85506091 100644
--- a/src/mongo/s/client/shard_registry.h
+++ b/src/mongo/s/client/shard_registry.h
@@ -245,55 +245,41 @@ public:
     /**
      * Runs a command against a host belonging to the specified shard and matching the given
      * readPref, and returns the result.  It is the responsibility of the caller to check the
-     * returned BSON for command-specific failures.
+     * returned BSON for command-specific failures. It is also important that the command is safe
+     * to be retried in case we cannot verify whether or not it ran successfully.
      */
-    StatusWith<BSONObj> runCommandOnShard(OperationContext* txn,
-                                          const std::shared_ptr<Shard>& shard,
-                                          const ReadPreferenceSetting& readPref,
-                                          const std::string& dbName,
-                                          const BSONObj& cmdObj);
-    StatusWith<BSONObj> runCommandOnShard(OperationContext* txn,
-                                          ShardId shardId,
-                                          const ReadPreferenceSetting& readPref,
-                                          const std::string& dbName,
-                                          const BSONObj& cmdObj);
+    StatusWith<BSONObj> runIdempotentCommandOnShard(OperationContext* txn,
+                                                    const std::shared_ptr<Shard>& shard,
+                                                    const ReadPreferenceSetting& readPref,
+                                                    const std::string& dbName,
+                                                    const BSONObj& cmdObj);
+    StatusWith<BSONObj> runIdempotentCommandOnShard(OperationContext* txn,
+                                                    ShardId shardId,
+                                                    const ReadPreferenceSetting& readPref,
+                                                    const std::string& dbName,
+                                                    const BSONObj& cmdObj);
 
 
     /**
-     * Same as runCommandOnShard above but used for talking to nodes that are not yet in the
-     * ShardRegistry.
+     * Same as runIdempotentCommandOnShard above but used for talking to nodes that are not yet in
+     * the ShardRegistry.
      */
-    StatusWith<BSONObj> runCommandForAddShard(OperationContext* txn,
-                                              const std::shared_ptr<Shard>& shard,
-                                              const ReadPreferenceSetting& readPref,
-                                              const std::string& dbName,
-                                              const BSONObj& cmdObj);
-
-    /**
-     * Runs a command against a config server that matches the given read preference, and returns
-     * the result.  It is the responsibility of the caller to check the returned BSON for
-     * command-specific failures.
-     */
-    StatusWith<BSONObj> runCommandOnConfig(OperationContext* txn,
-                                           const ReadPreferenceSetting& readPref,
-                                           const std::string& dbname,
-                                           const BSONObj& cmdObj);
+    StatusWith<BSONObj> runIdempotentCommandForAddShard(OperationContext* txn,
+                                                        const std::shared_ptr<Shard>& shard,
+                                                        const ReadPreferenceSetting& readPref,
+                                                        const std::string& dbName,
+                                                        const BSONObj& cmdObj);
 
     /**
-     * Helpers for running commands against a given shard with logic for retargeting and
-     * retrying the command in the event of a NotMaster response.
-     * Returns ErrorCodes::NotMaster if after the max number of retries we still haven't
-     * successfully delivered the command to a primary.  Can also return a non-ok status in the
-     * event of a network error communicating with the shard.  If we are able to get
-     * a valid response from running the command then we will return it, even if the command
-     * response indicates failure.  Thus the caller is responsible for checking the command
-     * response object for any kind of command-specific failure.  The only exception is
-     * NotMaster errors, which we intercept and follow the rules described above for handling.
+     * Runs command against a config server that matches the given read preference,  and returns
+     * the result.  It is the responsibility of the caller to check the returned BSON
+     * for command-specific failures.  It is also important that the command is safe to be retried
+     * in case we cannot verify whether or not it ran successfully.
      */
-    StatusWith<BSONObj> runCommandWithNotMasterRetries(OperationContext* txn,
-                                                       const ShardId& shard,
-                                                       const std::string& dbname,
-                                                       const BSONObj& cmdObj);
+    StatusWith<BSONObj> runIdempotentCommandOnConfig(OperationContext* txn,
+                                                     const ReadPreferenceSetting& readPref,
+                                                     const std::string& dbname,
+                                                     const BSONObj& cmdObj);
 
     class ErrorCodesHash {
     public:
diff --git a/src/mongo/s/commands/cluster_fsync_cmd.cpp b/src/mongo/s/commands/cluster_fsync_cmd.cpp
index 0f4cea8c754..732c75c2866 100644
--- a/src/mongo/s/commands/cluster_fsync_cmd.cpp
+++ b/src/mongo/s/commands/cluster_fsync_cmd.cpp
@@ -91,7 +91,7 @@ public:
                 continue;
             }
 
-            BSONObj x = uassertStatusOK(grid.shardRegistry()->runCommandOnShard(
+            BSONObj x = uassertStatusOK(grid.shardRegistry()->runIdempotentCommandOnShard(
                 txn,
                 s,
                 ReadPreferenceSetting{ReadPreference::PrimaryOnly},
diff --git a/src/mongo/s/commands/cluster_list_databases_cmd.cpp b/src/mongo/s/commands/cluster_list_databases_cmd.cpp
index 98dcbe593dc..23dad76f0a3 100644
--- a/src/mongo/s/commands/cluster_list_databases_cmd.cpp
+++ b/src/mongo/s/commands/cluster_list_databases_cmd.cpp
@@ -99,7 +99,7 @@ public:
                 continue;
             }
 
-            BSONObj x = uassertStatusOK(grid.shardRegistry()->runCommandOnShard(
+            BSONObj x = uassertStatusOK(grid.shardRegistry()->runIdempotentCommandOnShard(
                 txn,
                 s,
                 ReadPreferenceSetting{ReadPreference::PrimaryPreferred},
diff --git a/src/mongo/s/commands/cluster_user_management_commands.cpp b/src/mongo/s/commands/cluster_user_management_commands.cpp
index cedb28b3c87..a3d98a0fc0a 100644
--- a/src/mongo/s/commands/cluster_user_management_commands.cpp
+++ b/src/mongo/s/commands/cluster_user_management_commands.cpp
@@ -829,8 +829,8 @@ Status runUpgradeOnAllShards(OperationContext* txn, int maxSteps, const BSONObj&
     shardRegistry->getAllShardIds(&shardIds);
 
     for (const auto& shardId : shardIds) {
-        auto cmdResult =
-            shardRegistry->runCommandWithNotMasterRetries(txn, shardId, "admin", cmdObj);
+        auto cmdResult = shardRegistry->runIdempotentCommandOnShard(
+            txn, shardId, ReadPreferenceSetting{ReadPreference::PrimaryOnly}, "admin", cmdObj);
 
         if (!cmdResult.isOK()) {
             return Status(cmdResult.getStatus().code(),
diff --git a/src/mongo/s/shard_util.cpp b/src/mongo/s/shard_util.cpp
index 8b7ced7748e..777c33db427 100644
--- a/src/mongo/s/shard_util.cpp
+++ b/src/mongo/s/shard_util.cpp
@@ -42,12 +42,12 @@ namespace shardutil {
 StatusWith<long long> retrieveTotalShardSize(OperationContext* txn,
                                              ShardId shardId,
                                              ShardRegistry* shardRegistry) {
-    auto listDatabasesStatus =
-        shardRegistry->runCommandOnShard(txn,
-                                         shardId,
-                                         ReadPreferenceSetting{ReadPreference::PrimaryPreferred},
-                                         "admin",
-                                         BSON("listDatabases" << 1));
+    auto listDatabasesStatus = shardRegistry->runIdempotentCommandOnShard(
+        txn,
+        shardId,
+        ReadPreferenceSetting{ReadPreference::PrimaryPreferred},
+        "admin",
+        BSON("listDatabases" << 1));
     if (!listDatabasesStatus.isOK()) {
         return listDatabasesStatus.getStatus();
     }