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 child_exit() callback to configuration #1394

Merged
merged 3 commits into from Jan 27, 2017

Conversation

Projects
None yet
4 participants
@jonashaag
Contributor

jonashaag commented Nov 18, 2016

This adds a child_exit() callback to the Gunicorn configuration that works similar to worker_exit() but is called in the master process instead of the child process.

I also added some basic unit tests for the both.

A change to how Worker.pid is implemented was necessary in order for it to be meaningful in the master process (it would always be the PID of the master process, no information given about the worker process).

Our need for this is prometheus/client_python#115

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Nov 21, 2016

Owner

I will look at the patch. For your case why don't you clean the data when the worker is spawned though?

Owner

benoitc commented Nov 21, 2016

I will look at the patch. For your case why don't you clean the data when the worker is spawned though?

@jonashaag

This comment has been minimized.

Show comment
Hide comment
@jonashaag

jonashaag Nov 21, 2016

Contributor

For your case why don't you clean the data when the worker is spawned though?

We need the old worker's PID to do that

Contributor

jonashaag commented Nov 21, 2016

For your case why don't you clean the data when the worker is spawned though?

We need the old worker's PID to do that

jonashaag added some commits Nov 18, 2016

Add child_exit() callback
This is similar to worker_exit() in that it is called just after a
worker has terminated, but it's called in the Gunicorn *master* process,
not the *child* process.
@jonashaag

This comment has been minimized.

Show comment
Hide comment
@jonashaag

jonashaag Nov 25, 2016

Contributor

Rebased to latest master

Contributor

jonashaag commented Nov 25, 2016

Rebased to latest master

@berkerpeksag

This comment has been minimized.

Show comment
Hide comment
@berkerpeksag

berkerpeksag Dec 22, 2016

Collaborator

Looks good to me too. It would be nice to add a versionadded directive. @benoitc and @tilgovi should it be 19.7 or 20?

(We also need to use the "squash and merge" button when we merge this.)

Collaborator

berkerpeksag commented Dec 22, 2016

Looks good to me too. It would be nice to add a versionadded directive. @benoitc and @tilgovi should it be 19.7 or 20?

(We also need to use the "squash and merge" button when we merge this.)

@jonashaag

This comment has been minimized.

Show comment
Hide comment
@jonashaag

jonashaag Dec 22, 2016

Contributor

(We also need to use the "squash and merge" button when we merge this.)

I deliberately split the changes into three commits to make them easier to understand

Contributor

jonashaag commented Dec 22, 2016

(We also need to use the "squash and merge" button when we merge this.)

I deliberately split the changes into three commits to make them easier to understand

@jonashaag

This comment has been minimized.

Show comment
Hide comment
@jonashaag

jonashaag Jan 21, 2017

Contributor

Hey, what's the state of this? Is there anything I can do to have this merged?

Contributor

jonashaag commented Jan 21, 2017

Hey, what's the state of this? Is there anything I can do to have this merged?

@benoitc benoitc merged commit 21ffa92 into benoitc:master Jan 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

StephanErb added a commit to StephanErb/client_python that referenced this pull request Mar 18, 2017

Provide robust Gunicorn config in multiprocess docs
We need to use the recently added `child_exited` callback in order to correctly clean up after a segfaulted or OOM-killed worker (benoitc/gunicorn#1394). 

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

StephanErb added a commit to StephanErb/client_python that referenced this pull request Mar 20, 2017

Provide robust Gunicorn config in multiprocess docs
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).

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.

StephanErb added a commit to StephanErb/client_python that referenced this pull request Mar 21, 2017

Provide robust Gunicorn config in multiprocess docs
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).

brian-brazil added a commit to prometheus/client_python that referenced this pull request Mar 21, 2017

Provide robust Gunicorn config in multiprocess docs (#146)
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).

fofanov pushed a commit to fofanov/gunicorn that referenced this pull request Mar 16, 2018

Merge pull request #1394 from jonashaag/master
Add child_exit() callback to configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment