-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Multiprocess RQ workers (using supervisor) #4371
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.
We still some healthcheck, right? Can Supervisor handle it?
bin/docker-entrypoint
Outdated
export WORKERS_COUNT=${WORKERS_COUNT:-2} | ||
export QUEUES=${QUEUES:-} | ||
|
||
touch /tmp/worker.log && tail -f /tmp/worker.log & |
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.
Is there a reason you didn't configure supervisord to write to stdout directly?
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.
That's even simpler :) didn't think of that! 1fb1a7e handles this.
worker.conf
Outdated
@@ -0,0 +1,17 @@ | |||
[supervisord] | |||
logfile=/tmp/supervisord.log |
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.
Need to update this to stdout as well.
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.
In 092eb6c I've used supervisor_checks to run a check every minute (we should probably change the interval) - the check runs for every monitored process and asserts that the worker is behaving as expected (on top of what you get from supervisor, which is basically testing if the process is up). If a specific worker process is misbehaving, it is replaced. We can also add more checks for CPU / memory (provided OOTB by supervisor_checks) Let me know what you think of this. |
@arikfr can we merge this? |
What type of PR is this? (check all applicable)
Description
This is an alternative implementation to the one suggested in #4233. Here we use
supervisor
instead ofHoncho
in order to spin up, monitor and restart multiple worker processes.supervisor is installed in the Docker image and is launched inside the
worker
entrypoint. We use the non-daemonized mode of supervisor in order to allow the container to exit in case the supervisor process exits.Related Tickets & Documents
#4233