Skip to content

ROB-2 — end the process when a control-plane write panics - #92

Merged
physics515 merged 2 commits into
xname/declare-validationfrom
rob2/engine-panic-failstop
Oct 21, 2026
Merged

physics515 merged 2 commits into
xname/declare-validationfrom
rob2/engine-panic-failstop

Conversation

@physics515

Copy link
Copy Markdown
Collaborator

What this closes

ROB-2 in wave 2: "Storage-engine panic fail-stop: a turso-origin panic aborts."

Robustness section C12 sets the rule for where the machinery lives: the abort machinery
"lives in weftdb, which holds the engine types, but its hook and its knob live in the
binary
", and C22 says what has to happen: a storage-engine-origin panic while a write
is in flight ends the process instead of being answered. The plan's own split of the work is
"ROB-2 ships the engine-origin abort now; ROB-5 wraps legacy metadata writes", and C12
insists that "ROB-3's catch is the only layer" — so this slice adds the abort, and adds no
catch layer.

The half that was already here

weftdb::exec::control_plane_write (weftdb/src/types/exec.rs) is the write-scope flag the
plan asks for — "wraps every control-plane operation, and the write-scope task-local flag
set for its duration" — and it already wraps the whole control-plane write path: every
IndexTxn operation runs through run_while_open's retry loop inside it, which is why the
ops cannot be forgotten one by one. weftdb's test
every_op_runs_inside_the_control_plane_write_scope asserts exactly that. It is a task-local
rather than a thread-local, so the mark follows the task across await points and between
tokio worker threads, and the hook runs synchronously inside the poll that panicked, where
the mark is readable.

So the missing half was the hook, and the hook belongs to the binary.

What this adds

weft-server/src/panic_hook.rs, one purpose, with install():

  • chains the hook it replaces, so the panic is still reported with its message, file, line
    and span;
  • if weftdb::exec::in_control_plane_write() says the panicking task was inside the region,
    writes one line to stderr naming the decision and calls std::process::abort();
  • otherwise returns, leaving the panic unwinding.

weft-server/src/startup/hooks.rs gains init_panic_hook() and run() calls it immediately
after
init_tracing(). That order is not cosmetic: std::panic::set_hook replaces the
installed hook and hands the replaced one back, so installing ours first would leave the
tracing subscriber's hook calling ours and print every panic twice. install() is idempotent
for the same reason — a test that installs it directly does not change what startup does.

Three decisions worth arguing with

Abort, not exit 70. The poison path has a deliberate exit status (70,
weftdb::durable::AMBIGUOUS_COMMIT_EXIT_CODE, via WEFT_ON_AMBIGUOUS_COMMIT=exit) because an
ambiguous COMMIT is a known condition with a known operator action. A panic in the middle
of a write is not: nobody asked for that exit, and a supervisor distinguishes an abnormal
signal death from a status the program chose. FRE-6a's point is that the failure must not
arrive as an HTTP response; SIGABRT is what a supervisor sees, and
AMBIGUOUS_COMMIT_EXIT_CODE stays specific to the ambiguity it names.

No poison marker. The tempting move is to call weftdb::durable::poison_global() before
aborting. Deliberately not done, and the code says why: most of the control-plane region
reads before it writes — txn.plans() prepares inside the scope, SegmentIndex::open runs
inside it — so a panic inside the region does not imply a torn commit. The marker persists in
the store directory and makes the next start refuse writes until an operator deletes a
file, which would convert a benign panic into an outage. What the abort buys is the
restart, and the restart already runs the open-time verification that can actually judge the
store, with the engine's own crash recovery in front of it.

Nothing is caught. A panic outside the region is reported and left to unwind, so wave 6's
ROB-3 catch layer remains the only layer that catches panics, and read-path panics keep
answering 500. The legacy lane (weftdb/src/types/aspect.rs) is untouched — the plan gives
that to ROB-5.

Verification

weft-server/tests/control_plane_panic.rs checks both halves from outside the library. The
abort ends whatever process runs it, so each case re-executes the test binary as a child with
one test selected, the way poison_exit.rs does:

  • inside the region, through weftdb::exec::control_plane_write, with a yield_now between
    entry and panic so the mark is read from a later poll — the child dies by SIGABRT (6),
    its stderr carries the panic and the abort line, and it never reaches its post-panic
    print;
  • outside the region, in a spawned task — the child exits 0: the chained hook still
    printed the panic, the abort line is absent, and the process kept running.

