Retry locking if wait fails within timeout. Reset ttl after wait. #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This implies: - Resetting the TTL requires knowing the key before we enter the pipeline, which means we cant have it generated by redis during the aquireLock anymore. So that needs to be generated client-side now. - the acquireScript needs to extend the TTL if it's already owned, since we may be re-trying after a failed WAIT. At which point we may or may not have the lock in the current master already. And we'll need to be sure that the ttl will survive the next WAIT. This implicitly gives aquireLock the ability to extend a lock.
Also use a unique key for each test, so that one test failing does not break the others
|
Hey, sorry, I already reverted the readme etc changes but forgot to push and now it is out of sync. Could you fix the conflicts? Closing #2 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As mentioned in the other PRs discussion, the library could re-attempt to get a lock if synchronization fails within replicationTimeout. This is implemented in this PR.
Also I realized there was a flaw, which is that while waiting for replication, the already acquired lock in the primary is loosing it's time-to-live and may actually already have completely expired before the WAIT finishes. To mitigate that:
This implies:
pipeline, which means we can't have it generated by redis during the
aquireLock anymore. It needs to be generated client-side now, which I do by a cheap time + random.
we may be re-trying after a failed WAIT. At which point we may or may
not have the lock in the current master already. And we'll need to be
sure that the ttl will survive the next WAIT. This implicitly gives
aquireLock the ability to extend a lock.
Because acquireLock now actually extends a previously held lock, the same implementation is now used for the extendLock function, the difference being that extendLock requires to have the key in advance.