add timeout_worker_is_alive as an option for main.run. #2397
Conversation
The option let user set the timeout for judging if a process is dead in multiprocessing mode(i.e., workers > 1).
|
This PR does fix some edge cases. Please fix the CI failures so we can merge it. |
socrateslee
force-pushed
the
feature/timeout_worker_is_alive
branch
from
ea7ecc6
to
f1873bc
Compare
checks passed, please go ahead |
|
Defaulting to a low number doesn't seem right. |
It reads to me to be defaulting to the exact number that is already being used to preserve behavior I think? https://github.com/encode/uvicorn/blob/master/uvicorn/supervisors/multiprocess.py#L65 |
|
Hi @abersheeran, |
|
I've tried almost Uvicorn alternatives including Gunicorn, Hypercorn, Daphne, Sanic, Tornado, Waitress. |
| @click.option( | ||
| "--timeout-worker-is-alive", | ||
| type=float, | ||
| default=5, |
There was a problem hiding this comment.
I'm not sure this is wanted as default value?
There was a problem hiding this comment.
Ah, I've mentioned before: #2397 (comment)
There was a problem hiding this comment.
Yes, it's a good default value for most programs. Only those with heavy loads require larger values.
There was a problem hiding this comment.
Additionally, the default timeout for a single request in httpx is also 5 seconds.
There was a problem hiding this comment.
Failed some checks. Can I fix it?
There was a problem hiding this comment.
There's an unrelated issue on the pipeline.
There was a problem hiding this comment.
Hi @Kludex ,
I think, it's not only current branch issue.
Your master branch doesn't pass scripts/test too.
Could you please checkout?
Summary
The option lets users set the timeout for judging if a process is dead in multiprocessing mode(i.e., workers > 1).
When the module calls uvicorn.run has some heavy load like importing a lot of modules, the is_alive check in supervisors.multiprocessing may be blocked. When this happens, the default 5 seconds timeout may cause the misjudgement of a process and kill it.
Issues like #2396 , https://stackoverflow.com/questions/78680384/during-uvicorn-startup-child-process-dies-in-kubernetes-cluster may be caused by the case.
Checklist