Revert "Don't check the time more than needed when locking"

This reverts commit 7e2937e75e84c8cb27a22488a202cf4411977c6a.

10 files changed, 61 insertions, 91 deletions

diff --git a/src/mongo/db/catalog/index_catalog_entry.cpp b/src/mongo/db/catalog/index_catalog_entry.cpp
index 7e0624846d6..7028303c2d1 100644
--- a/src/mongo/db/catalog/index_catalog_entry.cpp
+++ b/src/mongo/db/catalog/index_catalog_entry.cpp
@@ -151,7 +151,7 @@ namespace mongo {
         }
 
         const ResourceId collRes = ResourceId(RESOURCE_COLLECTION, _ns);
-        LockResult res = txn->lockState()->lock(collRes, MODE_X, NULL, true);
+        LockResult res = txn->lockState()->lock(collRes, MODE_X, UINT_MAX, true);
         if (res == LOCK_DEADLOCK) throw WriteConflictException();
         invariant(res == LOCK_OK);
 
diff --git a/src/mongo/db/concurrency/d_concurrency.cpp b/src/mongo/db/concurrency/d_concurrency.cpp
index 82effb17668..ac5e466efd1 100644
--- a/src/mongo/db/concurrency/d_concurrency.cpp
+++ b/src/mongo/db/concurrency/d_concurrency.cpp
@@ -113,11 +113,10 @@ namespace mongo {
     }
 
     void Lock::ScopedLock::resetTime() {
-        // TODO fill in or delete
+        _timer.reset();
     }
 
     void Lock::ScopedLock::recordTime() {
-        // TODO fill in or delete
     }
 
     void Lock::ScopedLock::_tempRelease() {
@@ -174,10 +173,10 @@ namespace mongo {
         resetTime();
     }
 
-    Lock::GlobalWrite::GlobalWrite(Locker* lockState, Microseconds* timeBudgetRemaining)
+    Lock::GlobalWrite::GlobalWrite(Locker* lockState, unsigned timeoutms)
         : ScopedLock(lockState, 'W') {
 
-        LockResult result = _lockState->lockGlobal(MODE_X, timeBudgetRemaining);
+        LockResult result = _lockState->lockGlobal(MODE_X, timeoutms);
         if (result == LOCK_TIMEOUT) {
             throw DBTryLockTimeoutException();
         }
@@ -193,10 +192,10 @@ namespace mongo {
         recordTime();
     }
 
-    Lock::GlobalRead::GlobalRead(Locker* lockState, Microseconds* timeBudgetRemaining)
+    Lock::GlobalRead::GlobalRead(Locker* lockState, unsigned timeoutms)
         : ScopedLock(lockState, 'R') {
 
-        LockResult result = _lockState->lockGlobal(MODE_S, timeBudgetRemaining);
+        LockResult result = _lockState->lockGlobal(MODE_S, timeoutms);
         if (result == LOCK_TIMEOUT) {
             throw DBTryLockTimeoutException();
         }
@@ -318,8 +317,7 @@ namespace mongo {
         _dbwlock( NULL )
     { 
         try { 
-            Microseconds timeBudget(tryms * 1000);
-            _dbwlock.reset(new Lock::GlobalWrite(lockState, &timeBudget));
+            _dbwlock.reset(new Lock::GlobalWrite(lockState, tryms));
         }
         catch ( DBTryLockTimeoutException & ) {
             return;
@@ -337,8 +335,7 @@ namespace mongo {
         _dbrlock( NULL )
     {
         try { 
-            Microseconds timeBudget(tryms * 1000);
-            _dbrlock.reset(new Lock::GlobalRead(lockState, &timeBudget));
+            _dbrlock.reset(new Lock::GlobalRead(lockState, tryms));
         }
         catch ( DBTryLockTimeoutException & ) {
             return;
diff --git a/src/mongo/db/concurrency/d_concurrency.h b/src/mongo/db/concurrency/d_concurrency.h
index fd71d31926d..9ae00cf2db4 100644
--- a/src/mongo/db/concurrency/d_concurrency.h
+++ b/src/mongo/db/concurrency/d_concurrency.h
@@ -32,6 +32,7 @@
 #pragma once
 
 #include <boost/scoped_ptr.hpp>
+#include <climits> // For UINT_MAX
 
 #include "mongo/base/string_data.h"
 #include "mongo/db/concurrency/lock_mgr_defs.h"
@@ -116,6 +117,7 @@ namespace mongo {
 
             ParallelBatchWriterSupport _pbws_lk;
 
+            Timer _timer;
             char _type;      // 'r','w','R','W'
         };
 
@@ -131,8 +133,8 @@ namespace mongo {
             void _tempRelease();
             void _relock();
         public:
-            // timeBudgetRemaining is only for writelocktry -- deprecated -- do not use
-            GlobalWrite(Locker* lockState, Microseconds* timeBudgetRemaining = NULL);
+            // timeoutms is only for writelocktry -- deprecated -- do not use
+            GlobalWrite(Locker* lockState, unsigned timeoutms = UINT_MAX);
             virtual ~GlobalWrite();
         };
 
@@ -145,8 +147,8 @@ namespace mongo {
          */
         class GlobalRead : public ScopedLock {
         public:
-            // timeBudgetRemaining is only for readlocktry -- deprecated -- do not use
-            GlobalRead(Locker* lockState, Microseconds* timeBudgetRemaining = NULL);
+            // timeoutms is only for readlocktry -- deprecated -- do not use
+            GlobalRead(Locker* lockState, unsigned timeoutms = UINT_MAX);
             virtual ~GlobalRead();
         };
 
diff --git a/src/mongo/db/concurrency/lock_state.cpp b/src/mongo/db/concurrency/lock_state.cpp
index 3a5421ba0b9..a0db0a69082 100644
--- a/src/mongo/db/concurrency/lock_state.cpp
+++ b/src/mongo/db/concurrency/lock_state.cpp
@@ -57,8 +57,8 @@ namespace mongo {
         // on its use.
         const ResourceId resourceIdMMAPV1Flush = ResourceId(RESOURCE_MMAPV1_FLUSH, 2ULL);
 
-        // How often to check for deadlock if a lock has not been granted for some time
-        const Microseconds DeadlockTimeout(100*1000);
+        // How often (in millis) to check for deadlock if a lock has not been granted for some time
+        const unsigned DeadlockTimeoutMs = 100;
 
         /**
          * Used to sort locks by granularity when snapshotting lock state. We must report and
@@ -263,9 +263,7 @@ namespace mongo {
         _result = LOCK_INVALID;
     }
 
-    LockResult CondVarLockGrantNotification::wait(int timeoutMs) {
-        if (timeoutMs <= 0) return LOCK_TIMEOUT;
-
+    LockResult CondVarLockGrantNotification::wait(unsigned timeoutMs) {
         boost::unique_lock<boost::mutex> lock(_mutex);
         while (_result == LOCK_INVALID) {
             if (!_cond.timed_wait(lock, Milliseconds(timeoutMs))) {
@@ -312,12 +310,15 @@ namespace mongo {
     }
 
     template<bool IsForMMAPV1>
-    LockResult LockerImpl<IsForMMAPV1>::lockGlobal(LockMode mode,
-                                                   Microseconds* timeBudgetRemaining) {
+    LockResult LockerImpl<IsForMMAPV1>::lockGlobal(LockMode mode, unsigned timeoutMs) {
         LockRequestsMap::Iterator it = _requests.find(resourceIdGlobal);
         if (!it) {
             // Global lock should be the first lock on any operation
             invariant(_requests.empty());
+
+            // Start counting time since first global lock acquisition (that's when effectively
+            // any timing for the locker counts from).
+            _timer.reset();
         }
         else {
             // No upgrades on the GlobalLock are currently necessary. Should not be used until we
@@ -325,7 +326,7 @@ namespace mongo {
             invariant(it->mode >= mode);
         }
 
-        LockResult globalLockResult = lock(resourceIdGlobal, mode, timeBudgetRemaining);
+        LockResult globalLockResult = lock(resourceIdGlobal, mode, timeoutMs);
         if (globalLockResult != LOCK_OK) {
             invariant(globalLockResult == LOCK_TIMEOUT);
 
@@ -334,8 +335,13 @@ namespace mongo {
 
         // Special-handling for MMAP V1 concurrency control
         if (IsForMMAPV1 && !it) {
+            // Obey the requested timeout
+            const unsigned elapsedTimeMs = _timer.millis();
+            const unsigned remainingTimeMs =
+                elapsedTimeMs < timeoutMs ? (timeoutMs - elapsedTimeMs) : 0;
+
             LockResult flushLockResult =
-                lock(resourceIdMMAPV1Flush, _getModeForMMAPV1FlushLock(), timeBudgetRemaining);
+                lock(resourceIdMMAPV1Flush, _getModeForMMAPV1FlushLock(), remainingTimeMs);
 
             if (flushLockResult != LOCK_OK) {
                 invariant(flushLockResult == LOCK_TIMEOUT);
@@ -404,7 +410,7 @@ namespace mongo {
 
             while (true) {
                 LockResult result =
-                    lock(resourceIdMMAPV1Flush, _getModeForMMAPV1FlushLock(), NULL, true);
+                    lock(resourceIdMMAPV1Flush, _getModeForMMAPV1FlushLock(), UINT_MAX, true);
 
                 if (result == LOCK_OK) break;
 
@@ -416,7 +422,7 @@ namespace mongo {
     template<bool IsForMMAPV1>
     LockResult LockerImpl<IsForMMAPV1>::lock(const ResourceId& resId,
                                              LockMode mode,
-                                             Microseconds* timeBudgetRemaining,
+                                             unsigned timeoutMs,
                                              bool checkDeadlock) {
 
         LockResult result = lockImpl(resId, mode);
@@ -428,10 +434,8 @@ namespace mongo {
         // unsuccessful result that the lock manager would return is LOCK_WAITING.
         invariant(result == LOCK_WAITING);
 
-        if (timeBudgetRemaining && *timeBudgetRemaining <= Microseconds(0)) return LOCK_TIMEOUT;
-
         // Tracks the lock acquisition time if we don't obtain the lock immediately
-        Timer timer(timeBudgetRemaining ? Timer::START : Timer::DONT_START);
+        Timer timer;
 
         // Under MMAP V1 engine a deadlock can occur if a thread goes to sleep waiting on
         // DB lock, while holding the flush lock, so it has to be released. This is only
@@ -446,12 +450,9 @@ namespace mongo {
 
         // Don't go sleeping without bound in order to be able to report long waits or wake up for
         // deadlock detection.
-        Microseconds timeToWait = DeadlockTimeout;
-        if (timeBudgetRemaining && *timeBudgetRemaining < timeToWait)
-            timeToWait = *timeBudgetRemaining;
-
-        for (int attempt = 0; /*forever*/; attempt++) {
-            result = _notify.wait(timeToWait.total_milliseconds());
+        unsigned waitTimeMs = std::min(timeoutMs, DeadlockTimeoutMs);
+        while (true) {
+            result = _notify.wait(waitTimeMs);
 
             if (result == LOCK_OK) break;
 
@@ -465,21 +466,17 @@ namespace mongo {
                 }
             }
 
-            if (timeBudgetRemaining) {
-                *timeBudgetRemaining -= Microseconds(timer.microsReset());
-                if (*timeBudgetRemaining < timeToWait) {
-                    timeToWait = *timeBudgetRemaining;
-                }
-            }
+            const unsigned elapsedTimeMs = timer.millis();
+            waitTimeMs = (elapsedTimeMs < timeoutMs) ?
+                std::min(timeoutMs - elapsedTimeMs, DeadlockTimeoutMs) : 0;
 
-            if (timeToWait <= Microseconds(0)) {
+            if (waitTimeMs == 0) {
                 break;
             }
 
             // This will occasionally dump the global lock manager in case lock acquisition is
             // taking too long.
-            static const int attemptsPerLog = 30*1000*1000 / DeadlockTimeout.total_microseconds();
-            if ((attempt % attemptsPerLog) == 0) {
+            if (elapsedTimeMs > 30000U) {
                 dumpGlobalLockManagerAndCallstackThrottled(this);
             }
         }
@@ -499,8 +496,6 @@ namespace mongo {
             invariant(LOCK_OK == lock(resourceIdMMAPV1Flush, _getModeForMMAPV1FlushLock()));
         }
 
-        if (timeBudgetRemaining) *timeBudgetRemaining -= Microseconds(timer.micros());
-
         return result;
     }
 
diff --git a/src/mongo/db/concurrency/lock_state.h b/src/mongo/db/concurrency/lock_state.h
index 00e31336121..8ccf18aa297 100644
--- a/src/mongo/db/concurrency/lock_state.h
+++ b/src/mongo/db/concurrency/lock_state.h
@@ -55,7 +55,7 @@ namespace mongo {
          *
          * @param timeoutMs How many milliseconds to wait before returning LOCK_TIMEOUT.
          */
-        LockResult wait(int timeoutMs);
+        LockResult wait(unsigned timeoutMs);
 
     private:
 
@@ -95,7 +95,7 @@ namespace mongo {
 
         virtual LockerId getId() const { return _id; }
 
-        virtual LockResult lockGlobal(LockMode mode, Microseconds* timeBudgetRemaining = NULL);
+        virtual LockResult lockGlobal(LockMode mode, unsigned timeoutMs = UINT_MAX);
         virtual void downgradeGlobalXtoSForMMAPV1();
         virtual bool unlockAll();
 
@@ -106,7 +106,7 @@ namespace mongo {
 
         virtual LockResult lock(const ResourceId& resId,
                                 LockMode mode, 
-                                Microseconds* timeBudgetRemaining = NULL,
+                                unsigned timeoutMs = UINT_MAX,
                                 bool checkDeadlock = false);
 
         virtual void downgrade(const ResourceId& resId, LockMode newMode);
@@ -176,6 +176,9 @@ namespace mongo {
         int _wuowNestingLevel;
         std::queue<ResourceId> _resourcesToUnlockAtEndOfUnitOfWork;
 
+        // For maintaining locking timing statistics
+        Timer _timer;
+
 
         //////////////////////////////////////////////////////////////////////////////////////////
         //
diff --git a/src/mongo/db/concurrency/lock_state_test.cpp b/src/mongo/db/concurrency/lock_state_test.cpp
index ee5365970cf..2f23711c76c 100644
--- a/src/mongo/db/concurrency/lock_state_test.cpp
+++ b/src/mongo/db/concurrency/lock_state_test.cpp
@@ -81,8 +81,7 @@ namespace mongo {
 
         MMAPV1LockerImpl locker2(2);
         ASSERT(LOCK_OK == locker2.lockGlobal(MODE_IX));
-        Microseconds timeBudget(0);
-        ASSERT(LOCK_TIMEOUT == locker2.lock(resId, MODE_S, &timeBudget));
+        ASSERT(LOCK_TIMEOUT == locker2.lock(resId, MODE_S, 0));
 
         ASSERT(locker2.isLockHeldForMode(resId, MODE_NONE));
 
@@ -104,8 +103,7 @@ namespace mongo {
         ASSERT(LOCK_OK == locker2.lock(resId, MODE_S));
 
         // Try upgrading locker 1, which should block and timeout
-        Microseconds timeBudget(1);
-        ASSERT(LOCK_TIMEOUT == locker1.lock(resId, MODE_X, &timeBudget));
+        ASSERT(LOCK_TIMEOUT == locker1.lock(resId, MODE_X, 1));
 
         locker1.unlockAll();
         locker2.unlockAll();
diff --git a/src/mongo/db/concurrency/locker.h b/src/mongo/db/concurrency/locker.h
index b8d768d1d76..c5d6e237da5 100644
--- a/src/mongo/db/concurrency/locker.h
+++ b/src/mongo/db/concurrency/locker.h
@@ -28,6 +28,7 @@
 
 #pragma once
 
+#include <climits> // For UINT_MAX
 #include <vector>
 
 #include "mongo/base/disallow_copying.h"
@@ -66,19 +67,14 @@ namespace mongo {
          *
          * @param mode Mode in which the global lock should be acquired. Also indicates the intent
          *              of the operation.
-         * @param timeBudgetRemaining If NULL, wait indefinitely for the lock.
-         *                            If not NULL, we will return LOCK_TIMEOUT if it takes longer
-         *                            then *timeBudgetRemaining, and *timeBudgetRemaining will be
-         *                            decremented by the time spent waiting for locks. This allows
-         *                            you to acquire several locks in sequence using the same
-         *                            timeout. If *timeBudgetRemaining is <= 0, returns immediately
-         *                            if the lock couldn't be taken without blocking.
+         * @param timeoutMs How long to wait for the global lock (and the flush lock, for the MMAP
+         *          V1 engine) to be acquired.
          *
          * @return LOCK_OK, if the global lock (and the flush lock, for the MMAP V1 engine) were
          *          acquired within the specified time bound. Otherwise, the respective failure
          *          code and neither lock will be acquired.
          */
-        virtual LockResult lockGlobal(LockMode mode, Microseconds* timeBudgetRemaining = NULL) = 0;
+        virtual LockResult lockGlobal(LockMode mode, unsigned timeoutMs = UINT_MAX) = 0;
 
         /**
          * Decrements the reference count on the global lock.  If the reference count on the
@@ -122,13 +118,10 @@ namespace mongo {
          *
          * @param resId Id of the resource to be locked.
          * @param mode Mode in which the resource should be locked. Lock upgrades are allowed.
-         * @param timeBudgetRemaining If NULL, wait indefinitely for the lock.
-         *                            If not NULL, we will return LOCK_TIMEOUT if it takes longer
-         *                            then *timeBudgetRemaining, and *timeBudgetRemaining will be
-         *                            decremented by the time spent waiting for locks. This allows
-         *                            you to acquire several locks in sequence using the same
-         *                            timeout. If *timeBudgetRemaining is <= 0, returns immediately
-         *                            if the lock couldn't be taken without blocking.
+         * @param timeoutMs How many milliseconds to wait for the lock to be granted, before
+         *              returning LOCK_TIMEOUT. This parameter defaults to UINT_MAX, which means
+         *              wait infinitely. If 0 is passed, the request will return immediately, if
+         *              the request could not be granted right away.
          * @param checkDeadlock Whether to enable deadlock detection for this acquisition. This
          *              parameter is put in place until we can handle deadlocks at all places,
          *              which acquire locks.
@@ -137,7 +130,7 @@ namespace mongo {
          */
         virtual LockResult lock(const ResourceId& resId,
                                 LockMode mode,
-                                Microseconds* timeBudgetRemaining = NULL,
+                                unsigned timeoutMs = UINT_MAX,
                                 bool checkDeadlock = false) = 0;
 
         /**
diff --git a/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp b/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
index f36e406820b..7a75b607182 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
@@ -254,8 +254,7 @@ namespace {
 
         virtual bool try_lock(OperationContext* txn, const Milliseconds& timeout) {
             try {
-                Microseconds timeoutLocal(timeout.total_microseconds());
-                _rlock.reset(new Lock::GlobalRead(txn->lockState(), &timeoutLocal));
+                _rlock.reset(new Lock::GlobalRead(txn->lockState(), timeout.total_milliseconds()));
             }
             catch (const DBTryLockTimeoutException&) {
                 return false;
diff --git a/src/mongo/util/time_support.h b/src/mongo/util/time_support.h
index 8d7c5ad2e88..f7e62476eb8 100644
--- a/src/mongo/util/time_support.h
+++ b/src/mongo/util/time_support.h
@@ -41,7 +41,6 @@
 
 namespace mongo {
 
-    typedef boost::posix_time::microseconds Microseconds;
     typedef boost::posix_time::milliseconds Milliseconds;
     typedef boost::posix_time::seconds Seconds;
 
diff --git a/src/mongo/util/timer.h b/src/mongo/util/timer.h
index e5f1a143b3c..b28db52d703 100644
--- a/src/mongo/util/timer.h
+++ b/src/mongo/util/timer.h
@@ -30,9 +30,9 @@
 #pragma once
 
 #include "mongo/client/export_macros.h"
-#include "mongo/util/assert_util.h"
 
 namespace mongo {
+
     /**
      * Time tracking object.
      *
@@ -50,12 +50,7 @@ namespace mongo {
         static const long long microsPerSecond = 1000 * millisPerSecond;
         static const long long nanosPerSecond = 1000 * microsPerSecond;
 
-        enum ShouldStart { START = true, DONT_START = false };
-
-        Timer(ShouldStart shouldStart = START) : _old(shouldStart ? now() : unstartedTime) {}
-
-        bool isStarted() const { return _old != unstartedTime; }
-
+        Timer() { reset(); }
         int seconds() const { return (int)(micros() / 1000000); }
         int millis() const { return (int)(micros() / 1000); }
         int minutes() const { return seconds() / 60; }
@@ -65,24 +60,15 @@ namespace mongo {
          *  @return time in milliseconds.
          */
         inline int millisReset() {
-            return static_cast<int>(microsReset() / 1000);
-        }
-
-        /** Get the time interval and reset at the same time.
-         *  @return time in microseconds.
-         */
-        inline long long microsReset() {
-            dassert(isStarted());
             const long long nextNow = now();
             const long long deltaMicros =
                 ((nextNow - _old) * microsPerSecond) / _countsPerSecond;
 
             _old = nextNow;
-            return deltaMicros;
+            return static_cast<int>(deltaMicros / 1000);
         }
 
         inline long long micros() const {
-            dassert(isStarted());
             return ((now() - _old) * microsPerSecond) / _countsPerSecond;
         }
 
@@ -99,8 +85,6 @@ namespace mongo {
         static long long _countsPerSecond;
 
     private:
-        static const long long unstartedTime = -1;
-
         long long now() const;
 
         long long _old;