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
logrotate (USR1 signal) restarts uvicorn worker #1559
Comments
Uvicorn's reload strategy is different from gunicorn. Something about this is written on #1205. |
it's not about uvicorn reload strategy, it's about the fact our UvicornWorker is disabling a signal that gunicorn uses to reload its workers and trigger logrotate, see https://docs.gunicorn.org/en/latest/deploy.html?highlight=logrotate#logging |
Sorry. Wrong signal 😅 HUP is the one for reload. I'm going to check this. 👍 |
Steps to reproduceMinimal Fastapi code
Run with gunicorn
Set up logging
Set up logrotate/etc/logrotate.d/uvicorn-test
Send USR1 signal
We can see the process is terminated:
Proposal: Change uvicorn/worker.py to the following code:
Re-run test
Process is no longer terminated, logrotates fine and log.txt is still appended after the file rotation.
Clearly this change only impacts Uvicorn's gunicorn worker class. i.e. But Btw, we can't simply fully copy gunicorn's init_signals method as suggested by yinkh in #896 (comment) As the below signals are not compatible with asyncio.
We would end up with errors like below.
Testing other signals such as TTIN, TTOU, QUIT, INT, TERM on gunicorn's signal handling page (https://docs.gunicorn.org/en/stable/signals.html) seem to work fine. I have no idea if these two lines from gunicorn worker base class https://github.com/benoitc/gunicorn/blob/027f04b4b4aee4f50b980a7158add0feaf4c1b29/gunicorn/workers/base.py#L185
If there is no problem with the above change. I'd like to make a PR. I don't seem to have permission to push a branch now though. Thanks. |
You can fork the project and create a branch there. When the PR creation window opens, it will target the main project. |
Closed by #1565. It will be available on the next |
Discussed in #1558
Originally posted by wonjoonSeol-WS July 7, 2022
I Use FastAPI+Uvicorn+Gunicorn and issuing USR1 signal to uvicorn worker restarts the worker.
I believe the issue is to do with uvicorn worker not implementing signal handler - github.com/encode/uvicorn/blob/….
It converts it to signal.SIG_DFL.
SIGUSR1 and SIGUSR2 both have the default action Term -- the process is terminated. (unix.stackexchange.com/questions/38589/…)
According to a similar issue page, #896 (comment)
yinkh changed init_signals to gunicorn worker's original will not restart the worker.
I also notice that Uvicorn only handles two signals:
I would be more than happy to contribute to uvicorn, if I can change the worker init_signals function to do the same with gunicorn worker?
The text was updated successfully, but these errors were encountered: