From a13edcf7d12b70be5f060b9de2d8342d1189db0e Mon Sep 17 00:00:00 2001 From: Jeremy Wadsack Date: Thu, 30 Aug 2018 13:06:11 -0700 Subject: [PATCH] Fix `#initial_set` which is causing a double attempt and delay on lock 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](https://github.com/antirez/redis-doc/issues/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. --- lib/suo/client/base.rb | 10 ++-------- lib/suo/client/memcached.rb | 2 ++ lib/suo/client/redis.rb | 3 ++- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/suo/client/base.rb b/lib/suo/client/base.rb index 7f37355..9f1a52b 100644 --- a/lib/suo/client/base.rb +++ b/lib/suo/client/base.rb @@ -55,10 +55,7 @@ module Suo 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) @@ -101,10 +98,7 @@ module Suo 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) diff --git a/lib/suo/client/memcached.rb b/lib/suo/client/memcached.rb index 83b37b6..c54ad79 100644 --- a/lib/suo/client/memcached.rb +++ b/lib/suo/client/memcached.rb @@ -22,6 +22,8 @@ module Suo def initial_set(val = BLANK_STR) @client.set(@key, val) + _val, cas = @client.get_cas(@key) + cas end end end diff --git a/lib/suo/client/redis.rb b/lib/suo/client/redis.rb index b13119e..657b482 100644 --- a/lib/suo/client/redis.rb +++ b/lib/suo/client/redis.rb @@ -35,7 +35,8 @@ module Suo end def initial_set(val = BLANK_STR) - @client.set(@key, val) + set(val, nil) + nil end end end