Skip to content

fix(chaos): bound the sigkill mid-stream kill point by construction (#2841) - #2842

Merged
devarismeroxa merged 1 commit into
mainfrom
fix/chaos-sigkill-midstream-2841
Aug 29, 2026
Merged

fix(chaos): bound the sigkill mid-stream kill point by construction (#2841)#2842
devarismeroxa merged 1 commit into
mainfrom
fix/chaos-sigkill-midstream-2841

Conversation

@devarismeroxa

@devarismeroxa devarismeroxa commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What

tests/chaos/sigkill_test.go's mid-stream SIGKILL case had no production cap and no watermark gate: it estimated its crash window with arithmetic (killAfterReads=95 reads at paceMS=15 = "one ~1s flush landed, a second pending"), then SIGKILLed. Under parent descheduling (a stall), the child keeps producing through the gap between observation and signal, so the crash lands far past the window the case claims to test — and because every value in the range yields the same gap-free verdict, the case passes vacuously and never flakes. Identical defect to #2836's property2, which #2840's stall injection made fail. (#2841)

The fix applies the #2835/#2840 bracket pattern exactly:

  • Lower bound (observed): the kill is gated on the child's durable upstream commit watermark reaching 70 (waitForUpstreamCommittedAtLeast), not on parent-observed stdout READ progress. The watermark only moves when a persister flush lands, so the gate cannot fire before a checkpoint is durably persisted, however long the parent is stalled.
  • Upper bound (structural): holdAt=100 caps the first child's production via fix(chaos): bound property2 kill points by construction, not by a race #2840's merged CONDUIT_CHAOS_HOLD_AT seam (merged onto main while this branch was in flight — reused, not duplicated). Nothing past 100 can ever be produced, acked, or committed, and the capped child never exits on its own, so the kill can never miss a dead process.
  • Every value in [70, 100] yields the identical verdict — bounding a test, not tuning one.
  • Preconditions are asserted, not trusted: a kill landing outside the bracket fails the test (watermark < 70, or > 100), and resumePos > holdAt is fatal.
  • The deterministic cap regression test is fix(chaos): bound property2 kill points by construction, not by a race #2840's TestChild_HoldAt_CapsProductionBelowTotal (property2_test.go, same seam, already merged) — this PR deliberately adds no duplicate; its only cap coverage is the case-level detectors above.

This PR is tests/chaos/sigkill_test.go only. Net diff vs main: +124/−30 in one file.

Proof (all runs on this machine, both prune classes where noted)

(a) Pre-fix + injected 2.5s stall (between gate and SIGKILL): case PASSED — vacuously, with the kill point blown through: watermarkAtKill=192/193, producedAtKill=254/255 vs the claimed ~95–133 window. With a temporary bracket assertion enabled, both classes FAIL deterministically: SILENT WINDOW-SKIP DEMONSTRATED (pre-fix). This is the silent-window-skip the issue describes.

(b) Post-fix + same 2.5s stall: PASS on both prune classes (TestSIGKILL_PruningUpstream_NoGap, TestSIGKILL_DurableUpstream_NoGap). The watermark gate fires after a provably-durable flush and the ceiling holds.

(c) Perturbation (cap removed): deterministic FAIL on both classes — #2840's regression test times out waiting for HELD (10s), and mid-stream fails precondition violated: ... watermark at kill time (257) exceeded the production ceiling holdAt (100). A future edit that removes the bound identifies itself.

Verification

  • go test -race -count=3 -shuffle=on ./tests/chaos/... — green (exact CI invocation; post-rebase run 362.5s)
  • golangci-lint v2.12.2 (pinned) on ./tests/chaos/... — clean
  • go vet ./tests/chaos/... — clean
  • Rebasing onto the now-merged fix(chaos): bound property2 kill points by construction, not by a race #2840 collapsed the seam files to main's exact content: git diff origin/main shows only sigkill_test.go (no duplicate env seam, no duplicate test symbol)

Failure-mode analysis

  • What could break: the mid-stream case itself (wrong values → false fail/true pass), or other scenarios if the seam leaked behavior to them. The seam is opt-in (0 disables; every other scenario leaves it 0, and only the FIRST child gets it — the resumed child runs uncapped to total). parseChildEnv (merged, fix(chaos): bound property2 kill points by construction, not by a race #2840) makes a bad combo a loud child-side fatal, which surfaces as a parent-side test failure, never a silent wrong run. Chaos CI job (tests/chaos (race, x3), required on main) shows a regression as a failing check; rollback is reverting the case's two fields.
  • Adversarial self-review (post-rebase, fresh): re-read the full diff after dedupe — kill-gate switch (mid-snapshot keeps its READ-count gate + persistDelayMS structural precondition untouched), precondition asserts bracket [70, 100], soft resumePos > holdAt detector, resumed-child cap isolation, no new exported symbols. Stated, not assumed: the bracket's upper end depends on the first child being killed before it could ever produce past 100 — which is structural (produceLoop stops at 100), and the precondition assert turns any violation into a failure, not a vacuous pass.

Roadmap

Issue #2841 — chaos-suite hardening under the arch-v2 graduation workstream (3c: crash-window coverage). No production code touched; tests/chaos only.

Risk tier: Tier 3 (chore — tests/chaos only, same tier as #2835/#2840: no production code, no data path, no public contract).

@devarismeroxa
devarismeroxa requested a review from a team as a code owner August 29, 2026 20:19
@devarismeroxa devarismeroxa added this to the v0.20.0 milestone Aug 29, 2026
…2841)

The mid-stream SIGKILL case estimated its crash window with arithmetic
(killAfterReads=95 at paceMS=15: one ~1s flush landed, a second pending)
then SIGKILLed. Under parent descheduling the child keeps producing
through the observation-to-signal gap, so the crash lands far past the
window the case claims to test - and since every landing point yields
the same gap-free verdict, the case passes vacuously and never flakes
(the identical defect #2836 diagnosed for property2).

Apply the #2835/#2840 bracket pattern exactly:

- Lower bound (observed): gate the kill on the child's durable upstream
  commit watermark reaching killAfterCommitted=70
  (waitForUpstreamCommittedAtLeast) instead of parent-observed READ
  progress - the watermark only moves when a flush lands, so the gate
  cannot fire before a checkpoint is durably persisted.
- Upper bound (structural): holdAt=100 caps the first child's production
  via #2840's CONDUIT_CHAOS_HOLD_AT seam (merged on main after this
  branch started - reused, not duplicated). Nothing past it is ever
  produced, acked, or committed, and the capped child never exits on its
  own, so the kill can never miss a dead process.
- Every value in [70, 100] yields the identical verdict.
- Preconditions asserted, not trusted: a kill landing outside the
  bracket, or a resume position past the ceiling, fails the test. The
  deterministic cap regression test is #2840's
  TestChild_HoldAt_CapsProductionBelowTotal (property2_test.go) - this
  PR adds no duplicate.

Proven: (a) pre-fix + 2.5s stall passes vacuously with the kill point
blown through (watermark 192/193, produced 254/255 vs the claimed
~95-133 window; a temporary bracket assertion fails both prune classes
deterministically - the silent window-skip); (b) post-fix + same stall
passes on both prune classes; (c) cap removed fails deterministically
on both (HELD-marker timeout, and precondition watermark 257 > holdAt
100).

Verification: go test -race -count=3 -shuffle=on ./tests/chaos/... green
(362.5s), golangci-lint v2.12.2 clean, go vet clean.
@devarismeroxa
devarismeroxa force-pushed the fix/chaos-sigkill-midstream-2841 branch from 243548f to a42c4be Compare August 29, 2026 20:27
@devarismeroxa
devarismeroxa merged commit b0a6bf3 into main Aug 29, 2026
11 checks passed
@devarismeroxa
devarismeroxa deleted the fix/chaos-sigkill-midstream-2841 branch August 29, 2026 20:37
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