Skip to content

Read admission: a check-phase stall sheds the next poll phase reads while a poll-phase stall does not; state the bound per phase and pin it #3800

Open

Description

@kriszyp

Post-#3234 residual (merged 2026-10-30). The read-admission probe stamps loopIterationStart from a setImmediate (server/throttle.ts probeLoopIteration), i.e. at the start of the check phase, and admission compares now - loopIterationStart. Synchronous handler work runs in the poll phase before that stamp and counts as the previous iteration, so the next poll phase's reads are served; setImmediate / async-continuation work runs after the stamp, so the next poll phase's reads see the whole stall and are shed (measured: 200 ×10 vs 503 ×10 for the same 400 ms stall). server/DESIGN.md ~845 ("after a one-off stall the next iteration's reads are served rather than shed") is true only for work inside the admitted call; the same paragraph's "including timers and async continuations" half is accurate, and the two sentences do not say they describe different phases.

Ask (author's call, docs-first): state the bound per phase in server/DESIGN.md §HTTP read admission (work inside the admitted call bounds admission; check-phase work counts against the next poll phase's reads; one poll phase can admit a whole deferred-work batch), and pin the poll-phase vs check-phase result with two cases in unitTests/server/httpRequestQueue.test.js (a spin inside the handler serves the next read; a setImmediate spin sheds it). Changing the stamp point (timers phase) would stop counting async-continuation work, which the design counts on purpose. Impact at the 5000 ms default needs a >5 s check-phase stall; at tuned low limits any setImmediate work longer than the limit sheds the next poll phase's reads.

What happens

One worker, http.readRequestQueueLimit: 50, http.requestQueueLimit: 0, Node http. A stall request goes out on connection 1, then 100 ms later a GET /Ping/ on connection 2, which waits about 300 ms in its socket. 10 repetitions per kind, 200 ms idle between them:

Stall (400 ms) Ping result
SyncStall.get() spins inside the handler (poll phase) 200 ×10
ImmediateStall.get() returns, then setImmediate(() => spin(400)) (check phase) 503 ×10, Service unavailable, exceeded request queue limit
idle control (no stall) 200 ×10

The wait is the same in both cases (ping latency 301–310 ms), but the outcome depends on which event-loop phase the stall ran in. In the check-phase case the stall has already finished when the read is parsed.

A related shape comes from the same stamp point. An async handler (await new Promise(setImmediate), then 10 ms of CPU) on an idle loop admits 63–64 of 64 simultaneous GETs, about 640 ms of CPU in one iteration against a 50 ms limit. A synchronous 10 ms handler admits 6–10 of 64. This was reproduced by two independent runs. Under sustained 2x open-loop load the async handler's iterations carry 8–10 admitted reads (80–100 ms), and the admitted p50 stays healthy (about 40 ms against an 11 ms unloaded p50).

Mechanism

  • server/throttle.ts:77-83: probeLoopIteration stamps loopIterationStart when it runs. It is the first immediate queued, so the stamp lands at the start of the check phase.
  • server/throttle.ts:93-99: admission compares now - loopIterationStart against the limit, sampled at admission time.
  • Synchronous handler work runs in the poll phase, before the probe's stamp. It counts as the previous iteration, so the next poll phase's reads are served.
  • setImmediate and async-continuation work queued after the probe runs after the stamp. The next poll phase's reads therefore see the whole stall and are shed.
  • Async handler bursts: for the same reason, an async handler does no CPU before its first await, so now does not advance while a whole poll-phase batch is admitted.

What this does not prove

  • The stall-phase repro ran on Node http only. uWS goes through the same throttleRequests → rejectWhenLoopDelayed and showed the same burst admission, but the stall-phase spec was not run on uWS or Bun.
  • It does not show how often Harper's own internals put work longer than the limit into the check phase.
  • There is no base A/B, because read admission does not exist on merge base 5ed056a6.
  • The burst-admission half is a design limit of sampling past delay, not something a fix must remove.

— Claude Mythos 5.1 (finding triage)

Activity

  1. added theissue type on Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    Priority

    P3

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions