-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatic lock release - fixes #21 #22
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I added a few comments. I'm still trying to wrap my head around how this would work and possible edge cases.
This covers only the case where a task is currently being executed by a worker?
What about when you have a task pending in the queue already? Would this determine the task is not active, release the lock and now you have two duplicate tasks in the queue?
Thanks for your precious guidelines ! Here is a sum up of the new option : Note that there are some edge cases, as you suggested @steinitzu : this may lead to multiple tasks running if a task submitted removes a lock and another task is scheduled before the worker state is updated (task does not appear in scheduled or active tasks). This new behaviour was useful for long running tasks, scheduled at relatively low frequency (every 1-15 minutes), in an environnement where workers are ungracefully restarted for exploitation needs. This may not suite every needs. |
It still seems strange to me to only cover scheduled (ETA) and active tasks. Cause far as I understand, if your workers are busy and you're queuing up new tasks without scheduling this will always clear the lock and you can add infinite duplicate tasks to the queue. Otherwise I can live with some occasional race conditions and performance issues with this option as long as they're documented and users are aware of them. |
6451a97
to
11365ae
Compare
To my knowledge, there is no way to list all submitted tasks in a broker agnostic way. I updated the documentation with performances consideration and a warning of the incompatibility of the Also, the challenge now scans for reserved tasks and the logic is now described in the README --- here are some details about worker inspection --- |
Add a configuration for allowing automatic lock release for a task.
Decision to release a lock is based on celery worker inspection.