{{ message }}
Schedule the round-robin request timeout once, not twice#2229
Open
pavel-ptashyts wants to merge 2 commits into
Open
Schedule the round-robin request timeout once, not twice#2229pavel-ptashyts wants to merge 2 commits into
pavel-ptashyts wants to merge 2 commits into
Conversation
In ROUND_ROBIN mode every request resolves up front (it needs the resolved IP to key the pool), and resolveAddresses scheduled the request timeout there. On a pooled hit the reuse path (sendRequestWithOpenChannel) then scheduled it again, and the second schedule cancelled the first (NettyResponseFuture.setTimeoutsHolder) — a redundant TimeoutsHolder allocation plus a timer-wheel insert and cancel on every keep-alive round-robin request. Make the up-front resolveAddresses skip scheduling (new scheduleTimeout=false argument) and let the reuse-or-connect path schedule exactly once: sendRequestWithOpenChannel for a pooled hit (unchanged), or the round-robin branch of sendRequestWithNewChannel for a new connection (added). DEFAULT mode is unchanged — its new-channel resolveAddresses still passes true. Redirect/retry per-hop timeout reset is preserved, since each path still schedules a fresh holder per attempt. One behavioural nuance: in ROUND_ROBIN the request timeout no longer bounds the up-front DNS resolve itself (only the reuse/connect that follows). That resolve is a cache hit in the steady state, and an uncached one is still bounded by the resolver's own timeout. Adds a RoundRobinSendTypeTest case asserting a round-robin request still hits the request timeout (guarding the new-connection path's schedule); existing round-robin send/failover/redirect and load-balance config tests pass unchanged. No public API change (resolveAddresses is private). Fixes finding AsyncHttpClient#6.
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.

In ROUND_ROBIN mode every request resolves up front (it needs the resolved IP to key the pool), and resolveAddresses scheduled the request timeout there. On a pooled hit the reuse path (sendRequestWithOpenChannel) then scheduled it again, and the second schedule cancelled the first (NettyResponseFuture.setTimeoutsHolder) — a redundant TimeoutsHolder allocation plus a timer-wheel insert and cancel on every keep-alive round-robin request.
Make the up-front resolveAddresses skip scheduling (new scheduleTimeout=false argument) and let the reuse-or-connect path schedule exactly once: sendRequestWithOpenChannel for a pooled hit (unchanged), or the round-robin branch of sendRequestWithNewChannel for a new connection (added). DEFAULT mode is unchanged — its new-channel resolveAddresses still passes true. Redirect/retry per-hop timeout reset is preserved, since each path still schedules a fresh holder per attempt.
One behavioural nuance: in ROUND_ROBIN the request timeout no longer bounds the up-front DNS resolve itself (only the reuse/connect that follows). That resolve is a cache hit in the steady state, and an uncached one is still bounded by the resolver's own timeout.
Adds a RoundRobinSendTypeTest case asserting a round-robin request still hits the request timeout (guarding the new-connection path's schedule); existing round-robin send/failover/redirect and load-balance config tests pass unchanged. No public API change (resolveAddresses is private).