propose a heartbeat implementation

[seamusabshere]

postgres advisory locks are nice because if the database connection (session) dies, the lock goes away.

since you can't do that with redis, here's a heartbeat proposal to extend redlock.

[josegonzalez]

Seems like this needs to be rebased?

[seamusabshere]

@josegonzalez fixed, thanks!

[maltoe]

Hey @seamusabshere,

thanks for the proposal. Though, there's a bad race condition in it since you start the heartbeat only after all lock keys have been set, so a second Redis process might delete them already before the first one starts the heartbeat, and in the end both may think they own the lock. You would have to make sure you set your heartbeat key at the same time with the key itself (in a MULTI transaction or the like), or probably use the same set-if-not-set logic the main lock key uses (SETNX command). Not sure what else would be required to make this work properly, would have to take a deeper look into it. You should also probably not discard the Thread objects.

Generally speaking, not sure what to think about this. Imho it adds a lot of complexity to the algorithm itself (new shared data value, ...) that we might not completely understand, for little benefit. Couldn't the same thing basically be implemented on higher level by simply re-newing locks from time to time (in contrast to having a long-lived lock and a heartbeat)? Maybe we could add an extend method here somewhere that resets key expiration dates for already acquired locks.

[antirez]

Hello, my opinion is similar to @maltoe, if you read the Redlock specification, you'll see that there are already hints about how to extend the algorithm in order to refresh a lock in a safe way. Using this approach, then it's up to the specific implementation to do this in a different thread in a heartbeat-alike fashion, or alternatively, if from time to time while doing work refresh the lock.

So the first step is to formalize in a better way the process of lock renewal, but should not be too hard. However note that renewing the lock for more than a given amount of time, breaks the assumption that locks have a maximum time of T, so that restarting the servers with a delay of 2T will be enough to ensure the safety property without fsyncing on disk.

[seamusabshere]

got it, working on a patch based on https://github.com/sbertrang/redis-distlock 's implementation of extend

[antirez]

Thanks @seamusabshere, the first step is actually to analyze the protocol to extend the lock and check if there are the same safety guarantees. Check how this plays in the context of server restarting when persistence is turned off. Later we can write code. With distributed algorithms writing the code is the simplest step.