-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 KEEP_ALIVE_TIMEOUT default to 120 seconds #2670
Conversation
Sanic has used 5 second timeout to quickly close idle connections, for no particular reason, as idle connections use very little resources. This PR increases it to 80 seconds, slightly larger than Nginx default of 75 s, to avoid a problem with a request failing when Sanic has closed the connection but Nginx hasn't noticed. This should also improve practical performance on long latency connections, where reconnecting is expensive, and better fits typical user flow browsing pages with longer-than-5-second intervals. The root cause of the next request failing (i.e. connection not being properly terminated) should be looked at in more detail, but meanwhile this should be a useful workaround to keep the defaults working fluently, as well as to improve performance for the aforementioned reasons.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2670 +/- ##
=============================================
+ Coverage 88.887% 88.944% +0.057%
=============================================
Files 92 92
Lines 7001 7001
Branches 1194 1194
=============================================
+ Hits 6223 6227 +4
+ Misses 531 529 -2
+ Partials 247 245 -2
☔ View full report in Codecov by Sentry. |
Tackle timeout overhaul in June? |
Yes, sounds good. This could be included as a workaround in 23.3 but probably better to avoid the hassle (docs update, release notes etc) and simply do it in 23.6 in one go with the simpler timeouts. |
Reopening for inclusion in 23.6 because we didn't get to do the bigger timeout revamping yet. |
Sanic has used 5 second timeout to quickly close idle connections, for no particular reason, as idle connections use very little resources. This PR increases it to 80 seconds, slightly larger than Nginx default of 75 s, to avoid a problem with a request failing when Sanic has closed the connection but Nginx hasn't noticed. This should also improve practical performance on long latency connections, where reconnecting is expensive, and better fits typical user flow browsing pages with longer-than-5-second intervals.
The root cause of the next request failing (i.e. connection not being properly terminated) should be looked at in more detail, but meanwhile this should be a useful workaround to keep the defaults working fluently, as well as to improve performance for the aforementioned reasons.
Fix #2681