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

use worker_init/worker_process_init to start event processor threads in celery #444

Closed
wants to merge 1 commit into from

Conversation

beniwohli
Copy link
Contributor

Celery uses a prefork model for its worker processes. The app is loaded
in a "worker", which then forks one or more "worker processes". Similiarly
to the uwsgi prefork issues we had to work around, this fork happens
after we created the event processor thread, so the worker processes
don't have an event processor thread of their own, and just fill up the
event queue without it ever being processed.

To work around this, we use worker_init and worker_process_init signals
to start the event processor thread after the fork.

This only became an issue with version 4.2.0 of the agent. In earlier
versions of the agent, threads were started relatively late, after
celery already forked the worker processes.

…in celery

Celery uses a prefork model for its worker processes. The app is loaded
in a "worker", which then forks one or more "worker processes". Similiarly
to the uwsgi prefork issues we had to work around, this fork happens
_after_ we created the event processor thread, so the worker processes
don't have an event processor thread of their own, and just fill up the
event queue without it ever being processed.

To work around this, we use worker_init and worker_process_init signals
to start the event processor thread _after_ the fork.

This only became an issue with version 4.2.0 of the agent. In earlier
versions of the agent, threads were started relatively late, after
celery already forked the worker processes.
Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Not a Celery expert, but looks reasonable. Did you try this with the non-prefork pool types? (solo, eventlet, gevent)

@beniwohli
Copy link
Contributor Author

Did you try this with the non-prefork pool types? (solo, eventlet, gevent)

Not yet, good point. These signals are only fired for the pre-fork worker model AFAIK, so at least it shouldn't make things worse for the other worker pool types.

@beniwohli
Copy link
Contributor Author

Update: solo works, eventlet and gevent are missing spans, but this was always the case, as we don't currently support eventlet/gevent.

@beniwohli beniwohli closed this in 46feea4 Apr 2, 2019
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.

None yet

2 participants