Repository navigation
Race connection attempts across resolved addresses for the Classic client (RFC 8305) - #936
arturobernalg wants to merge 1 commit into
Conversation
9a052a2 to
cd5e03d
Compare
|
Validated the racing mechanism and fallback timing against real clients over real hosts:
Same race semantics in all three that have it; winner differs only by each client's own DNS ordering (v4 vs v6), not by the racing logic. Fallback timing confirmed with a stalled address + real local listener: 266ms, matching the 250ms stagger. Also marked the opt-in setters |
|
@arturobernalg This feature was requested by @rschmitt I am not sure I am able to review this or any of the related pull requests. |
|
Ryan hasn't been around in a long time, so I don't think we should block on that. I've tested this thoroughly - real fallback timing, live comparisons against Go's net.Dialer and curl's native Happy Eyeballs across multiple hosts, concurrency stress tests on the race/cleanup paths - and I'm confident it's correct and behaves as intended. Happy to walk through any specific part you're unsure about. |
|
@arturobernalg This is true @rschmitt has not been around much, but he has not resigned from the project either. Invite him to review the pull requests. If he does not respond, feel free to merge them but please keep this feature as opt-in, at least initially. I would also wait until 5.8 unless you have pressing reasons to put this feature into 5.7. |
|
@arturobernalg And please fix the deprecation warnings |
|
Sounds good, will target 5.8 then. @rschmitt if you get a chance, would appreciate you taking a look at this new version - opt-in, disabled by default. |
…ient Replaces the one-thread-per-address, sleep-based stagger with a single dispatcher that starts the first address immediately and waits up to the connection attempt delay for an outstanding attempt to finish before starting the next; on a failure it advances immediately, at the RFC 8305 minimum spacing (10ms), instead of waiting out the full delay. No worker thread ever sleeps a delay itself, bounding concurrency to the number of resolved addresses actually in flight. Socket registration goes through a small atomic registry instead of a bare reference: deciding a winner and a worker registering its socket can happen at the same instant, and whichever runs second closes the socket, so nothing registered around that moment survives open regardless of timing. shutdownNow() alone cannot abort a blocking Socket#connect; closing the socket is what actually does that, on every exit path (winner found, every address failed, or interrupted) via one cleanup call in a finally block. An interruption during the minimum-spacing wait is now propagated instead of swallowed, aborting the race immediately like interruption anywhere else in it. Racing is skipped in favor of the existing sequential path when a specific, non-ephemeral local port is requested, since binding several attempts to it concurrently would trade a real connect failure for a spurious bind conflict. Public, blocking connect() behavior, TLS, SOCKS and local binding are otherwise unchanged.
cd5e03d to
38e8db5
Compare
Implements RFC 8305 (Happy Eyeballs v2) connection racing for the Classic (blocking) client, as the third and final PR of this series, alongside #935 (Async connection racing) and arturobernalg#20 (Async independent A/AAAA resolution, stacked on #935 pending its merge). This PR is independent of the other two: the Classic client has its own connection path (java.net.Socket, not the reactor), so it needed its own, smaller mechanism rather than reusing the Async internals.
Dependency
None. Based directly on upstream master, independent of #935 and arturobernalg#20.