Skip to content

Provide robust Gunicorn config in multiprocess docs#146

Merged
brian-brazil merged 1 commit into
prometheus:masterfrom
StephanErb:gunicorn-doc-fixes
Mar 21, 2017
Merged

Provide robust Gunicorn config in multiprocess docs#146
brian-brazil merged 1 commit into
prometheus:masterfrom
StephanErb:gunicorn-doc-fixes

Conversation

@StephanErb

Copy link
Copy Markdown
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.

Comment thread README.md Outdated

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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