Commit be3675c8 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Explain why ExclusiveLease has no #cancel

[ci skip]
parent ffe5ac41
...@@ -15,6 +15,25 @@ module Gitlab ...@@ -15,6 +15,25 @@ module Gitlab
# seconds then two overlapping operations may hold a lease for the same # seconds then two overlapping operations may hold a lease for the same
# key at the same time. # key at the same time.
# #
# This class has no 'cancel' method. I originally decided against adding
# it because it would add complexity and a false sense of security. The
# complexity: instead of setting '1' we would have to set a UUID, and to
# delete it we would have to execute Lua on the Redis server to only
# delete the key if the value was our own UUID. Otherwise there is a
# chance that when you intend to cancel your lease you actually delete
# someone else's. The false sense of security: you cannot design your
# system to rely too much on the lease being cancelled after use because
# the calling (Ruby) process may crash or be killed. You _cannot_ count
# on begin/ensure blocks to cancel a lease, because the 'ensure' does
# not always run. Think of 'kill -9' from the Unicorn master for
# instance.
#
# If you find that leases are getting in your way, ask yourself: would
# it be enough to lower the lease timeout? Another thing that might be
# appropriate is to only use a lease for bulk/automated operations, and
# to ignore the lease when you get a single 'manual' user request (a
# button click).
#
class ExclusiveLease class ExclusiveLease
def initialize(key, timeout:) def initialize(key, timeout:)
@key, @timeout = key, timeout @key, @timeout = key, timeout
...@@ -27,6 +46,8 @@ module Gitlab ...@@ -27,6 +46,8 @@ module Gitlab
!!redis.set(redis_key, '1', nx: true, ex: @timeout) !!redis.set(redis_key, '1', nx: true, ex: @timeout)
end end
# No #cancel method. See comments above!
private private
def redis def redis
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment