Arm request timeouts on an event loop

[pavel-ptashyts]

Problem

Request and read timeouts are armed on the client's HashedWheelTimer. That has two
properties that only show up on short deadlines:

• A wheel quantizes. It fires on the first tick at or after the deadline, so a
  deadline near or below hashedWheelTimerTickDuration is rounded up to it.
• One thread carries every expiry for the whole client, and HashedWheelTimer's
  default taskExecutor is ImmediateExecutor, so each expiry runs inline on the wheel
  thread — including future.completeExceptionally(...) and therefore whatever the caller
  chained onto the response future.

On a one-second budget the first costs 0.3% and nobody notices. On a budget of tens of
milliseconds a tick is a large fraction of it, and a burst of expiries has no headroom to
absorb before the wheel starts running late.

Measured

2000 timeouts armed as one burst on Netty 4.2.16, JDK 17, tasks doing nothing but
recording their own lag. This is the floor; real work on the firing thread only adds to it.

instrument	deadline	mean	p50	p99	max
wheel, tick 5 ms	20 ms	+2.7	+2	+5	+5
wheel, tick 1 ms	20 ms	+1.3	+1	+2	+2
EventLoop.schedule	20 ms	+0.0	+0	+0	+0
wheel, tick 5 ms	1000 ms	+3.0	+3	+3	+3
wheel, tick 1 ms	1000 ms	+1.9	+2	+2	+2
EventLoop.schedule	1000 ms	+1.6	+2	+2	+2

An event loop shows zero overshoot because it schedules by deadline and derives its own
select() timeout from the nearest one. There is no quantum to round to.

This was a throwaway probe rather than JMH — client/src/jmh/java is not currently wired
into the build, so its benchmarks do not compile. Happy to add a proper benchmark if that
is fixed first, or as part of this.

Change

AsyncHttpClientConfig#isUseEventLoopTimeouts(), off by default, arms the request and
read timeouts on an event loop instead of the timer.

• Channel affinity where it exists. On the pooled path the channel is already in hand,
  so its own loop is used and the timeout expires on the thread that would have to close
  it. On the connect path there is no channel yet — deliberately, so the timeout also bounds
  address resolution and the connect — and any loop will do, because what the wheel costs is
  a single thread and a rounded-up tick rather than the identity of the thread.
• Arming allocates nothing extra. The cancellation handle lives on the task rather than
  in a wrapper, and the existing done flag stands in for the scheduler's already-expired
  flag, which the two schedulers spell differently.
• Shutdown race closed. isShuttingDown() can return false and schedule reject
  immediately after. Netty answers a rejected timeout with a logged warning rather than an
  exception, which would leave the exchange with nothing to end it, so a rejection falls
  back to the timer.
• The connection-pool cleaner stays on the timer either way.

Off by default because the expiry — and so whatever the caller chained onto the future —
then runs on an I/O thread, and blocking one stalls every connection it serves. The javadoc
says so and points callers at handleAsync.

Why not a wheel per event loop

That is how the Aerospike client solves the same problem: EventLoopBase owns a
HashedWheelTimer that is a Runnable the loop ticks itself. Deliberately not copied here.
A wheel arms in O(1) against O(log n) for a deadline queue, but at a few thousand timeouts
per loop that is a dozen comparisons, while the quantization it reintroduces costs
milliseconds on a 20 ms budget — the third row above is the whole point. A wheel also has to
be ticked forever, waking every loop even with nothing armed. Aerospike wrote its own because
its EventLoop abstracts over NIO, Netty and direct NIO and needed one timer; AHC is
Netty-only and gets a per-loop deadline queue for free.

API compatibility

No exception needed — revapi passes as-is. The change is additive: the existing
TimeoutsHolder constructor is kept and delegates, TimeoutTimerTask gains Runnable
without losing anything, and nothing is removed.

One place where narrowing was avoided on purpose: TimerTask#run declares throws Exception and Runnable#run does not. Rather than re-declaring the abstract method without
the throws clause, which would break an external subclass that declares it, the Runnable
entry point catches and logs.

Tests

EventLoopTimeoutTest asserts the switch itself rather than its side effects: which thread
onThrowable is called on. With the flag on it is one of the client's I/O threads; with it
off it is the timer thread. Both go through a real request against an endpoint that answers
well after the deadline.

Verification

mvnw clean verify — BUILD SUCCESS, 1466 tests, 0 failures, 0 errors, 21 skipped. Error
Prone, NullAway and Revapi all clean.

Caveat on the testing gate: AGENTS.md requires the build to run on JDK 11 and no JDK 11 is
installed on this machine, so it was run on JDK 17 (also in the CI matrix). The JDK 11
leg of CI on this PR is the real gate.

Claude Code on behalf of @pavel-ptashyts

🤖 Generated with Claude Code

[hyperxpro]

Round 1

[hyperxpro]

arm() catches RejectedExecutionException, this doesn't. Netty's ScheduledFutureTask.cancel goes through removeScheduled, which for an off-loop caller lazyExecutes a removal task, and offerTask rejects once the loop is shut down. So after a client.close() any late future.cancel(true) or abort() throws out of ListenableFuture.cancel(), which never threw before. The same try/catch arm() already has would settle it.

[hyperxpro]

This arms a task holding this from inside the constructor, so before requestTimeoutTask and requestTimeoutArmed are frozen and before the future has been handed the holder. At the pooled call site it is also before attachChannel. The wheel's 100 ms tick hid that window, an event loop won't: with a small enough request timeout the task can run first, find channel null in expire() and leave the pooled socket open, and setTimeoutsHolder then installs a holder that has already fired. Arming after the constructor returns closes it.

[hyperxpro]

This is almost never the loop the channel ends up on. initAndRegister calls next() again, so for an N loop group the affinity the javadoc above claims holds about one time in N. Every completion then cancels a scheduled entry on a foreign loop, and that entry sits in its queue waking it at the original request timeout deadline long after the request finished. Read timeout re-arms cross loops too. NettyConnectListener.onSuccess already has both the channel and the holder, it sets the resolved address there, so re-homing at that point would make the claim true on the connect path as well.

[hyperxpro]

Separately: calling next() only to pick a timeout thread advances the group's round robin counter, and that is the same counter that assigns channels to loops. With a power of two group and a fixed number of next() calls per request, registrations can settle onto a subset of the loops. Whether that helps or hurts depends on whether an address resolver group is configured, so it is action at a distance either way. Picking with ThreadLocalRandom over the group's executors, or re-homing once the channel exists, leaves the chooser alone.

[hyperxpro]

The handle is recorded after schedule() returns. If the exchange completes in that gap, cancel() wins the CAS, release() cancels the previous handle, and then armedOn writes the live one with nobody left to cancel it. cancelled is one shot, so nobody ever will. No wrong abort, since the task no-ops on done, but the entry stays in that loop's queue until the full deadline. Re-checking cancelled after arming and cancelling there closes it.

[hyperxpro]

This rationale is now spelled out four times: here, the TimeoutsHolder class doc, timeoutExecutor's Javadoc, and the class doc on TimeoutTimerTask. Keeping this as the canonical explanation and linking to it from the other three says it once and leaves a single place to update.

[hyperxpro]

This block is alphabetical, so it belongs just before defaultUseInsecureTrustManager, not between the two failedIpCooldown entries. The same split repeats in the field list, the constructor parameters and assignments, the Builder field and copy constructor, the build() call, AsyncHttpClientConfigDefaults, and ahc-default.properties, and the getter lands between isFailedIpCooldownEnabled and getFailedIpCooldownPeriod. The constructor is positional with a lot of adjacent booleans, so order is worth more here than tidiness.

[hyperxpro]

The subclass this defends against cannot exist. The only constructor is package private, so nothing outside this package can extend TimeoutTimerTask, and both subclasses are in it and neither throws. Which also means run(Timeout) could just drop its throws clause and run() could call it straight, no catch needed.

[hyperxpro]

Both tests use a fresh client's first request, so timeoutExecutor always takes the channel null branch. The channel branch, the whole reason scheduleRequestTimeout grew a third parameter and the only place affinity actually exists, is never exercised: drop the channel argument at the pooled call site and both tests stay green. The RejectedExecutionException fallback in arm() has no test either, and neither does a read timeout under the new mode, which is where isClaimed replaced isExpired.

[hyperxpro]

Both assertions rest on this substring. The timer factory is the pool name plus a timer suffix and the I/O factory is the pool name alone, so a pool name containing timer makes the first test pass for the wrong reason and the second fail, and setting a thread factory on the config drops the suffix entirely and breaks the default case with no bug present. Asserting on the executor rather than the thread name would be steadier. Minor: each test leaves the echo handler sleeping 5s while the server closes, so the class pays that stop timeout twice.