Skip to content

lsqb: give supervision waits room above scheduler noise - #23

Merged
alexy merged 1 commit into
mainfrom
work/lsqb-timing-windows
Sep 21, 2026
Merged

alexy merged 1 commit into
mainfrom
work/lsqb-timing-windows

Conversation

@alexy

@alexy alexy commented Sep 20, 2026

Copy link
Copy Markdown
Member

The observation-worker tests assert how the coordinator reacts inside wall-clock windows of tens of milliseconds. On a starved host a process can be descheduled for longer than the window, and the test reports a supervision failure that did not happen — four such failures are recorded in docs/LSQB_RUNNER_TIMING_FLAKES.md, three of them in a single run on a documentation-only commit.

What changed

Waits that mean this bounded wait ends now get an order of magnitude over scheduler noise, named once rather than repeated as literals: READY_AND_REAP_MS = 10_000, REAP_GRACE_MS = 1_000.

Three numbers deliberately stay small, because they are the scenario rather than the tolerance:

  • the 25 ms deadline in ready_execution_plan_survives_hard_timeout_without_a_result_record, whose test asserts the deadline fires
  • the 50 ms reap in hung_cleanup_cannot_hold_the_coordinator_indefinitely, whose test asserts the reap gives up
  • the 200 ms query deadline in post_result_cleanup_is_recovery_not_query_time, against a 300 ms cleanup sleep — the point being that cleanup is not query time

Widening those would delete the tests rather than stabilise them.

Every run(...) in plan_tests.rs now reports through unwrap_or_else(|error| panic!("supervision failed: {error}")) instead of unwrap().

Verdict

ci-local: PASSED every gate at c72fb55 on Linux x86_64 in 1860s

Host grust, Linux x86_64, own detached worktree, fetch-upstream.sh run first.

What this evidence does and does not show

It does not show the flake is fixed. These tests passed on main this afternoon with the tight windows on this same host, so there is no local failure for the change to repair. Its justification is the four recorded runner failures and the 0.4-CPU reproduction in the doc.

I ran that reproduction, before and after, and it did not discriminate: at --cpus=0.4 both binaries failed 12 of 12, and at --cpus=1.0 both failed 8 of 8 — where the documented environment saw 0 failures in 27 runs at one CPU. This host has run five full gates today and saturates at quotas the recorded environment handled comfortably. At that severity the failures are not about windows at all:

supervision failed: failed to spawn observation worker
assertion failed: error.contains("pipes remained open")

I stopped rather than raise the quota until the old binary passed, because a regime found by tuning until the result appears is not a control.

Two things the reproduction did establish:

  1. The new error reporting works — that supervision failed: failed to spawn observation worker line is this change, and the doc's evidence table has a row reading "message not captured".
  2. Two assertions depend on which error string the supervisor produced rather than on any window (pipes remained open, READY). Widening cannot help those, and on a runner they would look like the same flake while being something else. Worth separate attention; not addressed here.

Per board #5, the workflow's retry loop stays until a starved run survives. This PR does not ask for it to be removed.

This is G3.

The observation-worker tests assert how the coordinator reacts inside wall-clock
windows of tens of milliseconds. On a starved host a process can be descheduled
for longer than the window, and the test reports a supervision failure that did
not happen: four such failures are recorded in docs/LSQB_RUNNER_TIMING_FLAKES.md,
three of them in one run on a documentation-only commit.

Waits that mean this bounded wait ends now get an order of magnitude over
scheduler noise, named once as READY_AND_REAP_MS and REAP_GRACE_MS rather than
repeated as literals. Three numbers stay small because they are the scenario
rather than the tolerance: the 25 ms deadline whose test asserts the deadline
fires, the 50 ms reap whose test asserts the reap gives up, and the 200 ms query
deadline against a 300 ms cleanup, whose point is that cleanup is not query time.

Every run() in plan_tests reports through unwrap_or_else rather than unwrap, so
the next failure names its cause instead of printing Err.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alexy added a commit that referenced this pull request Sep 20, 2026
The PR carries the verdict and the limits of its evidence, including that the
starved reproduction did not discriminate on this host and that two assertions
fail on error strings rather than windows.

Raises one question for quegee before Q5: whether bca732c sits inside the pin
range the sweep measures, since its PageRank regression would then be inside the
measurement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alexy
alexy merged commit ce97148 into main Sep 21, 2026
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