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

"timeout" setting used for two things that should be distinct #1658

Closed
tomjaguarpaw opened this issue Dec 6, 2017 · 7 comments
Closed

"timeout" setting used for two things that should be distinct #1658

tomjaguarpaw opened this issue Dec 6, 2017 · 7 comments

Comments

@tomjaguarpaw
Copy link
Contributor

Workers are killed if they have not responded within timeout seconds. However, when a worker is spawned it is also killed if it doesn't finish spawning within timeout seconds. In my opinion these two timeout values should be configured independently. I have a worker which takes a long time to spawn (over 30s) but it should handle requests quickly. I would like the timeout setting set to a small value (say 30) and a new spawn_timeout set to a large value (say 60).

Would this be a good idea? What do others think?

@tilgovi
Copy link
Collaborator

tilgovi commented Dec 6, 2017

I can't find a reference right now, but I'll look more later. We have discussed this in other issues before, for sure.

I mostly agree. I'm not sure there needs to be a difference between the startup timeout and the worker timeout, but I do think a separate request timeout would be helpful especially because the current timeout functions as a request timeout for the sync worker (but not for the others).

@tilgovi
Copy link
Collaborator

tilgovi commented Dec 9, 2017

@benoitc @berkerpeksag should we add a request timeout setting to distinguish it from the worker heartbeat timeout?

@rbwinslow
Copy link

It looks like a proper request timeout feature will require separate implementations for each worker class. A narrower feature, along the lines of allow_initial_delay, could be implemented generically, as an elaboration of the logic for the timeout feature.

@tilgovi et. al., might you be receptive to such an approach?

@rbwinslow
Copy link

There's a commit (the more recent one) demonstrating such an initialization-time delay. I used a slightly less obscure config setting name, timeout_start_after. Worth creating a new issue, maybe, and a pull request?

@opiumozor
Copy link

@tilgovi any update on this? A request timeout for async worker would be a great feature

@tilgovi
Copy link
Collaborator

tilgovi commented Mar 5, 2018

@opiumozor nope! I'm happy to review PRs and discuss implementation if you want to take it on, though!

@opiumozor
Copy link

@tilgovi I wrote a small PR (#1730) to implement a request timeout for async worker. Let me know what you think and if you like the idea, i will keep working on it

rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Mar 14, 2019
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Mar 14, 2019
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Mar 14, 2019
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Mar 14, 2019
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Sep 24, 2019
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Jun 1, 2020
rbwinslow pushed a commit to rbwinslow/gunicorn that referenced this issue Jun 1, 2020
A new timeout-start-after value is added to config, granting workers an
initial grace period to do time-consuming initialization, after which
they are held to timeout stricture.

Previous attempts of mine to form an acceptable pull request have failed
linting in CI, all in code I didn't touch. This branch is newly minted
from the current upstream master.
@benoitc benoitc closed this as completed May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants