Rename ExpiringLock to ExclusiveLease

4 files changed, 39 insertions, 54 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bc8019193ee..15fee9948ec 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -246,7 +246,7 @@ class ApplicationController < ActionController::Base
 
   def ldap_security_check
     if current_user && current_user.requires_ldap_check?
-      return unless Gitlab::LDAP::Access.try_lock_user(user)
+      return unless Gitlab::LDAP::Access.try_lock_user(current_user)
 
       unless Gitlab::LDAP::Access.allowed?(current_user)
         sign_out current_user
diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb
new file mode 100644
index 00000000000..f801e8b60b3
--- /dev/null
+++ b/lib/gitlab/exclusive_lease.rb
@@ -0,0 +1,37 @@
+require 'securerandom'
+
+module Gitlab
+  # This class implements an 'exclusive lease'. We call it a 'lease'
+  # because it has a set expiry time. We call it 'exclusive' because only
+  # one caller may obtain a lease for a given key at a time. The
+  # implementation is intended to work across GitLab processes and across
+  # servers. It is a 'cheap' alternative to using SQL queries and updates:
+  # you do not need to change the SQL schema to start using
+  # ExclusiveLease.
+  class ExclusiveLease
+    def initialize(key, timeout)
+      @key, @timeout = key, timeout
+    end
+
+    # Try to obtain the lease. Return true on succes,
+    # false if the lease is already taken.
+    def try_obtain
+      !!redis.set(redis_key, redis_value, nx: true, ex: @timeout)
+    end
+
+    private
+
+    def redis
+      # Maybe someday we want to use a connection pool...
+      @redis ||= Redis.new(url: Gitlab::RedisConfig.url)
+    end
+
+    def redis_key
+      "gitlab:exclusive_lease:#{@key}"
+    end
+
+    def redis_value
+      @redis_value ||= SecureRandom.hex(10)
+    end
+  end
+end
diff --git a/lib/gitlab/expiring_lock.rb b/lib/gitlab/expiring_lock.rb
deleted file mode 100644
index ef77a24cf7d..00000000000
--- a/lib/gitlab/expiring_lock.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-module Gitlab
-  # This class implements a distributed self-expiring lock.
-  #
-  # [2] pry(main)> l = Gitlab::ExpiringLock.new('foobar', 5)
-  # => #<Gitlab::ExpiringLock:0x007ffb9d7cb7f8 @key="foobar", @timeout=5>
-  # [3] pry(main)> l.try_lock
-  # => true
-  # [4] pry(main)> l.try_lock # Only the first try_lock succeeds
-  # => false
-  # [5] pry(main)> l.locked?
-  # => true
-  # [6] pry(main)> sleep 5
-  # => 5
-  # [7] pry(main)> l.locked? # After the timeout the lock is released
-  # => false
-  #
-  class ExpiringLock
-    def initialize(key, timeout)
-      @key, @timeout = key, timeout
-    end
-
-    # Try to obtain the lock. Return true on succes,
-    # false if the lock is already taken.
-    def try_lock
-      # INCR does not change the key TTL
-      if redis.incr(redis_key) == 1
-        # We won the race to insert the key into Redis
-        redis.expire(redis_key, @timeout)
-        true
-      else
-        # Somebody else won the race
-        false
-      end
-    end
-
-    # Check if somebody somewhere locked this key
-    def locked?
-      !!redis.get(redis_key)
-    end
-
-    private
-
-    def redis
-      # Maybe someday we want to use a connection pool...
-      @redis ||= Redis.new(url: Gitlab::RedisConfig.url)
-    end
-
-    def redis_key
-      "gitlab:expiring_lock:#{@key}"
-    end
-  end
-end
diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb
index 29347c05b7d..76786169a49 100644
--- a/lib/gitlab/ldap/access.rb
+++ b/lib/gitlab/ldap/access.rb
@@ -10,7 +10,7 @@ module Gitlab
       LOCK_TIMEOUT = 600
 
       def self.try_lock_user(user)
-        Gitlab::ExpiringLock.new("user_ldap_check:#{user.id}", LOCK_TIMEOUT).try_lock
+        Gitlab::ExclusiveLease.new("user_ldap_check:#{user.id}", LOCK_TIMEOUT).try_obtain
       end
 
       def self.open(user, &block)