Skip to content

test(engine): wait for every client before counting accepts in the #662 queued-accept rig - #680

Merged
FumingPower3925 merged 3 commits into
mainfrom
test/pause-accept-queued-count
Sep 19, 2026
Merged

FumingPower3925 merged 3 commits into
mainfrom
test/pause-accept-queued-count

Conversation

@FumingPower3925

Copy link
Copy Markdown
Contributor

Summary

Test-only fix. Main's Unit job went red on c4d1cb5 (run 35428669128) in engine/epoll:

--- FAIL: TestPauseAcceptQueuedControl (0.42s)
    pause_accept_queued_linux_test.go:199: pause=false workers=2 blockers=3 queued=8 outcomes=map[200:8] accepts=10 (before release 2) closes=0 errors=0 onConnect=10
    pause_accept_queued_linux_test.go:214: AcceptCount = 10, want 11 (3 blockers + 8 queued): every connection the engine takes off an accept queue must be counted
    pause_accept_queued_linux_test.go:262: after client close: active=0 accepts=11 closes=11 onDisconnect=11 errors=0

Cause. The rig holds every loop inside a blocking handler.

  • With 2 loops, it took 3 dials to get both loops into the handler. The second dial landed on a loop that was already held, so it waited in that loop's accept queue. That is why accepts before the release = 2.
  • After the release, the test waited only for the 8 queued connections' responses, then read AcceptCount.
  • Nothing orders the extra blocker's accept before those 8 responses. If every queued connection hashes to the other loop, all 8 can answer while the held loop has not yet come back to its queue. The count then reads 2 + 8 = 10.
  • The log's own last line shows the connection was accepted and counted right after: 11 accepts, 11 closes.

The engine is fine; the test raced its own measurement. The io_uring copy of the rig (engine/iouring/pause_accept_queued_linux_test.go) has the same gap. It cannot trip in CI's one-worker Unit shape, but it can at two workers.

Fix. Both copies now also wait for every blocker's response before reading the metrics. The blockers' responses are still unscored. A response exists only after its connection was accepted and counted: AcceptCount is an atomic add at accept time, at engine/epoll/loop.go:939 and engine/iouring/worker.go:1913. So once every client has a response, the check is exact, with no polling and no new timing.

Proof, on the runner

The commits go up in order:

  1. The fix plus a throwaway mutant in engine/epoll/loop.go that makes each loop skip counting its first accept, the smallest real miscount. CI on this head must fail the two queued-accept tests on the count checks. This shows the fixed test still catches a lost accept.
  2. The mutant removed. CI must pass.

The run links will be added below. The squash merge carries no mutant.

This waits to merge until the #657 PR-2 branch has opened its PR, because both touch engine/.

… queued-accept rig

runPauseQueued662 (epoll and io_uring) asserted AcceptCount ==
blockers + queued right after the queued connections answered. A blocker
that landed on a loop already held sits in that loop's accept queue until
the loop comes back, and nothing orders its accept before the queued
connections' responses: when every queued connection hashes to another
loop, all of them can answer first and the count reads one short. It
happened once on main's Unit job (c4d1cb5, epoll, the control arm):
"AcceptCount = 10, want 11", accepts before release 2, blockers 3, and 11
after the clients closed.

Both copies now read every blocker's response too (still unscored)
before reading the metrics. A response exists only after its connection
was accepted and counted (the counter is an atomic add at accept time in
both engines), so the count check is exact with no polling.
Each epoll loop skips counting its first accept. The fixed queued-accept tests must fail on this commit; the next commit removes it.
@FumingPower3925 FumingPower3925 added this to the v1.6.0 milestone Sep 19, 2026
@FumingPower3925 FumingPower3925 added the testing Testing infrastructure and helpers label Sep 19, 2026
Restores engine/epoll/loop.go byte-for-byte to main; the branch now differs from main only in the two test files.
@FumingPower3925

Copy link
Copy Markdown
Contributor Author

Proof on the runner, as described above:

This merges once the #657 PR-2 branch has opened its PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Testing infrastructure and helpers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant