[FumingPower3925]

Lane L4 EP-READ of v1.6.0, the read path of the epoll engine. Refs #885, Refs #886.

Method: each item's premise was checked on main first, with a test that shows it as an assertion. Commits 1 and 2 are those tests and fail on main; commit 3 is a comment; commits 4 to 7 are one change each (885.17 is last so it can be dropped on its own).

item	verdict	commit
885.41 drainRead has no read bound	premise TRUE (probe + deterministic test), read budget implemented	1, 4
886.25 reclaimTransplant counts a recv twice	premise TRUE, fixed	2, 5
885.28 async gate reads cs.protocol unlocked	premise TRUE for a client that sends before the 101 (race detector, 6 of 6 on main); the conformant client is ordered by x/sys, which is why the issue's probe saw 0	2, 6
885.17 share the inline flush with the replay	done as a refactor (decided 2026-10-12 A), no failing test possible; cost measured	7
885.55 RegisterConn comment	comment only	3

885.41: the probe first (maintainer decision U-15)

TestProbeDrainReadFloodLatency885 (run by hand: CELERIS_PROBE_885=1, it skips by default; the benchmark below measures the same thing, the probe is kept because it prints the control arm). Two loops, docker --cpus 2, golang:1.27.3. Conn A pipelines GET requests in 64 KiB writes as fast as its socket takes them and discards the responses. B is on A's loop (checked through /who, which returns the worker id) and sends one request every 5 ms. C is on the other loop and is the control: a stall C shares is the test process's CPU, not the loop. 5 runs x 2 buffer sizes (8 KiB, 64 KiB) = 10 arms each, under flood:

	B (A's loop) max	arms with B max over 100 ms	C (other loop) max
main	3.91 s, 5.64 s, 1.97 s, 566 ms, 59 ms, 18 ms, 10 ms, 4.6 ms, 1.5 ms, 1.1 ms	4 of 10	0.6 to 22 ms
this PR	10 to 39 ms, every arm	0 of 10	4 to 38 ms

Main's B p50 is also above C's in most arms without a stall (0.39 to 0.71 ms in six of seven, against 0.17 ms); this PR's B p50 is 0.16 to 0.33 ms, C's 0.17 to 0.24 ms. Raw: scratchpad l4epread/out/probe-{base,head}.log.

The stall is the loop: BenchmarkDrainReadFloodBystander885 (below) hit its 30 s read deadline in 1 of 10 interleaved runs of main and in 1 of 10 of the A/A twin of main, and two more runs of the twin had a max of 8 s and 10.5 s: 4 of 20 runs of main's code, against 0 of 10 with the budget (max 25 ms). In a separate series of 12 runs of main (not interleaved) 3 of the first 5 ended with a max of 27 s, 13 s or the 30 s timeout, and the process then did not end in 10 minutes; the same series on the budget build (256 KiB then) had 12 of 12 clean, max 31 ms.

The deterministic gate, TestDrainReadIsBoundedUnderAFlood885: the peer keeps the conn's socket full, and one drainRead call (one readiness event) must return after a bounded number of bytes. On main it reads 26.7 to 27.5 MB in one call under -race and returns only when the watchdog stops the peer after 2 s (5 of 5 runs fail). With the budget it returns after 64 KiB plus one buffer.

885.41: the fix

drainRead counts the bytes of a turn and, at drainReadBudget (64 KiB), re-arms the conn and returns; the loop serves the events of the batch that were ready already, and the conn gets its next turn in the next epoll_wait.

• The re-arm is an EPOLL_CTL_MOD with the conn's own interest (epollInterest(epollOut): EPOLLOUT if armed, no EPOLLIN while recv is paused), through modEpollOut (the hijack guard for async conns). ep_modify re-polls the descriptor and puts it on the ready list if it is readable, so no readiness is lost, and the event carries cs.gen in Pad, so a conn that closed meanwhile, or whose number was reused, is dropped by the existing identity check. This is why it is not the driver's queue (driverReadQ): connState is pooled (a queue of pointers is a reuse hazard), it needs hunks in run() where epoll: the write timeout of a parked response is a minimum rate on acknowledged bytes, no ceiling, inbound bytes are not progress (v1.6.0, Refs #885) #1119 is, and setRecvPaused's resume path already relies on the MOD re-poll.
• The check is at the top of the for, so every continue (partial body, protocol not decided, async hand-off, upgrade) is bounded. Bytes, not reads: BufferSize is 4 KiB to 256 KiB and a read into the body buffer can be larger.
• A closing conn (peerClosed; closeWhenFlushed's mask has no EPOLLIN by design) does not yield.
• A turn that yielded does not run the half-close, error and transplant steps of its event (Loop.readYielded, read by run()): with unread bytes before the FIN, onPeerHalfClose closes the conn and cuts them off. The next event carries EPOLLRDHUP/HUP/ERR again (the MOD re-polls). The first version of this change did not have it; the test below caught it.
• Why 64 KiB and not the driver's 256 KiB: a conn on the loop waits for at most one turn of the flooding conn, and a turn is the parse time of the budget. Sweep of the three sizes, 6 rounds interleaved, medians (BenchmarkDrainReadFloodThroughput885, BenchmarkDrainReadFloodBystander885, 2 vCPU container):

budget	flood throughput (pipelined tiny requests)	4 MiB upload	B mean latency	B p99
16 KiB	265 MB/s	9617 MB/s	109 us	906 us
64 KiB	271 MB/s	9666 MB/s	233 us	624 us
256 KiB	284 MB/s	9080 MB/s	527 us	1483 us

main is 287 MB/s on the flood bench. 256 KiB costs the flooding conn nothing visible but makes B wait about 0.5 ms (more than main's typical 0.3 to 0.4 ms); 64 KiB gives B less than main's typical and costs the flood about 4% (one MOD and one epoll_wait per 64 KiB of parse). The upload reads the body in one read, so it yields once per read, not per 64 KiB. Uploads that are not read straight into the body buffer (the async engine, chunked bodies, a 256 KiB buffer) were measured by the reviewer at 4 to 5% lower single-connection throughput with the budget (1 MiB body async: 5364 to 5151 MB/s; chunked 16K sync: 4682 to 4449 MB/s; the A/A twin within 1%, ranges overlapping), the same order as the 4% of the pipelined flood: the price of the fairness; the budget is unchanged. A budget relative to BufferSize would halve the MOD rate for big buffers (not done).

Benchmarks (docker --cpus 2, arm64, interleaved rounds of main, this PR and an A/A twin of main, medians)

Loop level, BenchmarkDrainReadLoop885 (socketpair, no client; 20 rounds; ns/op):

	main	this PR	A/A twin
1 request per read	1406	1372	1387
16 requests per read	2824	2752	2770

End to end (10 rounds; this PR here is all commits, so it includes flushInline and the gate change):

bench	main	this PR	A/A twin
EpollRecvShapes884/get ns/op	19609	17786	18298
EpollRecvShapes884/post ns/op	15450	15234	16754
KeepAliveDispatch/nodriver ns/op	17843	20362	16490
KeepAliveDispatch/withdriver ns/op	20055	16382	21022
DrainReadFloodThroughput885/pipeline ns per 64 KiB	233374	234840	226345
DrainReadFloodThroughput885/upload ns per 4 MiB	516804	516294	516802
DrainReadFloodBystander885 mean ns	404378	222098	590361
same, p99 ns	792375	674313	2688208

The A/A twin differs from main by up to 16% on the ping-pong benches (two vCPUs shared with the client), so the end-to-end ping-pong rows say "no difference beyond the floor"; the loop-level rows (1 to 3%, this PR faster, the twin 1% faster) are the resolving ones: the budget is an add and a compare per read, flushInline one call per recv. The bystander rows of main and the twin carry the stalls above (the median hides them; the 30 s timeouts are excluded from it).

Guards (pass on main, each checked against a mutant)

The read budget has to give the conn another turn, so these are guards, not failing-first tests: TestDrainReadBudgetGivesTheConnAnotherTurn885, ...AfterADirectBody885, ...KeepsEPOLLOUT885, ...SparesAClosingConn885, TestDrainReadBudgetEndToEnd885 (a 2 MiB pipelined burst or a 1 MiB POST body written at once behind a request that takes 300 ms, so that the burst collects in the kernel behind it (all of it where the socket buffers are not capped, most of it on a CI runner); sync and async engines; with a FIN after the burst on the sync one), TestRunLoopHandlesAHalfCloseAfterAYieldedTurn885 (A yields; then A holds the loop thread in a 300 ms handler while B's request and FIN are both queued behind it, so only the run loop's RDHUP step can end B; without the readYielded = false before drainRead B is not ended, 10 of 10 runs fail, and the head passes 10 of 10; an earlier version of the test without the hold caught the mutant in about 56% of runs, 14 of 25). Mutants, all failing the named test: no MOD (1024 of 3000 answered; 513 of 2001; the end-to-end burst times out at 4096 of 65537, the async one at 4096 of 24577), the MOD with a plain interest (no EPOLLOUT event after the peer takes the response; the end-to-end burst at 50474 of 65537), no peerClosed guard (a closing conn stops after 16384 bytes), no half-close skip (2602 of 6145 answered, then EOF). The tests need a socketpair queue of 210 KB (the default wmem_max), checked with the SO_SNDBUF requests removed.

886.25

reclaimTransplant re-adopts a conn onto its own loop with the carry whose bytes drainRead had counted; the replay counted them again. TestReclaimTransplantDoesNotCountTheCarriedRecvTwice886 (a sync conn at a request boundary with the first bytes of the next request buffered, a refusing target): RequestCount batch is 2 on main (5 of 5), 1 now. attachFD(..., recvCounted); attachAdoptedFD keeps its signature and is the not-counted-yet case (an adoption from another engine counts its first recv here, as #886 item 5 says).

885.28

TestAsyncGateProtocolReadHasAnEdgeWithTheUpgradeSwitch885 (-race): one write carries a request that promotes the conn, an h2c upgrade request, the client preface, SETTINGS and a request on stream 3 whose handler runs inline on the dispatch goroutine after switchToH2Local has written cs.protocol; the test holds it there and sends a PING. On main the detector reports 6 of 6, and two races, not one: the gate's read of cs.protocol and the read of cs.h1State right after it (switchToH2Local sets it to nil; if cs.h1State != nil { cs.h1State.NowNs = now } is a check then a use). A client that waits for the 101 is ordered by x/sys: unix.Write and unix.Read are annotated as one synchronisation, so the flush of the 101 orders it (the issue's 92k-upgrade probe could not report).

Fix: h2Switched, a flag the goroutine sets in switchToH2Local after the three writes, under asyncInMu (taken under detachMu, the documented order, and not across ProcessH2, which runs handlers), and the gate reads under asyncInMu, which it takes right after anyway (lockAsyncInUnlessSwitched): a switched conn is read after that lock, which orders the reads. A bool in the padding byte after asyncPromoted (TestConnStateStaysInItsSizeClass still passes at 448). Cost: for a promoted conn, none on the HTTP/1 path (the lock was taken there already); one lock pair per read for a switched one.

refusesRecv (poolwait_gate_885.go), reached at the top of drainRead once the pool wait's gate has set refuseRecv, read cs.protocol for a promoted conn the same way (found by the review of this PR's first head). Reproduced: TestRefusesRecvReadsProtocolInOrderWithTheUpgradeSwitch885 is the same scenario with a second conn on the loop whose /arm request, run inline on the loop thread, sets refuseRecv; on main (this PR's tests without the fix) the detector reports 5 of 6 runs. Fix: for a promoted conn refusesRecv returns !cs.switchedToH2() (h2Switched under asyncInMu, an instant-long lock the goroutine never holds across a handler, so the gate pass cannot stall on it; lock order is not involved, the pass holds neither lock). Refusing on asyncPromoted alone would refuse the HTTP/2 conns the pool wait serves, so a switched conn is not refused, an HTTP/1 one is, as before. The other loop-thread readers of protocol and h1State were checked by the reviewer and are ordered (closeIdleH1 and the sweep under detachMu, tryTransplant after the park read under asyncInMu). io_uring's protocol is an atomic and its h1State read goes through a TryLock.

stopDetachedProducers (the gate pass over the detached conns, from #1116) read cs.h1State before taking detachMu, while the switch sets it to nil: the same race, found by the second review. TestStopDetachedProducersReadsH1StateInOrderWithTheUpgradeSwitch885 (a request on a second conn runs the pass inline on the loop thread while the goroutine is in the switch) is red 10 of 10 on the code before the fix, in separate -race processes, and green 10 of 10 after. Fix, the pattern #1123 uses for io_uring's twin: a promoted conn's h1State is tested under the TryLock, a conn the loop owns before it; the pass still never waits for a lock the goroutine holds across a handler. A PING ACK assertion after the 101 in the three edge tests (the switched conn is served past the gate) fails against a gate that refuses every promoted conn.

Found and not changed: the bytes a client sends during the switch (before the 101) are appended to asyncInBuf and dropped by the asyncInBuf[:0] after it; and the loop thread blocks on detachMu for as long as the goroutine's ProcessH2(info.Remaining) runs a handler inline (a loop stall during an async h2c switch, on main as here).

885.17

flushInline is the block of drainRead after the handler (flush, resync of pendingBytes, EPOLLOUT or dirty list, OnError, refused write); flushCarriedReplay is a one-line call of it, and the "stays a copy on purpose" paragraph is gone. The direct-body block of drainRead is not this one and stays (it has no OnError, no removeDirty on an error and no disarmEpollOut when everything went out; sync-only by its gate); the doc says so. The error and h2c blocks were shared by #1015 (h1verdict.go). The old objection was a call per recv: measured above, 1 to 3% faster than main at the loop level (within the floor). The existing #908 and write-clock tests guard it. Commit 7 is separate so it can be dropped (commits 8 and 9, after review round 1, add the refusesRecv test and fix and the readYielded guard; comments only on the skips of the ERR/HUP and transplant steps, which are conservative); after review round 2: the stopDetachedProducers test (red on the code before) and the PING ACK assertion, a deterministic half-close test, then the stopDetachedProducers fix and a corrected comment on the ERR/HUP skip: an RST does not purge the receive queue on Linux, the next turn reads the bytes and then the error) if a reviewer reads the cost differently.

885.55

RegisterConn's comment now says what keeps the first byte (the registration's generation in Pad, driverCandidate, and lookupDriver waiting on driverMu, which is held from the insert to the ADD); the aa2331d wording was already in the test's comment.

Release notes

• epoll: a connection whose peer keeps its socket full (a deep pipeline, a large upload) no longer holds its event loop: each read turn ends after 64 KiB and the connection is re-armed, so the other connections on the loop wait for at most one turn.
• epoll: the dispatch goroutine's switch of a connection to HTTP/2 and the loop's reads of the connection's protocol, in the read gate and in the shutdown pool wait, are ordered (a client that sends HTTP/2 frames before it has read the 101 was a data race).
• epoll: a transplant hand-off the target refuses no longer counts the connection's carried request twice in RequestCount.

What to attack

• A conn's turn ends with unread bytes; anything in run() after drainRead that assumes the socket is drained: I found RDHUP/HUP/ERR and tryTransplant (both skipped for the turn). EPOLLOUT is not skipped. onPeerHalfClose's deferred paths and drainDetachQueue read no socket.
• The MOD while the dispatch goroutine of an async conn runs: modEpollOut's hijack guard, and epollOut being loop-thread state.
• recvPaused conns and WebSocket/SSE (detached) conns yielding: the interest comes from epollInterest; the detached backpressure setRecvPaused after the inline flush is in the same turn and unchanged.
• The h2Switched gate for a conn that is promoted and has been handed back as HTTP/2 (asyncH2Promoted consumed by drainDetachQueue): the flag stays set until release; releaseConnState resets it.
• 64 KiB as a constant for every BufferSize and protocol (HTTP/2 conns go through drainRead too): one extra MOD and epoll_wait per 64 KiB of a saturated conn. A big-buffer deployment might want it larger; it is a package variable only for the tests.

Leads found by the guards (all present on main, none changed here)

• Async engine: a conn whose pipelined burst (about 2 MiB of tiny requests) produces more response bytes than the write cap before the dispatch goroutine flushes is closed (the goroutine answers the whole batch first).
• Async engine: a burst that ends in a FIN while the goroutine is still in the first request is answered not at all (0 of 6145), with or without the budget.
• Sync engine: a 2 MiB pipelined burst ending in a FIN is answered in part (48853 of 65537) on main and with an unbounded budget alike; 192 KiB is answered whole.
• Sync engine, the same burst ending in a FIN: the client can get a response cut off mid-header (a single write in closeOnReadEnd before the close), not only fewer responses (a truncated response is worse than a missing one).
• Async engine: a 1 MiB POST followed by a FIN while the goroutine is busy loses the POST (1 of 2 answered).
• Async engine: a keep-alive conn uploading 4 MiB bodies one at a time is closed after 4 to 56 uploads, on main and here. Likely cause, not confirmed: the backpressure close in drainRead (len(asyncInBuf)+len(data) > maxPendingInputBytes) where maxPendingInputBytes is 4 MiB, the same as the default body limit.
• Bytes a client sends during the h2c switch are dropped (see 885.28).
• A refused hand-off of a conn whose last recv was read straight into the body buffer (never counted by drainRead) and overshot into a partial pipelined request now counts 0 recvs where main counted 1 (RequestCount only; needs sync mode, a drain, a refusing target). Not changed: it needs a per-conn "last recv counted" record.

Verification (head after review round 2): ./internal/engine/epoll/... with -race (230 s, ok), the new tests -count=10 -race (ok), golangci-lint 0 issues; earlier: (226 s, ok) on the rebased branch (main baa43d9); the new tests -count=20 -race (ok); golangci-lint (0 issues). Local runs in docker golang:1.27.3, --cpus 2, one container at a time; arm64.

[coderabbitai]

Caution

Review failed

The pull request is closed.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 89.28571% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/engine/epoll/loop.go	92.30%	5 Missing ⚠️
internal/engine/epoll/poolwait_gate_885.go	60.00%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!