[release/6.0] Respect the Keep-Alive response header on HTTP/1.0 #73245
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Respect the Keep-Alive response header on HTTP/1.0 * Remove TimeoutOffset * Update Trace message * Update tests * Adjust test timeouts
|
Tagging subscribers to this area: @dotnet/ncl |
ghost
assigned 
|
This PR has been pre-approved for 6.0.9 by Tactics today, pending final agreement with @stephentoub. |
karelz
added
NO-MERGE
|
Update: We have reached consensus over email with @stephentoub and decided to take this change into 7.0 and 6.0.x servicing. Removing the |
karelz
removed
the
NO-MERGE
karelz
approved these changes
Aug 12, 2022
carlossanlop
approved these changes
Aug 12, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #73020 and #73585
Fixes #72958
Customer Impact
There is an inherent race in HTTP/1.x between the server closing an idle connection and the client trying to send a request at the same time (#60644). To mitigate this, the server may respond with a
Keep-Alive: timeout=60header that informs the client of how long it can reuse an idle connection for.This change makes
SocketsHttpHandlerlook for this header and use it to limit the idle timeout of a connection.As a result, we avoid the race condition and fewer requests fail with a "Connection reset by peer" error.
Testing
Targeted test cases have been added and the patch was validated by an internal party in production.
Risk
There is a tradeoff between mitigating the race condition and how long we consider an idle connection usable for.
There is a risk that the user may see more connections being established if they make requests on an interval that falls within a 1-second offset of the server Keep-Alive timeout.
For example, if the server responds with a timeout of 5 seconds and the user has a script pinging the server every 4.5 seconds, every request would create a new connection instead of reusing the existing one.
Users facing such an issue can increase the interval at which they make requests or if possible increase the timeout on the server.