Repository navigation
test(adaptive): the #592 settled arm checks its reference window was pinned, and reports VOID and re-arms when it was not - #954
Conversation
… was pinned, and reports VOID and re-arms when it was not (celeris#790) The ratio compares the post-promotion /ping median with the median taken while /kv was still settled. When the kernel's reuseport hash gives the probe's worker no /kv conn (~2^-8 per run), that window is fast and the ratio is 1.0 although every #592 state assertion held. The premise is now checked (ref_ping_med >= queuedBar589 = D/2); a miss is VOID, logged on the RESULT592 line, and the rig is re-armed with a new engine and new conns up to 3 times. Three misses fail as an apparatus failure, worded apart from NOT_FIXED. State failures (promoted_in_bound etc.) still decide first.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe settled-route test now treats an unpinned reference window as VOID, resets the clock stub between attempts, and retries up to three times. It reports an apparatus failure if all attempts are VOID. ChangesSettled-route reference validation and retries
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other · Severity of issue fixed: Low Suggested labels: Merge Risk: ⚪ Minimal · up to Unpinned measurements are retried rather than reported as regressions. No merge-blocking issue remains after normal checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TestAdaptiveSettledRouteRetime592/epoll/settledjudged its speedup ratio without checking the ratio's premise: that the reference window, taken while/kvwas still settled, was pinned. When the kernel's reuseport hash leaves the/pingprobe's worker with no/kvconn (about 2^-8 per run), that window is fast. The ratio then comes out at 1.0, and the run readNOT_FIXEDeven though every #592 state assertion held (CI run 36360645780:ref_ping_med_ms=0.151 ref_queued_frac=0.000).What changes (test file only:
adaptive_settled_retime_linux_test.go)refUnpinned790: a reference window counts as pinned only if its/pingmedian is at leastqueuedBar589(delay), which is D/2, 150 ms at the default D. Pinned windows measure 0.29 to 2.10 s and unpinned ones about 0.05 to 0.15 ms, so the bar sits three orders of magnitude from both worlds. The check lives inassertSettledRetimed592, after the state cases (promoted_in_bound,promoted_after,async_promoted_conns). Those still decide first, so a adaptive dispatch never re-times a settled route: a store-backed handler that turns slow runs inline on the engine worker forever (#493 item 4, measured) #592 regression is never VOIDed.RESULT592 ... verdict=VOID attempt=k/3and aVOID790line with the reason. ThenrunStall589runs again: a new engine and new conns, so new reuseport hashes. Only a non-VOID attempt decides.stubNowNanodoes not nest, so the clock stub is released between attempts (releaseClockStub592). This is safe becauserunStall589has already joined its engine when it returns.apparatus failure, NOT a celeris#592 regression: the rig could not pin a reference window in 3 attempts. At the default placement that is about 2^-24 per run.:255comment is corrected. Placement is the kernel's hash, and the arm now checks it instead of relying on it.Not done here: the issue's optional item 4 (checking placement before the flip, which would also cover the controls). It needs a test-only view of which loop owns each fd, and that is engine code under the #443 freeze.
Evidence
All runs used Docker
golang:1.27linux/arm64, 4 CPUs and memlock 8 MiB (the CI shape, so the io_uring arms skip), withgo test -race -v -run 'TestAdaptiveSettledRouteRetime592/epoll/settled$'. Verdicts are counted from theRESULT592lines.CELERIS_589_KVCONNS=1is the issue's forcing: one/kvconn over 2 workers makes the premise fail on about half the runs.ref_queued_frac=0.000,promoted_in_bound=true, speedup 1.0 to 1.2)On the forced runs, 9 VOIDs in 19 attempts is about 1/2, and 2 apparatus failures in 12 runs is close to the expected (1/2)^3 = 1/8. That shows the apparatus path works. At the default placement it is about 2^-24.
Control (the defect must still fail). The celeris#592 re-opener is disabled by commenting out
r.reopenSettled()inrouter.go. This is a local edit and is not committed.promoted_in_bound=false:/kv was not promoted within 8s)Fixes #790