using uuid-$hostname-$pid replace default uuid as lock token. #217
Conversation
|
@sibson Do you have time to look at this pull request? We'd like to use this feature as soon as possible. Many thanks! |
|
This seems like a good idea. If I'm understanding it correctly it means we're sticking machine identifying information into redis. I don't see any obvious problems with that but thought I'd mention in case someone sees an issue with doing so. It looks like there are some lint errors, if you clean those up and no one objects I'll merge. |
|
@sibson Lint errors already fixed, pls review. |
|
Can we merge now? @sibson |
|
Is there a specific waiting time? We would like to know approximately when we can merge and release a new version Many Thanks! |
|
A month has passed and it looks like no one has voiced any objections, can we merge now? @sibson Many Thanks! |
| @@ -163,13 +165,20 @@ def __init__(self, app=None): | |||
| self.schedule_key = self.key_prefix + ':schedule' | |||
| self.statics_key = self.key_prefix + ':statics' | |||
| self.lock_key = self.either_or('redbeat_lock_key', self.key_prefix + ':lock') | |||
| self.lock_token = self.generate_lock_token() | |||
There was a problem hiding this comment.
I think this token should be generated at acquire time to keep the same semantics we currently have. Also I would guess including the token in the info log below the acquire line would be good since ideally you would want to be able to check your logs to figure out which host has the lock rather than accessing redis for that information.
Ideally we'd also want to move towards using the stock redis-py Lock implementation since it now has a reacquire method that has the behaviour we're overriding extend with.
We run multiple redbeat instances for high availability, and sometimes we find that the service is not behaving as expected, we want to know which server is running, but it's hard to know right now, so we want to use uuid-$hostname-$pid as a lock token to aid troubleshooting.