Optimize sleeping while blocking for a lock

When waiting to acquire a lock, the Lock object will sleep until
the lock is acquired or until blocking_timeout has elapsed. This optimization
calculates whether the next iteration will occur after blocking_timeout
has elapsed and short circuits it immediately.

Fixes #1263

3 files changed, 23 insertions, 9 deletions

diff --git a/CHANGES b/CHANGES
index 2ca755d..5cbc7ca 100644
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,10 @@
       would be hidden from the user. Thanks @jdufresne. #1281
     * Expanded support for connection strings specifying a username connecting
       to pre-v6 servers. #1274
+    * Optimized Lock's blocking_timeout and sleep. If the lock cannot be
+      acquired and the sleep value would cause the loop to sleep beyond
+      blocking_timeout, fail immediately. Thanks @clslgrnc. #1263
+
 * 3.4.1
     * Move the username argument in the Redis and Connection classes to the
       end of the argument list. This helps those poor souls that specify all
diff --git a/redis/lock.py b/redis/lock.py
index 8d481d7..2f5aa27 100644
--- a/redis/lock.py
+++ b/redis/lock.py
@@ -124,8 +124,6 @@ class Lock(object):
         self.thread_local = bool(thread_local)
         self.local = threading.local() if self.thread_local else dummy()
         self.local.token = None
-        if self.timeout and self.sleep > self.timeout:
-            raise LockError("'sleep' must be less than 'timeout'")
         self.register_scripts()
 
     def register_scripts(self):
@@ -184,7 +182,8 @@ class Lock(object):
                 return True
             if not blocking:
                 return False
-            if stop_trying_at is not None and mod_time.time() > stop_trying_at:
+            next_try_at = mod_time.time() + sleep
+            if stop_trying_at is not None and next_try_at > stop_trying_at:
                 return False
             mod_time.sleep(sleep)
 
diff --git a/tests/test_lock.py b/tests/test_lock.py
index f92afec..8b2cc5b 100644
--- a/tests/test_lock.py
+++ b/tests/test_lock.py
@@ -91,10 +91,13 @@ class TestLock(object):
     def test_blocking_timeout(self, r):
         lock1 = self.get_lock(r, 'foo')
         assert lock1.acquire(blocking=False)
-        lock2 = self.get_lock(r, 'foo', blocking_timeout=0.2)
+        bt = 0.2
+        sleep = 0.05
+        lock2 = self.get_lock(r, 'foo', sleep=sleep, blocking_timeout=bt)
         start = time.time()
         assert not lock2.acquire()
-        assert (time.time() - start) > 0.2
+        # The elapsed duration should be less than the total blocking_timeout
+        assert bt > (time.time() - start) > bt - sleep
         lock1.release()
 
     def test_context_manager(self, r):
@@ -110,10 +113,18 @@ class TestLock(object):
             with self.get_lock(r, 'foo', blocking_timeout=0.1):
                 pass
 
-    def test_high_sleep_raises_error(self, r):
-        "If sleep is higher than timeout, it should raise an error"
-        with pytest.raises(LockError):
-            self.get_lock(r, 'foo', timeout=1, sleep=2)
+    def test_high_sleep_small_blocking_timeout(self, r):
+        lock1 = self.get_lock(r, 'foo')
+        assert lock1.acquire(blocking=False)
+        sleep = 60
+        bt = 1
+        lock2 = self.get_lock(r, 'foo', sleep=sleep, blocking_timeout=bt)
+        start = time.time()
+        assert not lock2.acquire()
+        # the elapsed timed is less than the blocking_timeout as the lock is
+        # unattainable given the sleep/blocking_timeout configuration
+        assert bt > (time.time() - start)
+        lock1.release()
 
     def test_releasing_unlocked_lock_raises_error(self, r):
         lock = self.get_lock(r, 'foo')