Decide read timeout arming on the nano deadline - #2391

Problem

TimeoutsHolder.startReadTimeout(task, delay) decides whether a read timeout needs a timer entry of its own by checking whether it would be due before the request deadline. It still made that check on the wall clock:

delay < requestTimeoutMillisTime - millisClock.getAsLong()

requestTimeoutMillisTime was DateUtils.unpreciseMillisTime() plus the timeout, taken in the constructor. A step of the wall clock between the two readings changed the answer:

• Step back: a read timeout was armed even though the request deadline comes first.
• Step forward: the read timeout was skipped while the request deadline was still far off. The exchange then had only the request timeout to end it.

#2382 already moved every other check of the request deadline onto nanoClock (requestDeadlineNanos, compared by subtraction). This line kept the wall clock only because #2377 had rewritten it in parallel.

Change

• startReadTimeout compares against untilRequestDeadline() on nanoClock, through a new dueBeforeRequestDeadline(delay).
• The default path (read timeout no shorter than the request timeout) reads no clock at all. A delay no shorter than the whole request timeout can never be due first, so the check returns false before it reads anything. Serve the read timeout from the request timeout's timer entry #2382 keeps the deadline off nanoClock on this path on purpose, and this change keeps it that way.
• Below that threshold the deadline is always on nanoClock:
  • An absolute deadline is set in the constructor, as before.
  • A per-attempt deadline is now also set in the constructor, under the same condition start() uses to arm the entry early: the read timeout is the shorter. It used to be set in start(). But NettyResponseFuture.setTimeoutsHolder makes the holder reachable through the future before it calls start(), so a startReadTimeout in that window would have read an unset deadline. The clock is read the same number of times as before.
  • start() still sets the deadline for an event-loop entry that is not early, which rehomeOn may move.
  • A delay shorter than the read timeout only comes from a read timeout that has already passed this check in full.
• The wall-clock deadline requestTimeoutMillisTime had no readers left, so it is gone, together with its clock read in the constructor and the test-only accessor. TimeoutsHolderTest now checks the deadline through untilRequestDeadline().

Overflow: a request timeout of Long.MAX_VALUE makes millis + timeout overflow. That did not break the old check, because the subtraction cancels the overflow again; a test written for it passes on main. The new code adds the saturated toNanos(Long.MAX_VALUE) to a nano reading, which wraps, and survives for the same reason. The test is kept as a guard for that.

No public API change. Revapi is clean.

Tests

In RequestTimeoutTimerTaskTest:

Other test changes:

• ReadTimeoutTimerTaskTest.aReadTimeoutStillDueBeforeTheRequestTimeoutIsReArmed called startReadTimeout() on a holder that was never started. It ran on the real nano clock, so on the new code it would no longer test what it claims. It now reaches the read timeout's own entry the way an exchange does: the early entry fires before the request is written. It runs on fake clocks.
• The two Long.MAX_VALUE tests build their holder after moving the fake monotonic clock to a positive origin. A real monotonic clock never steps back across a constructor.

Benchmark

RequestTimeoutsArmBenchmark.armAndCancel, JDK 17.0.2, Windows 11, -f 2 -wi 5 -w 2s -i 8 -r 2s -t 1 -prof gc, -Xms256m -Xmx256m -XX:+UseParallelGC. main and the branch were run alternately.

• Default path: no clock read is left in the holder, where main read the wall clock in the constructor. The holder is 8 bytes smaller.
• Early entry: level with main.
• An intermediate version set the per-attempt deadline in start() and measured 5.40 / 5.31 ops/us on the early path. The number of clock reads was the same. With -XX:+PrintInlining the difference shows up as a different inlining tree: the 224-byte constructor is "already compiled into a big method" where the 180-byte one was inlined. It was given up so that a holder reachable before start() reads a set deadline.
• -t 8 was too noisy on this shared machine to read (+-100%), so it is not shown.

Second-agent review

Both reviewers ran read-only in fresh CLI sessions.

Findings and what was done:

• The no-clock-read shortcut silently relied on start() setting the deadline whenever the read timeout is the shorter (round 1, risk). Fixed: the comments say so where the deadline is set and read (79391ae). Round 2's fix then made this structural.
• A holder is reachable through its future before start() (rounds 1 and 2, risk). startReadTimeout there read an unset deadline, and on main the constructor set it. Fixed in 2fdb3aa: the per-attempt deadline is set in the constructor. aReadTimeoutStartedBeforeTheHolderIsStartedStillGetsAnEntry covers it.
• No wall-clock test under an absolute deadline (round 1, nit). Added aWallClockStepForwardDoesNotKeepAReadTimeoutFromStartingUnderAnAbsoluteDeadline.
• nanoOrigin = ONE_HOUR_MS is a millisecond constant used as a nano origin (round 2, nit). Not changed: it mirrors the neighbouring test from Serve the read timeout from the request timeout's timer entry #2382, and the value only needs to be positive.
• "The deadline is not on nanoClock" on the default path (round 3, nit). Not true for event-loop timeouts. Reworded to "need not be" (8f5db61).
• Ambiguous "it" in the javadoc (round 4, nit). Fixed (128b91c).
• The test helper drops a holder without saying why (round 4, nit). Commented (128b91c). The reason was then corrected in round 5 (beadd55).
• aPerAttemptTimeoutGivesTheSecondHopItsOwnBudget would pass with an unset deadline (round 4, nit). Not changed: aPerAttemptExchangeIsNotBoundedAsAWhole next to it asserts the exact budget, as the reviewer noted.
• aReadTimeoutStartedBeforeTheHolderIsStartedStillGetsAnEntry also passes on main (round 5, nit). Not changed: it guards the constructor anchor, not the wall-clock fix, as the table above says.

Claude Code on behalf of Pavel Ptashyts

🤖 Generated with Claude Code