Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ only the control plane, not the segment frames.
it (unset or `poison` keeps the default); the library itself never exits the process
(see the options entry under Changed). `SegmentStore::wait_until_poisoned()` and
`subscribe_poison()` let an embedder do the same.
- **A panic during a control-plane write ends the process** (robustness track ROB-2, release
plan C12, C22). `weftdb::exec::control_plane_write` already marks the region that the whole
`segment_index.db` write path runs in; the server now acts on it. `weft_server::panic_hook::install()`
is installed at startup immediately after the tracing subscriber, chained to the hook it
replaces so the panic is still reported with its message and location. A panic whose
panicking task is inside that region is written to stderr and the process
`std::process::abort()`s: the exit carries a signal a supervisor sees (SIGABRT) instead of
an HTTP status, and the restart that follows reopens the store under the engine's own crash
recovery, which is the component that can tell whether a commit was torn. A panic anywhere
else is reported and left unwinding, because wave 6's ROB-3 catch layer is the only layer
that catches panics. The hook does not write the poison marker — most of the region reads
before it writes, so a marker there would turn a benign panic into an outage lasting until
an operator deletes a file. `weft-server/tests/control_plane_panic.rs` proves both halves
by re-executing the test binary as a child, the way `poison_exit.rs` does.
- **Store format marker and migration registry** (freeze design §4.3, FRE-12a). A
segment store root now carries a `STORE_FORMAT` file (plain JSON: `layout_version`,
`min_read_layout`, `min_write_layout`, `last_written_layout`, `migrating_to`,
Expand Down Expand Up @@ -69,9 +83,10 @@ only the control plane, not the segment frames.
- `weftdb::exec::control_plane_write` and `weftdb::durable::poison_global()`
(robustness track ROB-2): every segment-index transaction runs inside the
task-local control-plane write scope, so a panic hook can tell a panic in the middle
of a control-plane write from one in a read, and the process-wide poison it would set
refuses every write of every store, as a store's own poison does, while reads go on.
It refuses every store open too (`weftdb::Poisoned`), since an open writes.
of a control-plane write from one in a read. `poison_global()` is a process-wide
poison that, once set, refuses every write of every store, as a store's own poison
does, while reads go on. It refuses every store open too (`weftdb::Poisoned`), since
an open writes.
- `LOCK.holder` records the holder's host name and boot id beside its pid, and a
second opener's error names the host (`… in use by pid 1 on host db-2 (session …)`).
The short wait for a lock that a child being spawned still shares now applies only to
Expand Down
2 changes: 1 addition & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili
- [x] → **D-S9** Write-once maintenance II: overlap merge, squash, compaction (+ P1/P2 output ids) — #72
- [x] **X-ERR** One error envelope and code table in `error.rs` (absorbs SEC-2, FRE-6a, ROB-1, TAG-0's envelope) — *#90: `weft-server/src/error.rs` (685 lines) is the one module that names a refusal. `Code`'s 42 variants are asserted by name **and** status against the plan's §4 table, and the nine synonyms that table drops (`invalid_request`, `unauthorized`, `store_unconfigured`, `maintenance_busy`, `limit_exceeded`, `panic`, `segment_invalid`, `not_found`, `too_large`) are asserted absent, so a later lane cannot quietly reintroduce one. The envelope is `{"error","code","request_id"}` plus optional `details`/`limit`/`committed` on **every** path: `StrictJson`/`StrictQuery` replace axum's extractors, so a wrong `Content-Type` is `unsupported_media_type` and a malformed body or query is `invalid_json`/`bad_request` instead of axum's empty 400, and `Router::fallback` answers `route_not_found` naming the method, the path, where the aspect list is and where `docs/INTEGRATION.md` is. Classification is by **type**: `classify_read`, `classify_ingest` and `classify_typed` downcast `weftdb::InvalidAspectName`, `weftdb::StoreError`, `SealError`, `ReduceError` and `SketchError`, and the four `SegmentStore` seal wrappers (`weftdb/src/types/segment_store.rs:2782/2798/2814/2826`) now wrap with `anyhow::Error::from(error).context(format!("seal failed: {shown}"))` — `anyhow`'s `Display` prints only the outermost context, so the sentence stays byte-identical while `downcast_ref::<SealError>()` starts working; a reworded wrapper can no longer move a status. Two message rules survive, each commented with the slice that retires it: `no declared schema` (D-S13 types it as `UnknownAspect`, a store-lane edit) and `out-of-order timestamp` (`weft-arrow-store/src/lib.rs:233`'s untyped `bail!`, D-S10/D-S14). `ConvertError` is direction-dependent, so there are two classifiers and `weft-server/tests/error_contract.rs` asserts both halves of the same `Parquet(…)` — a client `400` ingesting, a server `500` reading. Statuses split where §4 names a different condition: `ToleranceExceeded` is `422 value_not_representable` with `details.max_abs_error`/`details.tolerance` (was a `400` carrying a `seal failed` sentence), `Encode(EncodeError::NotFinite)` is `400 not_finite` with `details.row`, a name the store cannot address is `400 invalid_aspect_name` with `details.aspect`/`details.reason`, a store this binary will not open is `503 incompatible_store` (exit 65 in the §4 C7 table), and a downsample `ReduceError` is 422/400 instead of 500. Two findings worth keeping: `weft_physical_type::EncodeError` names only `Overflow` and `NotFinite`, so a wildcard arm beside them was dead code and is gone — a new arm is meant to stop the build where someone picks the §4 code; and `ApiError::message()` returns the cause **by design** (it is the sentence for the log), so the sanitisation tests assert `wire_message()` is the code's fixed text — `internal` "the server could not complete the request", `incompatible_store` "the store on disk uses a format this server cannot open". `axum`'s `CatchPanicLayer` is dropped (FRE-6a: a panic is a process death with the exit code a supervisor sees, not a body), and `ConvertError`/`ReduceError`/`SketchError` gained `#[non_exhaustive]`.*
- [x] → **X-NAME** `aspect_name::encode` and `validate_new` at HTTP declare (absorbs FRE-7a, SEC-0) — *partly on main: `validate` and the declare check (#66), `encode` (#72). #91 is the rest: `weftdb::aspect_name::validate_new` (`weftdb/src/types/aspect_name.rs`) runs `validate`'s chain and then refuses an ASCII uppercase letter with a new `AspectNameReason::UppercaseAsciiLetter` (the enum is `#[non_exhaustive]`, so the variant is not a breaking change). Order is asserted: the capital rule runs **after** `reason()`, so `../Price` is refused for escaping the directory, not for its capital — the more dangerous reason is never hidden. The hole is real because the legacy frame file is `<aspect>-<id>.weftseg`, which writes the name literally, and NTFS, APFS (by default) and ext4 with `casefold` fold ASCII case: two aspects differing only in case share a file, whichever seal lands second overwrites the other's bytes, and reads serve one series' values for the other's name. `encode` (#72) removes that collision for the frames D-S10 will write; this closes the declaration of a new collision. Applied at **creation only** — `POST /api/v1/storage/aspects` — so `SegmentStore::declare`, every read, seal, maintenance and store-wide sweep keep plain `validate` and a store that already holds `Price` stays readable, writable and compactable (the test asserts both halves: the refusal, then a raw catalog declare of `Price` followed by a 201 write). Two things are deliberately **not** refused: the characters only Windows forbids (`< > : " | ? *`), because `job:rate5m` is blessed on Unix by this module's own docs and is a shape in use, and the cross-platform hazard belongs one layer down where `encode` escapes `:` for write-once frame names (D-S10, the slice the plan names for it); and Unicode NFC/NFD aliases, which need `unicode-normalization` this crate does not carry, while `encode` escapes every non-ASCII byte anyway. A refusal keeps the #90 shape: `400 invalid_aspect_name` with `details.aspect` and `details.reason`.*
- [ ] **ROB-2** Storage-engine panic fail-stop: a turso-origin panic aborts
- [x] → **ROB-2** Storage-engine panic fail-stop: a turso-origin panic aborts — *#92: the write scope was already here — `weftdb::exec::control_plane_write`, and every `IndexTxn` operation runs inside it, which weftdb's own `every_op_runs_inside_the_control_plane_write_scope` asserts — so C12's "hook and knob" was the half that was missing, and it lives in the binary: `weft_server::panic_hook::install()` chains the hook it replaces, and when `in_control_plane_write()` says the panicking task held the scope it names the panic and calls `std::process::abort()`: SIGABRT for a supervisor, not the poison path's exit 70, and never an HTTP response (FRE-6a). Startup calls it immediately after `init_tracing`, because `set_hook` hands back the hook it replaced. It deliberately does not write the poison marker: that region reads before it writes (`txn.plans()`, `SegmentIndex::open`), and the marker survives the panic until an operator deletes a file. Nothing is caught, so ROB-3 stays the only catch layer and read-path panics still answer 500; the legacy `weftdb/src/types/aspect.rs` lane is ROB-5's. Proved by `weft-server/tests/control_plane_panic.rs`, which re-executes the test binary the way `poison_exit.rs` does.*
- [ ] **PAC-3** Library crate hygiene: no global allocator or wgpu features imposed on library users
- [ ] L-FMT: **X-FRAME** Frame registry, v7 envelope, `read_frame*`/`describe_frame`, `FrameFault` classes (absorbs FRE-4, TAG-2)
- [ ] Legacy lane: **FRE-14** Legacy data-directory marker, numbered migrations and measurement upgrade test
Expand Down
1 change: 1 addition & 0 deletions weft-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub mod ingest;
pub mod interpolate;
pub mod maintain;
pub mod metrics;
pub mod panic_hook;
pub mod reconcile_daemon;
pub mod settings;
pub mod state;
Expand Down
81 changes: 81 additions & 0 deletions weft-server/src/panic_hook.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//! The process-wide panic hook (robustness track **ROB-2**, release plan C12, C22).
//!
//! A panic in the middle of a control-plane write is not like a panic in a read: Turso's
//! pager may be part way through a commit, its locks do not poison, and the task that
//! panics is holding a `&mut` borrow of state that the rest of the process still believes
//! is consistent. Unwinding lets the request finish as a 500 and lets the *next* request
//! answer as if nothing happened — a write that was half-issued reporting success, which is
//! the condition C22 exists to prevent. So:
//!
//! - a panic while the task is inside [`weftdb::exec::control_plane_write`] is reported on
//! stderr and then the process **aborts**, so the exit carries a signal a supervisor sees
//! (SIGABRT) instead of an HTTP response (FRE-6a); the restart that follows opens the
//! store under the engine's own crash recovery, which is the component that actually
//! knows whether a commit was torn;
//! - any other panic is reported and left unwinding — ROB-3's catch layer (wave 6) is the
//! only layer that catches panics, and it needs unwinding to stay reachable.
//!
//! Two things this deliberately is not:
//!
//! - It does not write the poison marker (`weftdb::durable::poison_global`). A panic inside
//! the region can precede any commit at all — most of the region reads before it writes —
//! so the marker would turn a benign panic into an outage lasting until an operator
//! deletes a file. Failing the process loudly is the fail-stop; judging the store is left
//! to the open-time verification that already runs on every start.
//! - It does not catch anything, and the library still never exits a process: this hook
//! lives in `weft-server`, the same split as the poison path, where `weftdb` reports and
//! the host decides how the process ends.
//!
//! Install it from the binary at startup, **after** the tracing subscriber: the hook chains
//! the hook it replaces, so ordering decides who prints the panic.

use weftdb::exec::in_control_plane_write;

/// Whether [`install`] has run in this process. Startup calls `install` unconditionally, and
/// a test that installs the hook first would otherwise replace it and print the panic twice
/// through two chained hooks.
static INSTALLED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);

/// Install the ROB-2 panic hook, chaining whatever hook is already installed so the panic
/// itself is still reported with its message and location.
///
/// Idempotent: a second call does nothing, which is what lets a test install the hook
/// directly and the binary's startup path skip it.
pub fn install() {
if INSTALLED.swap(true, std::sync::atomic::Ordering::SeqCst) {
return;
}
let previous = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
// First report, in the form the operator expects: message, file, line, and the
// span the panic happened in.
previous(info);
if in_control_plane_write() {
// Written to stderr rather than traced: the panicking task may be holding the
// pager's state, and this line has to reach whoever reads the crash log even
// when the logging pipeline is itself what failed. One line, so a log shipper
// that splits on newlines cannot separate the reason from the panic.
eprintln!("weft-server: the control plane was being written when this panic happened — aborting the process: {}", shown(info));
std::process::abort();
}
}));
}

/// The panic's payload and location, for the abort line.
///
/// Payload types are arbitrary and their `Display` can panic, which inside a panic hook
/// would abort the hook itself, so an unusable payload is reported as its type instead.
fn shown(info: &std::panic::PanicHookInfo<'_>) -> String {
let payload = info.payload();
let payload = if let Some(text) = payload.downcast_ref::<&str>() {
(*text).to_owned()
} else if let Some(text) = payload.downcast_ref::<String>() {
text.clone()
} else {
"a panic whose payload could not be printed".to_owned()
};
match info.location() {
Some(location) => format!("{payload} (at {}:{})", location.file(), location.line()),
None => payload,
}
}
5 changes: 3 additions & 2 deletions weft-server/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ pub const VARS: &[VarSpec] = &[
VarSpec { name: "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process the process-wide-poison test spawns: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" },
VarSpec { name: "WEFT_TEST_OPEN_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process the open-recovery test spawns: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" },
VarSpec { name: "WEFT_TEST_ORPHAN_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process that seals a frame and crashes before its commit: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" },
VarSpec { name: "WEFT_TEST_PANIC_CHILD_MODE", tier: Tier::Test, summary: "Names which half of the control-plane panic test the child process is exercising, so its parent can tell a deliberate abort from a real crash. No shipped binary reads it.", accepts: "`inside` panics while the control-plane write scope is held; `outside` panics in an ordinary task", default: "unset: the child returns and the test binary exits 0", read_by: "weft-server/tests/control_plane_panic.rs (test child)" },
VarSpec { name: "WEFT_TEST_POISON_EXIT_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child `weft-server` the poison-exit test re-invokes: the store root it opens, so the parent can point the child at the poisoned store.", accepts: "a directory path", default: "unset", read_by: "weft-server/tests/poison_exit.rs (test child)" },
VarSpec { name: "WEFT_TEST_RACE_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process that reconciles while a squash races it: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store.rs (test child)" },
VarSpec { name: "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", tier: Tier::Test, summary: "Set only in the child process whose reconcile is merged by a racing squash: the store root it opens.", accepts: "a directory path", default: "unset", read_by: "weftdb/types/segment_store/swap_tests.rs (test child)" },
Expand Down Expand Up @@ -363,7 +364,7 @@ mod tests {
assert!(!value.trim().is_empty(), "{} describes its {field} as empty", spec.name);
}
}
assert_eq!(VARS.len(), 51, "the registry's size is asserted so an entry added or dropped is a deliberate act");
assert_eq!(VARS.len(), 52, "the registry's size is asserted so an entry added or dropped is a deliberate act");
}

