feat: use racing, retrying dialer when attempting to connect #415
Conversation
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
Previously, we could get TCP I/O timeouts if we had routing issues. Empirically, we found that having two dialers, one that retries quickly and one that is the default dialer, decreases the routing issues on some networks. Signed-off-by: Chris Goller <goller@gmail.com>
goller
requested review from
121watts,
PedroGuerraPT,
ischolten,
jacobwgillespie and
lukevmorris
ischolten
approved these changes
Dec 17, 2025
Comment on lines
+59
to
+66
| // It uses two different dialers. | ||
| // The dialer connects first is returned, and the other is canceled. | ||
| // | ||
| // The first has a short timeout (200 ms) and continues to retry until it succeeds. | ||
| // The second dialer has no timeout and will block until it either succeeds or fails. | ||
| // | ||
| // We are doing this because we see connection timeouts perhaps caused by some competing network routes. | ||
| // Our workaround is to use a short timeout dialer that will retry until it succeeds. |
Contributor
There was a problem hiding this comment.
We dont need a retry limit bc the second one will eventually fail or succeed? Do we want to set some maximumum limit or log the timing so if this does end up being quite long we can see what the hold up is?
Comment on lines
+26
to
+31
| if errors.Is(err, context.DeadlineExceeded) { | ||
| continue | ||
| } | ||
| if errors.Is(err, os.ErrDeadlineExceeded) { | ||
| continue | ||
| } |
Contributor
There was a problem hiding this comment.
understanding check: these are the only errors we're retrying for and others will just get returned below?
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.
Previously, we could get TCP I/O timeouts if we had routing issues.
Empirically, we found that having two dialers, one that retries quickly and one that is the default dialer, decreases the routing issues on some networks.