SERVER-21050 Make dist lock timeouts uniform

This change makes the dist lock acquisition timeouts to be uniform across
all users and sets the default to be 5 seconds (which is the maximum that
we have been using so far).

16 files changed, 41 insertions, 43 deletions

diff --git a/src/mongo/s/catalog/catalog_manager_common.cpp b/src/mongo/s/catalog/catalog_manager_common.cpp
index a4c949a4635..49f460af05c 100644
--- a/src/mongo/s/catalog/catalog_manager_common.cpp
+++ b/src/mongo/s/catalog/catalog_manager_common.cpp
@@ -418,8 +418,7 @@ Status CatalogManagerCommon::createDatabase(OperationContext* txn, const std::st
     invariant(dbName != "config");
 
     // Lock the database globally to prevent conflicts with simultaneous database creation.
-    auto scopedDistLock =
-        getDistLockManager()->lock(txn, dbName, "createDatabase", Seconds{5}, Milliseconds{500});
+    auto scopedDistLock = getDistLockManager()->lock(txn, dbName, "createDatabase");
     if (!scopedDistLock.isOK()) {
         return scopedDistLock.getStatus();
     }
@@ -536,8 +535,7 @@ Status CatalogManagerCommon::enableSharding(OperationContext* txn, const std::st
 
     // Lock the database globally to prevent conflicts with simultaneous database
     // creation/modification.
-    auto scopedDistLock =
-        getDistLockManager()->lock(txn, dbName, "enableSharding", Seconds{5}, Milliseconds{500});
+    auto scopedDistLock = getDistLockManager()->lock(txn, dbName, "enableSharding");
     if (!scopedDistLock.isOK()) {
         return scopedDistLock.getStatus();
     }
diff --git a/src/mongo/s/catalog/dist_lock_manager.cpp b/src/mongo/s/catalog/dist_lock_manager.cpp
index d45765bd9aa..46b8cae2365 100644
--- a/src/mongo/s/catalog/dist_lock_manager.cpp
+++ b/src/mongo/s/catalog/dist_lock_manager.cpp
@@ -36,8 +36,9 @@
 
 namespace mongo {
 
-const stdx::chrono::milliseconds DistLockManager::kDefaultSingleLockAttemptTimeout(0);
-const stdx::chrono::milliseconds DistLockManager::kDefaultLockRetryInterval(1000);
+const stdx::chrono::seconds DistLockManager::kDefaultLockTimeout(5);
+const stdx::chrono::milliseconds DistLockManager::kSingleLockAttemptTimeout(0);
+const stdx::chrono::milliseconds DistLockManager::kDefaultLockRetryInterval(500);
 
 DistLockManager::ScopedDistLock::ScopedDistLock(OperationContext* txn,
                                                 DistLockHandle lockHandle,
@@ -77,4 +78,5 @@ Status DistLockManager::ScopedDistLock::checkStatus() {
 
     return _lockManager->checkStatus(_txn, _lockID);
 }
-}
+
+}  // namespace mongo
diff --git a/src/mongo/s/catalog/dist_lock_manager.h b/src/mongo/s/catalog/dist_lock_manager.h
index 15bdbdc2787..56bb5d85134 100644
--- a/src/mongo/s/catalog/dist_lock_manager.h
+++ b/src/mongo/s/catalog/dist_lock_manager.h
@@ -61,7 +61,15 @@ class StatusWith;
  */
 class DistLockManager {
 public:
-    static const stdx::chrono::milliseconds kDefaultSingleLockAttemptTimeout;
+    // Default timeout which will be used if one is not passed to the lock method.
+    static const stdx::chrono::seconds kDefaultLockTimeout;
+
+    // Timeout value, which specifies that if the lock is not available immediately, no attempt
+    // should be made to wait for it to become free.
+    static const stdx::chrono::milliseconds kSingleLockAttemptTimeout;
+
+    // If timeout is passed to the lock call, what is the default frequency with which the lock will
+    // be checked for availability.
     static const stdx::chrono::milliseconds kDefaultLockRetryInterval;
 
     /**
@@ -122,7 +130,7 @@ public:
         OperationContext* txn,
         StringData name,
         StringData whyMessage,
-        stdx::chrono::milliseconds waitFor = kDefaultSingleLockAttemptTimeout,
+        stdx::chrono::milliseconds waitFor = kDefaultLockTimeout,
         stdx::chrono::milliseconds lockTryInterval = kDefaultLockRetryInterval) = 0;
 
 protected:
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.cpp b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
index ec7eca3ffa0..978ec577aa9 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.cpp
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
@@ -126,14 +126,12 @@ StatusWith<ForwardingCatalogManager::ScopedDistLock> ForwardingCatalogManager::d
     OperationContext* txn,
     StringData name,
     StringData whyMessage,
-    stdx::chrono::milliseconds waitFor,
-    stdx::chrono::milliseconds lockTryInterval) {
+    stdx::chrono::milliseconds waitFor) {
     for (int i = 0; i < 2; ++i) {
         try {
             _operationLock.lock_shared();
             auto guard = MakeGuard([this] { _operationLock.unlock_shared(); });
-            auto dlmLock = _actual->getDistLockManager()->lock(
-                txn, name, whyMessage, waitFor, lockTryInterval);
+            auto dlmLock = _actual->getDistLockManager()->lock(txn, name, whyMessage, waitFor);
             if (dlmLock.isOK()) {
                 guard.Dismiss();  // Don't unlock _operationLock; hold it until the returned
                                   // ScopedDistLock goes out of scope!
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.h b/src/mongo/s/catalog/forwarding_catalog_manager.h
index 578b8760360..a7613718d0e 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.h
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.h
@@ -100,8 +100,7 @@ public:
         OperationContext* txn,
         StringData name,
         StringData whyMessage,
-        stdx::chrono::milliseconds waitFor = DistLockManager::kDefaultSingleLockAttemptTimeout,
-        stdx::chrono::milliseconds lockTryInterval = DistLockManager::kDefaultLockRetryInterval);
+        stdx::chrono::milliseconds waitFor = DistLockManager::kSingleLockAttemptTimeout);
 
     /**
      * Returns a pointer to the CatalogManager that should be used for general CatalogManager
diff --git a/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp b/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
index e510215d8a8..cd27e73ecd3 100644
--- a/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
+++ b/src/mongo/s/catalog/legacy/catalog_manager_legacy.cpp
@@ -555,14 +555,13 @@ Status CatalogManagerLegacy::dropCollection(OperationContext* txn, const Namespa
     LOG(1) << "dropCollection " << ns << " started";
 
     // Lock the collection globally so that split/migrate cannot run
-    stdx::chrono::seconds waitFor(2);
+    stdx::chrono::seconds waitFor(DistLockManager::kDefaultLockTimeout);
     MONGO_FAIL_POINT_BLOCK(setSCCCDropCollDistLockWait, customWait) {
         const BSONObj& data = customWait.getData();
         waitFor = stdx::chrono::seconds(data["waitForSecs"].numberInt());
     }
-    const stdx::chrono::milliseconds lockTryInterval(500);
-    auto scopedDistLock =
-        getDistLockManager()->lock(txn, ns.ns(), "drop", waitFor, lockTryInterval);
+
+    auto scopedDistLock = getDistLockManager()->lock(txn, ns.ns(), "drop", waitFor);
     if (!scopedDistLock.isOK()) {
         return scopedDistLock.getStatus();
     }
@@ -887,8 +886,7 @@ bool CatalogManagerLegacy::runUserManagementWriteCommand(OperationContext* txn,
         dispatcher.addCommand(configServer, dbname, cmdObj);
     }
 
-    auto scopedDistLock =
-        getDistLockManager()->lock(txn, "authorizationData", commandName, Seconds{5});
+    auto scopedDistLock = getDistLockManager()->lock(txn, "authorizationData", commandName);
     if (!scopedDistLock.isOK()) {
         return Command::appendCommandStatus(*result, scopedDistLock.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 da8558ff369..d1e2df60892 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set.cpp
@@ -463,14 +463,13 @@ Status CatalogManagerReplicaSet::dropCollection(OperationContext* txn, const Nam
     LOG(1) << "dropCollection " << ns << " started";
 
     // Lock the collection globally so that split/migrate cannot run
-    stdx::chrono::seconds waitFor(2);
+    stdx::chrono::seconds waitFor(DistLockManager::kDefaultLockTimeout);
     MONGO_FAIL_POINT_BLOCK(setDropCollDistLockWait, customWait) {
         const BSONObj& data = customWait.getData();
         waitFor = stdx::chrono::seconds(data["waitForSecs"].numberInt());
     }
-    const stdx::chrono::milliseconds lockTryInterval(500);
-    auto scopedDistLock =
-        getDistLockManager()->lock(txn, ns.ns(), "drop", waitFor, lockTryInterval);
+
+    auto scopedDistLock = getDistLockManager()->lock(txn, ns.ns(), "drop", waitFor);
     if (!scopedDistLock.isOK()) {
         return scopedDistLock.getStatus();
     }
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 87ce4f74bb4..8c74f77bf0c 100644
--- a/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp
+++ b/src/mongo/s/catalog/replset/dist_lock_catalog_impl.cpp
@@ -307,7 +307,7 @@ Status DistLockCatalogImpl::unlock(OperationContext* txn, const OID& lockSession
     request.setWriteConcern(_writeConcern);
 
     auto resultStatus = _client->runCommandOnConfigWithRetries(
-        txn, _locksNS.db().toString(), request.toBSON(), ShardRegistry::kNotMasterErrors);
+        txn, _locksNS.db().toString(), request.toBSON(), ShardRegistry::kAllRetriableErrors);
 
     if (!resultStatus.isOK()) {
         return resultStatus.getStatus();
diff --git a/src/mongo/s/catalog/replset/replset_dist_lock_manager.cpp b/src/mongo/s/catalog/replset/replset_dist_lock_manager.cpp
index 654da55feec..b18bd67fd94 100644
--- a/src/mongo/s/catalog/replset/replset_dist_lock_manager.cpp
+++ b/src/mongo/s/catalog/replset/replset_dist_lock_manager.cpp
@@ -430,4 +430,5 @@ void ReplSetDistLockManager::queueUnlock(const DistLockHandle& lockSessionID) {
     stdx::unique_lock<stdx::mutex> lk(_mutex);
     _unlockList.push_back(lockSessionID);
 }
-}
+
+}  // namespace mongo
diff --git a/src/mongo/s/catalog/replset/replset_dist_lock_manager_test.cpp b/src/mongo/s/catalog/replset/replset_dist_lock_manager_test.cpp
index 5cef4fb56c4..c0cfcaf7e8f 100644
--- a/src/mongo/s/catalog/replset/replset_dist_lock_manager_test.cpp
+++ b/src/mongo/s/catalog/replset/replset_dist_lock_manager_test.cpp
@@ -216,7 +216,7 @@ TEST_F(ReplSetDistLockManagerFixture, BasicLockLifeCycle) {
         auto lockStatus = getMgr()->lock(txn(),
                                          lockName,
                                          whyMsg,
-                                         DistLockManager::kDefaultSingleLockAttemptTimeout,
+                                         DistLockManager::kSingleLockAttemptTimeout,
                                          DistLockManager::kDefaultLockRetryInterval);
         ASSERT_OK(lockStatus.getStatus());
 
diff --git a/src/mongo/s/catalog/type_locks.cpp b/src/mongo/s/catalog/type_locks.cpp
index 944696e3b76..312fe209e0c 100644
--- a/src/mongo/s/catalog/type_locks.cpp
+++ b/src/mongo/s/catalog/type_locks.cpp
@@ -25,6 +25,9 @@
  *    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/s/catalog/type_locks.h"
 
 #include "mongo/base/status_with.h"
diff --git a/src/mongo/s/client/shard_registry.cpp b/src/mongo/s/client/shard_registry.cpp
index 8b9602205c2..1651bee350b 100644
--- a/src/mongo/s/client/shard_registry.cpp
+++ b/src/mongo/s/client/shard_registry.cpp
@@ -695,6 +695,8 @@ StatusWith<ShardRegistry::CommandResponse> ShardRegistry::_runCommandWithRetries
         }
 
         if (errorsToCheck.count(response.getStatus().code()) && retry < kOnErrorNumRetries) {
+            LOG(1) << "Command failed with retriable error and will be retried"
+                   << causedBy(response.getStatus());
             continue;
         }
 
diff --git a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp
index ec4d6a4fef6..b05733d86a6 100644
--- a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp
+++ b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp
@@ -65,6 +65,8 @@ namespace {
 
 AtomicUInt32 JOB_NUMBER;
 
+const stdx::chrono::milliseconds kNoDistLockTimeout(-1);
+
 /**
  * Generates a unique name for the temporary M/R output collection.
  */
@@ -455,11 +457,7 @@ public:
             {
                 // Take distributed lock to prevent split / migration.
                 auto scopedDistLock = grid.forwardingCatalogManager()->distLock(
-                    txn,
-                    finalColLong,
-                    "mr-post-process",
-                    stdx::chrono::milliseconds(-1),  // retry indefinitely
-                    stdx::chrono::milliseconds(100));
+                    txn, finalColLong, "mr-post-process", kNoDistLockTimeout);
 
                 if (!scopedDistLock.isOK()) {
                     return appendCommandStatus(result, scopedDistLock.getStatus());
diff --git a/src/mongo/s/dbclient_shard_resolver.cpp b/src/mongo/s/dbclient_shard_resolver.cpp
index bc7dc38fb27..3d2a32db338 100644
--- a/src/mongo/s/dbclient_shard_resolver.cpp
+++ b/src/mongo/s/dbclient_shard_resolver.cpp
@@ -26,6 +26,8 @@
  *    it in the license file.
  */
 
+#include "mongo/platform/basic.h"
+
 #include "mongo/s/dbclient_shard_resolver.h"
 
 #include <set>
diff --git a/src/mongo/s/write_ops/batched_command_response.cpp b/src/mongo/s/write_ops/batched_command_response.cpp
index 238141ce2b9..71afec87799 100644
--- a/src/mongo/s/write_ops/batched_command_response.cpp
+++ b/src/mongo/s/write_ops/batched_command_response.cpp
@@ -340,14 +340,6 @@ void BatchedCommandResponse::setOk(int ok) {
     _isOkSet = true;
 }
 
-void BatchedCommandResponse::unsetOk() {
-    _isOkSet = false;
-}
-
-bool BatchedCommandResponse::isOkSet() const {
-    return _isOkSet;
-}
-
 int BatchedCommandResponse::getOk() const {
     dassert(_isOkSet);
     return _ok;
diff --git a/src/mongo/s/write_ops/batched_command_response.h b/src/mongo/s/write_ops/batched_command_response.h
index 32ae02b9f62..bf2cb1c6f68 100644
--- a/src/mongo/s/write_ops/batched_command_response.h
+++ b/src/mongo/s/write_ops/batched_command_response.h
@@ -88,8 +88,6 @@ public:
     //
 
     void setOk(int ok);
-    void unsetOk();
-    bool isOkSet() const;
     int getOk() const;
 
     void setErrCode(int errCode);