Http: a request that timed out on a pooled socket is not sent again - #147
Merged
Conversation
The keep-alive pool reconnects and sends the request again whenever a pooled socket fails. For the case it was written for that is exactly right: the server reaps idle sockets, the next exchange on one fails immediately with a reset or a broken pipe or an EOF, nothing was ever served on it, and reconnecting is transparent. It was doing the same thing after a timeout, and a timeout is not that. The peer accepted the request and did not answer in time. It may have processed it. Sending it again on a fresh socket applies a write twice. This sits one layer below the routing guard that just stopped re-sending writes of unknown outcome, and it would have undone that guard from underneath: the routing layer can decline to retry all it likes while the transport quietly retries for it. Now a pooled exchange that fails with a timeout returns the error. Every other failure still reconnects and re-sends, so the reaped-socket path -- the reason the fallback exists -- is untouched. Deliberately applied to all requests rather than only writes. The transport does not know a write from a read, and threading that through every caller would put the decision in more places than can be kept honest. Reads lose nothing that matters: the routing layer above already refreshes and retries a read on any backend failure, so the recovery is still there, one layer up, where it can see what it is retrying. Measured by counting what the server received: the request arrived twice before this change and arrives once after. The test fails with "left: 2, right: 1" if the guard is removed.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The keep-alive pool reconnects and sends the request again whenever a pooled socket
fails. For the case it was written for that is exactly right: the server reaps idle
sockets, the next exchange on one fails immediately with a reset or a broken pipe or
an EOF, nothing was ever served on it, and reconnecting is transparent.
It was doing the same thing after a timeout, and a timeout is not that. The peer
accepted the request and did not answer in time. It may have processed it. Sending
it again on a fresh socket applies a write twice.
This sits one layer below the routing guard that just stopped re-sending writes of
unknown outcome, and it would have undone that guard from underneath: the routing
layer can decline to retry all it likes while the transport quietly retries for it.
Now a pooled exchange that fails with a timeout returns the error. Every other
failure still reconnects and re-sends, so the reaped-socket path -- the reason the
fallback exists -- is untouched.
Deliberately applied to all requests rather than only writes. The transport does not
know a write from a read, and threading that through every caller would put the
decision in more places than can be kept honest. Reads lose nothing that matters:
the routing layer above already refreshes and retries a read on any backend failure,
so the recovery is still there, one layer up, where it can see what it is retrying.
Measured by counting what the server received: the request arrived twice before this
change and arrives once after. The test fails with "left: 2, right: 1" if the guard
is removed.