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

Provide robust Gunicorn config in multiprocess docs #146

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

StephanErb
Copy link
Contributor

We need to use the recently added Gunicorn child_exited callback in order to
correctly clean up after a segfaulted or OOM-killed worker (for details, see
benoitc/gunicorn#1394 and #115).

The try catch is necessary to guard against unexpected race conditions and
errors in the cleanup logic. I have seen this twice, crippling the master
process in a cascading failure scenario.

README.md Outdated
from prometheus_client import multiprocess
multiprocess.mark_process_dead(worker.pid)
def child_exit(server, worker):
import logging
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not safe to do imports inside functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you are suggest that also from prometheus_client import multiprocess has to be moved?

I will need to re-test then, if the try catch is still needed after extracting the import.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

We need to use the recently added Gunicorn `child_exited` callback in order to
correctly clean up after a segfaulted or OOM-killed worker (for details, see
benoitc/gunicorn#1394).
@StephanErb
Copy link
Contributor Author

I have pushed a squashed commit with the extracted import. I also dropped the try catch. The previous errors I was seeing happened during import, as this is now done up-front I don't see any significant error potential.

@brian-brazil brian-brazil merged commit 3183fa7 into prometheus:master Mar 21, 2017
@brian-brazil
Copy link
Contributor

Thanks!

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.

2 participants