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

Exit with status 3 when worker starts failed #1077

Merged
merged 4 commits into from Jun 22, 2021

Conversation

fanchunke
Copy link
Contributor

This PR fixes #1066 .

When starting Gunicorn with Uvicorn worker, if exceptions raised at startup events, UvicornWorker exits with zero exit code, therefore Gunicorn will keep booting worker.

This PR sets exit code 3 when worker starts failed, Gunicorn will then handle SIGCHLD correctly and could shut it down to avoid infinite start/stop cycles.

@euri10
Copy link
Member

euri10 commented Jun 12, 2021

@fanchunke1991 you can use the scripts/lint locally to fix your linting issues

@fanchunke
Copy link
Contributor Author

@euri10 my bad, I have already used scripts/lint and fixed linting issues.

Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

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

Looks good to me, solves the issue at end,
However I'd like to give it for other reviewers a little more time

@euri10 euri10 added the hold Don't merge yet label Jun 21, 2021
@euri10 euri10 requested a review from Kludex June 21, 2021 06:56
@fanchunke fanchunke requested a review from euri10 June 21, 2021 12:23
Copy link
Sponsor Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Hey @fanchunke1991 ! 👋

Thanks for the PR! 🎉

I'm quite worried about magic numbers, and the round trip to found information about them. If we could reference the same variable that gunicorn uses, it will be great. Do you mind taking a look at the suggestion?

uvicorn/workers.py Outdated Show resolved Hide resolved
@Kludex
Copy link
Sponsor Member

Kludex commented Jun 22, 2021

@fanchunke1991 Thanks for PR! 🎉

@Kludex Kludex merged commit e885bbd into encode:master Jun 22, 2021
@Kludex Kludex removed the hold Don't merge yet label Jun 25, 2021
Kludex pushed a commit that referenced this pull request Nov 17, 2021
* fix: exit with status 3 when worker starts failed to aviod infinite start/stop cycles

* docs: fixes spelling error

* style: lint the code

* fix: replace magic exit code

Co-authored-by: fanchunke <fanchunke@laiye.com>
@gabac gabac mentioned this pull request Jan 5, 2022
2 tasks
Kludex pushed a commit to sephioh/uvicorn that referenced this pull request Oct 29, 2022
* fix: exit with status 3 when worker starts failed to aviod infinite start/stop cycles

* docs: fixes spelling error

* style: lint the code

* fix: replace magic exit code

Co-authored-by: fanchunke <fanchunke@laiye.com>
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.

gunicorn keeps booting workers when exceptions raised at startup events
3 participants