/// The registry's whole point: it cannot drift from the code. A name read but not
Expand Down Expand Up @@ -400,7 +401,7 @@ mod tests {
let experimental: Vec<&str> = VARS.iter().filter(|spec| spec.tier == Tier::Experimental).map(|spec| spec.name).collect();
assert_eq!(experimental, ["WEFT_GPU_CALIBRATE", "WEFT_SEGMENT_TRANSPOSED_MAX_OVERHEAD"], "both are knobs whose value is unproven (GPU calibration's thresholds, and a codec the measurements retired)");
let harness: BTreeSet<&str> = VARS.iter().filter(|spec| spec.tier == Tier::Test).map(|spec| spec.name).collect();
let allowlist: BTreeSet<&str> = ["SKIP_SLOW_TESTS", "TEST_DATA_DIR", "WEFT_FAULT", "WEFT_FAULT_TEST_CHILD", "WEFT_REQUIRE_GPU", "WEFT_TEST_AMBIGUOUS_CHILD_ROOT", "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", "WEFT_TEST_OPEN_CHILD_ROOT", "WEFT_TEST_ORPHAN_CHILD_ROOT", "WEFT_TEST_POISON_EXIT_CHILD_ROOT", "WEFT_TEST_RACE_CHILD_ROOT", "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", "WEFT_TEST_S8_SPLIT_RACE_ROOT", "WEFT_TEST_S9_DROPPED_SWAP_DIR", "WEFT_TEST_S9_SQUASH_RACE_DIR", "WEFT_TEST_S9_SUFFIX_RACE_DIR", "WEFT_TEST_SWAP_MATRIX", "WEFT_TEST_SWAP_MATRIX_DIR", "WEFT_TEST_TXN_CHILD_INDEX", "WEFT_BENCH_BACKUP_DIR", "WEFT_BTC_CSV", "WEFT_DB_NEW_CRASH_CHILD", "WEFT_LEGACY_CONSUMER_CRASH_CHILD", "WEFT_LEGACY_INGEST_CRASH_CHILD", "WEFT_VERBOSE"].into_iter().collect();
let allowlist: BTreeSet<&str> = ["SKIP_SLOW_TESTS", "TEST_DATA_DIR", "WEFT_FAULT", "WEFT_FAULT_TEST_CHILD", "WEFT_REQUIRE_GPU", "WEFT_TEST_AMBIGUOUS_CHILD_ROOT", "WEFT_TEST_GLOBAL_POISON_CHILD_ROOT", "WEFT_TEST_OPEN_CHILD_ROOT", "WEFT_TEST_ORPHAN_CHILD_ROOT", "WEFT_TEST_PANIC_CHILD_MODE", "WEFT_TEST_POISON_EXIT_CHILD_ROOT", "WEFT_TEST_RACE_CHILD_ROOT", "WEFT_TEST_S8_CONFLICT_CHILD_ROOT", "WEFT_TEST_S8_SPLIT_RACE_ROOT", "WEFT_TEST_S9_DROPPED_SWAP_DIR", "WEFT_TEST_S9_SQUASH_RACE_DIR", "WEFT_TEST_S9_SUFFIX_RACE_DIR", "WEFT_TEST_SWAP_MATRIX", "WEFT_TEST_SWAP_MATRIX_DIR", "WEFT_TEST_TXN_CHILD_INDEX", "WEFT_BENCH_BACKUP_DIR", "WEFT_BTC_CSV", "WEFT_DB_NEW_CRASH_CHILD", "WEFT_LEGACY_CONSUMER_CRASH_CHILD", "WEFT_LEGACY_INGEST_CRASH_CHILD", "WEFT_VERBOSE"].into_iter().collect();
assert_eq!(harness, allowlist, "the Test tier is the plan's allowlist: a channel a test harness sets for the child it spawns, and nothing else may sit in it");
assert_eq!(VARS.iter().filter(|spec| spec.tier == Tier::Tuning).count(), VARS.len() - stable.len() - experimental.len() - allowlist.len(), "every remaining name is Tuning: a knob whose value is deployment-specific and whose number is still moving");
}
Expand Down
Loading
Loading