Schedule the round-robin request timeout once, not twice by pavel-ptashyts · Pull Request #2229 · AsyncHttpClient/async-http-client · GitHub
Skip to content

Schedule the round-robin request timeout once, not twice#2229

Open
pavel-ptashyts wants to merge 2 commits into
AsyncHttpClient:mainfrom
maygemdev:perf/round-robin-single-timeout-schedule
Open

Schedule the round-robin request timeout once, not twice#2229
pavel-ptashyts wants to merge 2 commits into
AsyncHttpClient:mainfrom
maygemdev:perf/round-robin-single-timeout-schedule

Conversation

@pavel-ptashyts

Copy link
Copy Markdown
Contributor

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).

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant