From cda0b7e1b142130fd2e4d7073e451a3f8d73b693 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <contact@jacobvosmaer.nl>
Date: Thu, 10 Mar 2016 10:41:16 +0100
Subject: [PATCH] Rename ExpiringLock to ExclusiveLease
---
app/controllers/application_controller.rb | 2 +-
lib/gitlab/exclusive_lease.rb | 37 ++++++++++++++++
lib/gitlab/expiring_lock.rb | 52 -----------------------
lib/gitlab/ldap/access.rb | 2 +-
4 files changed, 39 insertions(+), 54 deletions(-)
create mode 100644 lib/gitlab/exclusive_lease.rb
delete mode 100644 lib/gitlab/expiring_lock.rb
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)
--
GitLab