From cda0b7e1b142130fd2e4d7073e451a3f8d73b693 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer 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) - # => # - # [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