Skip to content
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

Add --drain flag to worker command #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add --drain flag to worker command #59

wants to merge 1 commit into from

Conversation

j4mie
Copy link
Member

@j4mie j4mie commented Jan 21, 2023

python manage.py worker --drain will start a worker process which will drain all jobs in the queue (if there are any) and then exit. This addresses #16 (from seven years ago 😱) and #52, and is an alternative (and much simpler) solution to that proposed in #56.

Copy link
Member

@jordaneb jordaneb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice simple approach to solving this problem but I think we should warn that if the queue exceeds a certain size then the cron job could run for a really long time and depending on the frequency of the cron job then multiple DBQ workers could be spawned at once.

Something along the lines of: "Note that if the queue is large it will be necessary to manage the lifetime of the process with a tool like timeout (http://man.he.net/?topic=timeout&section=all) to make sure it doesn't run forever or that multiple worker processes aren't spawned depending on the frequency of your cron schedule."

@collinr3
Copy link
Contributor

Thanks @j4mie. I agree that it is simple and that there is attraction in that.
I'd previously gone for the more complex #56 --shift_limit approach, specifically because of the potential issue @jordaneb raises. The dynamic between --rate expected volumes of jobs, average job execution time and CRON interval is a complex one. The unknown is what undesirable side effects may be introduced into the data should there be multiple workers processing the same queue. That said, the issue cannot be easily mitigated in a CRON, environment, whatever the approach, so the 'warning' may be the most pragmatic.
One comment on the test for --drain consider adding an extra step to submit a further Job after the worker has initially drained the queue to check that it doesn't get processed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants