propose implementation of extend

[seamusabshere]

per @antirez this needs a theoretical review - would @maltoe and/or @leandromoreira or @antirez himself help me with that?

thanks to @sbertrang for original perl implementation https://github.com/sbertrang/redis-distlock and @biancalana for value-checking sbertrang/redis-distlock#2

[maltoe]

The "XX" is not needed here as the line before made sure the key exists.

Instead of carrying around your extend variable through the various lock methods, I'd suggest combining the SETNX command for lock and the extend script into one:

def lock(resource, ttl, extend: {}, &block)
  value = extend[:value] || SecureRandom.uuid
  lock_info = try_lock_instances resource, ttl, value
  [...]
end

LOCK_SCRIPT = <<-eos
if redis.call("exists",KEYS[1]) == 0 or redis.call("get",KEYS[1]) == ARGV[1] then
  return redis.call("set",KEYS[1],ARGV[1],"PX",ARGV[2])
end
eos
[...]

Executing a script for lock() each time is presumable worse in terms of performance than a SETNX command, but I'd still prefer this for readability.

[seamusabshere]

@antirez do you prefer always calling a script or choosing between a script and SETNX?

[maltoe]

@seamusabshere Thinking about it, unifying it into a single script sure changes the semantic of the algorithm, as "extending" could become a combination of "lock extended" (i.e. lock was still there with our value) and "new lock set" (key was expired). Not sure about the consequences though.

[maltoe]

@seamusabshere Unfortunately I can't provide you any input on algorithm correctness, hope @antirez can help us out here.

[leandromoreira]

off topic: should we release a new version @maltoe ?

[maltoe]

@leandromoreira Sure why not

[maltoe]

@antirez You have any comments on this? Do we violate any guarantees with this?

@leandromoreira Otherwise, should we proceed and merge this? I'm pretty sure the patch does not change anything to the original algorithm as long as one's not using extend.

[leandromoreira]

@maltoe I agree but let's wait a week to see if @antirez comments anything otherwise we merge. Thanks @seamusabshere and sorry to make you wait so long

[leandromoreira]

thanks to @seamusabshere and @maltoe =)

[leandromoreira]

@seamusabshere I tried to merge but github states:

This branch has conflicts that must be resolved

[maltoe]

Please see #24