-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Increase default keepalive_timeout server-side. #9285
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9285 +/- ##
==========================================
+ Coverage 97.31% 98.41% +1.10%
==========================================
Files 105 107 +2
Lines 34759 34828 +69
Branches 3351 4132 +781
==========================================
+ Hits 33825 34277 +452
+ Misses 692 380 -312
+ Partials 242 171 -71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could result in significantly more memory being used on busy systems. Its probably fine though as its easy enough to override, but it may surprise some users.
Well, a busy system should definitely have a proxy, and this shouldn't have any impact in that situation. |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply de997af on top of patchback/backports/3.11/de997af2069781a7d50639fe59ad9ac8e3e9847e/pr-9285 Backporting merged PR #9285 into master
🤖 @patchback |
(cherry picked from commit de997af)
As we strongly recommend deploying apps behind a reverse proxy it makes sense to optimise the defaults for this use case.
When using a reverse proxy it's important to have a longer timeout than the proxy's timeout, otherwise we likely hit race conditions with the proxy trying to forward another request as we close the connection.
For proxies, it'd probably make the most sense to actually remove the timeout entirely, but for users without proxies the defaults should still be safe, therefore I'm proposing to just make it substantially longer by default.
I expect the vast majority of proxies to have a keepalive of 1 hour or less, so have chosen a value just over the 1 hour mark (for reference, nginx defaults to 75 seconds, ALB to 1 hour).
See #9138