Skip to content

fix(pep): do not fall back on lane-capacity refusals in rescue races - #93

Merged
bojieli merged 1 commit into
mainfrom
fix/rescue-capacity-refusal
Sep 3, 2026
Merged

fix(pep): do not fall back on lane-capacity refusals in rescue races#93
bojieli merged 1 commit into
mainfrom
fix/rescue-capacity-refusal

Conversation

@bojieli

@bojieli bojieli commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Problem

CI on windows-11-arm failed TestPooledFlowsRecoverThroughOneReplacementGeneration/auto after #92:

pool_recovery_test.go:154: successful shared QUIC recovery recorded 1 TCP fallbacks

Reproduced locally under CPU contention (GOMAXPROCS=1, ~25% flake), which surfaced four distinct issues, all rooted in how the new parallel rescue interacts with slow machines:

  1. Capacity refusals fell through to fallback paths (the real bug). Under contention the sprayed racers win their rounds, so the pooled control JOIN arrives after the flow's lane ceiling is full. The resulting errLaneJoinCapacity was treated like any other failure: under --transport quic it fell back to an ordinary QUIC join (an extra dialBulkConn socket, breaking the test's socket ceiling), and under AUTO it triggered the TCP commit — which on localhost wins the race and hands the flow to TCP while a QUIC sibling already rescued it. A capacity refusal is a benign per-attempt outcome (usually a sibling already holds the lane); it now ends the attempt instead of falling back.

  2. queqiao_fallbacks_total counted at commit, not handoff. In a rescue race the AUTO TCP commit can be cancelled after a QUIC sibling wins — the flow never touches TCP but the counter incremented. It's now counted when a TCP rescue lane is actually installed.

  3. The stall watchdog queued signals behind in-flight rounds. A persistent stall re-signals every threshold interval (250ms floor); a signal queued during a multi-second round fired a redundant round the moment the first returned, before the fresh lane could prove itself. The watchdog now stays silent while a round is in flight (rescueInFlight), and the manager drains any signal queued just before the guard engaged.

  4. The test itself needed watchdog isolation. It pins exact dial counts for the recovery it injects; on a slow runner the watchdog legitimately starts rounds of its own (a slow echo is a real stall to it), each opening sprayed dials the ceiling can't budget for. New disableStallWatchdogForTest hook, used by this test. Watchdog behavior remains covered by stallwatch_test.go.

Verification

  • GOMAXPROCS=1 -count=30 clean for both subtests (previously ~25% flake)
  • -race on the stall/grace/race/pool/isolation tests green
  • Full internal/pep suite green
  • Diagnosis was done by tracing every socket creation to its caller frame: the excess came from dialBulkConn (the capacity-fallback path), confirming root cause docs: live comparison against deployed proxies, and a deployment guide #1 before fixing

CI on windows-11-arm (and any slow runner) failed
TestPooledFlowsRecoverThroughOneReplacementGeneration: under CPU
contention the sprayed racers win their rounds, the pooled control JOIN
arrives after the flow's lane ceiling is full, and its capacity refusal
fell through to the fallback paths -- an ordinary QUIC join (an extra
dialBulkConn socket over the test's ceiling) or, worse, the AUTO TCP
commit, which on localhost wins the race and hands the flow to TCP while
a QUIC sibling already rescued it. A capacity refusal is a benign
per-attempt outcome; it now ends the attempt instead of falling back.

Three adjacent corrections from the same investigation:

- queqiao_fallbacks_total is counted when a TCP rescue lane is installed,
  not when the AUTO path commits to TCP: in a rescue race the commit can
  be cancelled after a QUIC sibling wins, and the flow then never touched
  TCP.
- The stall watchdog stays silent while a rescue round is in flight
  (rescueInFlight), instead of queueing a signal that would fire a
  redundant round the moment the first returns; the manager also drains
  any signal queued just before the guard engaged.
- TestPooledFlowsRecoverThroughOneReplacementGeneration disables the
  watchdog via a new disableStallWatchdogForTest hook: the test pins
  exact dial counts, and on a slow runner the watchdog legitimately
  starts rounds of its own that the ceiling cannot budget for.

Verified: GOMAXPROCS=1 -count=30 clean for both subtests (previously
~25% flake), -race subset green.
@bojieli
bojieli merged commit 28ee7ba into main Sep 3, 2026
22 of 23 checks passed
tomatobobot pushed a commit to buyersystem/queqiao that referenced this pull request Sep 4, 2026
TestAutoFlowInstallsTCPRescueAfterAllQUICLanesFail read healthyLanes()[0]
and asked whether that lane was TCP. healthyLanes sorts by lane id and the
QUIC lane is created first, so the original lane holds position zero until
it is evicted -- a TCP rescue could therefore be fully installed while the
check still saw a QUIC lane and the test kept waiting for a state it had
already reached.

That is not a timing tolerance problem, it is the wrong question: the test
means "is there a TCP lane" and asked "does a TCP lane happen to sort
first". It now scans every healthy lane.

The failure was not theoretical. This test failed roughly half of recent
CI runs on ubuntu-24.04 amd64 while passing on the five other platforms,
it failed pull request bojieli#93 before that change merged, and it failed the
build gate of the v0.6.0 release workflow, which is what stopped the tag
from publishing on its first attempt.

The deadline also moves from 5s to 20s. Eight consecutive local runs now
complete in 6.5s total, so the rescue itself takes under a second and the
extra budget costs a passing run nothing; it only stops a loaded runner
from being scored as a failed rescue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HCTFdNai8pNgSyJwV8vGW
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant