Repository navigation
Do not abandon a connection attempt before ConnectTimeout has elapsed - #3277
Merged
Merged
Conversation
…#2894) The heartbeat's Connecting branch aborted an in-flight attempt as soon as the retry policy said a retry was due. With the default ExponentialRetry at half the connect timeout that is ~2.5s, so any attempt whose name resolution or TLS handshake took longer than that was killed inside its own ConnectTimeout budget and reported as a connect timeout. A primary DNS server that does not answer costs ~5s per lookup on glibc, so on Linux every reconnect attempt died during resolution and the connection never left Connecting. Require ConnectTimeout to have elapsed before the retry policy may abort an attempt. The socket connect already carries its own ConnectTimeout cancellation; this check bounds TLS and the handshake after it.
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
1 participant
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.
Partial fix for #2894 (Linux reconnect stuck in
Connecting).Problem
PhysicalBridge.OnHeartbeat'sConnectingbranch aborts an in-flight attempt as soon as the retry policy says a retry is due. The default policy isExponentialRetry(ConnectTimeout / 2), so with the default 5s connect timeout an attempt is killed after roughly 2.5s, well inside its ownConnectTimeoutbudget, and reported as a connect timeout.Name resolution runs inside
socket.ConnectAsync(DnsEndPoint, ct). When the primary DNS server does not answer, glibc waits ~5s per lookup before the secondary responds (Windows sidelines the dead server, which is why the report is Linux-only). Every reconnect attempt therefore died during resolution and the bridge never leftConnecting. Reproduced deterministically with anLD_PRELOADshim that delaysgetaddrinfo; 4s lookups never reconnected under the default policy and did underLinearRetry(10000).Change
The heartbeat may only abort an attempt once it has been running for at least
ConnectTimeoutand the retry policy says a retry is due. The socket connect already carries its ownConnectTimeoutcancellation; this check is what bounds TLS and the handshake after it. The elapsed-time calculation is factored into a helper shared withDueForConnectRetry.No public API change.
Test
ConnectionAttemptUnitTests.AttemptIsNotAbandonedBeforeConnectTimeout: a listener that accepts TCP and never answers TLS,ConnectTimeout = 1500, 100ms heartbeat,LinearRetry(50). Asserts the gap between consecutive timed-out attempts is at least the connect timeout. Fails onmainwith "attempt abandoned after 100ms, inside the 1500ms connect timeout".Not in this PR
A fallback to the last address an endpoint successfully connected to when resolution fails or times out, keeping the TLS host as the DNS name. That is the second half of the fix discussed on the issue and will follow separately.