Skip to content

Commit

Permalink
Fix #initial_set which is causing a double attempt and delay on loc…
Browse files Browse the repository at this point in the history
…k acquisition

The call to `#initial_set` in `#retry` and `#acquire_lock` is followed by `next` which leads to a second pass through the `#retry_with_timeout` loop and a sleep call for up to `:acquisition_delay`. This delay isn't necessary if the value can be set without a race condition.

Removing the `next` call causes the client to continue to retry because the transaction has been changed outside the transaction boundary:

In Redis, calling `SET` within a `WATCH`/`UNWATCH` block but not inside a `MULTI`/`EXEC` block will [cause the EXEC to fail the transaction](redis/redis-doc#734), so the first `#set` call fails and it requires a second pass. To resolve this I changed `#initial_set` to call `#set` within a `MULTI` block so that it would be inside the transaction.

In Memcache the call to `SET` without the `CAS` during `#initial_set` is going to cause the `SET` with `CAS` to fail (return `EXISTS`), and resulting in a second pass. To resolve this I changed `#initial_set` to use `SET` with `CAS` and return the CAS value to be used in the subsequent `#set` call that stores the lock token.
  • Loading branch information
jeremywadsack committed Aug 30, 2018
1 parent af1c476 commit a13edcf
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 9 deletions.
10 changes: 2 additions & 8 deletions lib/suo/client/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ def refresh(token)
retry_with_timeout do
val, cas = get

if val.nil?
initial_set
next
end
cas = initial_set if val.nil?

cleared_locks = deserialize_and_clear_locks(val)

Expand Down Expand Up @@ -101,10 +98,7 @@ def acquire_lock(token = nil)
retry_with_timeout do
val, cas = get

if val.nil?
initial_set
next
end
cas = initial_set if val.nil?

cleared_locks = deserialize_and_clear_locks(val)

Expand Down
2 changes: 2 additions & 0 deletions lib/suo/client/memcached.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def set(newval, cas)

def initial_set(val = BLANK_STR)
@client.set(@key, val)
_val, cas = @client.get_cas(@key)
cas
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/suo/client/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ def synchronize
end

def initial_set(val = BLANK_STR)
@client.set(@key, val)
set(val, nil)
nil
end
end
end
Expand Down

0 comments on commit a13edcf

Please sign in to comment.