Gates on this tree: cargo +stable check --workspace --all-targets --locked, clippy --workspace --all-targets --all-features with the CI correctness/suspicious/perf gates,
cargo +stable test -p weft-server --lib, RUSTDOCFLAGS="-D warnings" cargo +stable doc --workspace --no-deps --locked (CI's rustdoc job, which failed on #90 and is now in every
slice's gate set), and cargo +nightly fmt --all -- --check. The integration test itself
runs as cargo test -p weft-server --test control_plane_panic.

Notes

  • weftdb's exec.rs module doc now says what the hook does with the flag, and
    weft-server/src/startup/mod.rs's phase list names the panic hook, so the ordering
    requirement is visible where the wiring is.
  • CHANGELOG.md records the behaviour under Added, including the no-marker decision.
  • The test child's WEFT_TEST_PANIC_CHILD_MODE is registered in weft-server/src/settings.rs's
    VARS under Tier::Test, because
    settings::tests::every_environment_name_the_workspace_reads_is_registered walks every crate
    and fails any environment name that is read without a VarSpec — it is the guard that caught
    this test's new name, and the registry's size assertion moved 51 → 52 with it.
  • One-line follow-up, not this PR: a debug assertion that install() has run before the
    router serves, so a future refactor cannot quietly drop the hook from startup.

ROB-2 in wave 2, per release plan C12 and C22: an engine-origin panic while a write is in
flight ends the process instead of being answered.

`weftdb::exec::control_plane_write` was already the write-scope flag the plan asks for, and
it already wraps the whole control-plane write path — every `IndexTxn` operation runs inside
it, which weftdb's own `every_op_runs_inside_the_control_plane_write_scope` asserts — so the
missing half was the hook, and C12 puts the hook in the binary: `weftdb` holds the engine
types but never exits a process. `panic_hook::install` chains the hook it replaces, and when
`in_control_plane_write()` says the panicking task was inside the region it names the panic
and calls `std::process::abort()`. Startup calls it immediately after `init_tracing`, because
`set_hook` hands back the hook it replaced: installing ours first would leave the tracing
subscriber's hook calling ours and print every panic twice.

Abort rather than the poison path's exit 70: that status is chosen by a known condition with
a known operator action, and FRE-6a's point is that this failure must not arrive as an HTTP
response, so the exit should carry a signal a supervisor sees. The hook also deliberately does
not write the poison marker. Most of the region reads before it writes — `txn.plans()`
prepares inside the scope, `SegmentIndex::open` runs inside it — so a panic there does not
imply a torn commit, and the marker persists in the store directory until an operator deletes
a file, which would turn a benign panic into an outage. The abort buys the restart, and the
restart already runs the open-time verification that can judge the store.

Nothing is caught: a panic outside the region is reported and left unwinding, so wave 6's
ROB-3 catch layer stays the only catch layer and read-path panics keep answering 500. The
legacy metadata lane (`weftdb/src/types/aspect.rs`) is untouched; the plan gives that to ROB-5.

`tests/control_plane_panic.rs` checks both halves in a child process, the way `poison_exit.rs`
does: inside the region, through `control_plane_write` with a yield between entry and panic,
the child dies by SIGABRT with the panic and the reason on stderr; outside it, in a spawned
task, the child exits 0 — the chained hook still printed the panic, the abort line is absent,
and the process kept running.
… process

The box's wording names the mechanism ("a turso-origin panic aborts") but not the boundary it
hangs on, so the note records that the scope was already in the library (`weftdb::exec`'s
control-plane write flag, with weftdb's own test proving every index-transaction operation
runs inside it) and that #92 supplied only the hook C12 puts in the binary — plus the two
decisions a reviewer should be able to disagree with: abort rather than the poison path's
exit 70, and no poison marker for a panic that may have happened during the reading half of
the region.

The arrow marks the split the plan keeps: ROB-5 wraps the legacy metadata lane
(`weftdb/src/types/aspect.rs`), which this change leaves alone.
@physics515
physics515 merged commit 29fa247 into xname/declare-validation Oct 21, 2026
18 checks passed
@physics515
physics515 deleted the rob2/engine-panic-failstop branch October 21, 2026 15:30
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