Conversation
5f8a4a6 to
c9af502
Compare
c359531 to
dc7ca9e
Compare
|
Hello @palcalde, sorry for taking a while to get back to you. I understand your use case and see that what you are proposing is a valid option. However, this means by default the lock behaves a little different to what the algorithm described here: https://redis.io/docs/manual/patterns/distributed-locks/#making-the-algorithm-more-reliable-extending-the-lock Would you find it reasonable that this behavior is made available through an option on the NewMutex function? rs.NewMutex("dont-spin", WithSetNXOnExtend(true))That way Redsync will continue to behave how it does now by default and stay in line with the algorithm described on redis.io. But you can use this new option to make it work with your setup. |
Yes, makes sense to have it as an option. I'll make the changes required and let you know. Thank you! |
|
Changes done. Now is configured by the option |
|
@palcalde Thank you! |
|
Thank you! Do you mind doing a release? I think upgrading a minor should be fine, there is no breaking change. |
|
@palcalde That should be done already... https://github.com/go-redsync/redsync/tree/v4.12.0 Let me know if you are facing any issue with it. |
|
Thank you for the quick response. However, I had to do a fix in #151. One of my changes caused the extend to release the lock even if there was quorum for it. |
Hello!
We are heavy users of this library and it works great, however, there is a small improvement that would alleviate some pain in our case. Let me explain it:
We realized we are losing the lock too often, and the reason is this:
If one of them gets restarted or unavailable for some time (it happens), our service locks in two redises instead of 3 since it's enough for the quorum. That's expected, but the problem is that from now on we are extending with 2 redises instead of 3 (we can't extend the one that got restarted cause it's empty, no key there). It's now a matter of time until one of the two we use to extend gets restarted as well, we then lose all locks in that redis, as all operations to extend fail because there is only one instance with keys to be extended, the other two don't have them.
This MR fixes that by trying to "extend or lock" instead of just "extend". This way if one of the redis restarts we'll sync up quickly the keys and all of them will be operative for extending. The only way to lose a lock now is if two redises get restarted at the same time, which is very unlikely.
Let me know if this makes sense.
Thank you!