test(harness): de-collide e2e ports — per-fork ranges + AddrInUse respawn - #713
Conversation
…pawn The harness picked ports by binding :0, reading the assignment, and closing the socket, then handed the port to aisix to re-bind a spawn later. Under vitest's two parallel forks that check-then-use window races: a port picked in one fork can be re-issued to the other (or grabbed by a mock server) first. Observed twice in two days as e2e startup flakes: aisix exits 1 with AddrInUse while the readiness probe sees a different instance answer 404 on the stolen port, then ECONNREFUSED (sls-content-capture on PR #710, datadog-exporter on main). - ports.ts: allocate from a disjoint per-fork range (VITEST_POOL_ID, pid fallback) with a monotonic cursor + bind-verify, below the kernel's ephemeral range so :0 assignments never overlap it. Call sites unchanged. - app.ts: retry spawnApp with fresh ports (max 3 attempts) when aisix died at startup to AddrInUse — the residual race vs external processes; any other startup failure still surfaces immediately. - app.ts: keep the stderr HEAD in the failure excerpt — anyhow prints the error line before its backtrace, and the old tail-only slice cut exactly the line naming the failure, which is why the CI flake was undiagnosable from the job log. The respawn race itself is not deterministically testable (concurrency timing); the allocator and the failure classifier are unit-tested in src/harness/ports.test.ts, and the full 117-file suite passes with the new allocator.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1 participant
Problem
Recurring e2e startup flake (twice in two days:
sls-content-capture-responses-completionson #710's run,datadog-exporteron main run 28595570164):pickFreePortbinds:0, reads the assignment, closes the socket, and hands the port toaisixto re-bind a process spawn later. Under vitest's two parallel forks that check-then-use window races: the kernel can re-issue the port to the other fork (or a mock server) first. ThelastStatus=404-then-ECONNREFUSEDprobe shape is the tell — the probe was answered by a different instance that had stolen the port, while ouraisixdied to AddrInUse.Fix
ports.ts: allocate from a disjoint per-fork port range (VITEST_POOL_ID, pid fallback outside vitest) with a monotonic in-process cursor and a bind-verify probe. Ranges sit below the kernel's ephemeral range (32768+), so:0assignments can never overlap them. No call-site changes.app.ts:spawnApprespawns with fresh ports (max 3 attempts) only whenaisixexited at startup with AddrInUse — the residual race vs unrelated external processes. Any other startup failure still fails the test immediately.app.tsdiagnostics: keep the stderr head in the failure excerpt. anyhow printsError: …before its backtrace; the old tail-only slice cut exactly the line naming the failure, which is why the CI flake logs only showed backtrace frames.Tests
src/harness/ports.test.ts: allocator range/uniqueness/bindability, occupied-port skip, and the AddrInUse failure classifier (positive + negative).