From b0dec9c3b055b1bfb8640fc4e8e98c2f6989fab9 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 05:22:36 -0500 Subject: [PATCH 01/12] =?UTF-8?q?feat(weftdb):=20X-NAME=20=E2=80=94=20chec?= =?UTF-8?q?k=20an=20aspect=20name=20at=20creation,=20not=20only=20on=20use?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1.0 plan X-NAME (absorbs FRE-7a, SEC-0). `encode` (#72) and `validate` plus the declare check (#66) are already on main; the gap this closes is the rule that can only be applied to a name that is not in use yet. `aspect_name::validate_new` is `validate` plus: no ASCII uppercase letter. `price` and `PRICE` are one file name on a case-insensitive or case-folding filesystem (NTFS, APFS by default, ext4 with `casefold`), and the legacy `-.weftseg` scheme writes the aspect name into that name literally, so two aspects would share one frame file and the second one's frames would be read as the first's. The module docs already called that collision out as something the encoded write-once names remove; enforcing it at creation removes it for the names written today. `POST /api/v1/storage/aspects` is the only caller: declaring is where a name is born. Reads, seals, maintenance and the store-wide sweep keep plain `validate`, so an aspect that a older version put in the catalog with a capital in it is never stranded. The characters only Windows forbids (`< > : " | ? *`) are deliberately NOT added to the creation rules on Unix. `job:rate5m` is a name in use, aspect_name.rs blesses it on Unix, and `encode` escapes `:` one layer down, which is where D-S10 makes the frame name; the plan moves the containment check into D-S10 for the same reason. Unicode NFC/NFD aliasing is left as documented: `encode` escapes every non-ASCII byte, so new frame names cannot collide that way, and rejecting it at declare would need a normalisation table this crate does not carry. --- CHANGELOG.md | 14 ++++++ weft-server/src/declare.rs | 53 +++++++++++++++++++-- weftdb/src/types/aspect_name.rs | 84 +++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c40eba3..6d681b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -244,6 +244,20 @@ only the control plane, not the segment frames. process death with the exit code the supervisor sees, not a response body (FRE-6a). `ConvertError`, `ReduceError` and `SketchError` gained `#[non_exhaustive]`, so the mapping can grow without breaking a semver-minor release. +- **An aspect name is checked twice — at creation and on use** (1.0 plan X-NAME; absorbs FRE-7a, + SEC-0). `weftdb::aspect_name::validate_new` adds the rules that only a name not in use yet can + still be held to: no ASCII uppercase letter, because `price` and `PRICE` are one and the same + file name on a case-insensitive or case-folding filesystem (NTFS, APFS by default, ext4 with + `casefold`), and the legacy `-.weftseg` scheme writes the aspect name into that name + literally — two aspects, one file, and the second one's frames are read as the first's. Only + `POST /api/v1/storage/aspects` applies it, because declaring is where a name is born: reads, + seals, maintenance and the store-wide sweep keep plain `validate`, so a store that already holds + `Price` is still readable, writable and compactable. A refusal carries + `code: "invalid_aspect_name"` and `details.reason`. The characters only Windows forbids + (`< > : " | ? *`) are deliberately **not** refused at creation on Unix while the aspect name is + still part of the file name — `job:rate5m` is a name in use — and that cross-platform hazard is + closed one layer down instead, where `aspect_name::encode` escapes `:` for the write-once frame + names (D-S10). ### Changed diff --git a/weft-server/src/declare.rs b/weft-server/src/declare.rs index 61d35415..7405b936 100644 --- a/weft-server/src/declare.rs +++ b/weft-server/src/declare.rs @@ -109,9 +109,11 @@ fn schema_from_request(request: &DeclareAspectRequest) -> Result, StrictJson(request): StrictJson) -> Result { let store = state.store().cloned().ok_or(ApiError::unconfigured())?; drop(state); @@ -119,7 +121,13 @@ pub async fn declare_aspect(State(state): State, StrictJson(request): // store's `segments/` directory before anything is written. This has its own code in the §4 // table, so a client branches on `invalid_aspect_name` and on `details.reason` rather than on // a sentence shared with every other 400 this route can send. - weftdb::aspect_name::validate(&request.name).map_err(|err| error::aspect_name_refusal(&err))?; + // + // Declaring is where an aspect name is born, so this is the one place the stricter + // `validate_new` applies (1.0 plan X-NAME): it adds no capital letters, because `price` and + // `PRICE` would be one file name on a case-insensitive filesystem. Everywhere else — every + // read, seal, maintenance call — keeps plain `validate`, so an aspect declared before this + // rule was written is never stranded by it. + weftdb::aspect_name::validate_new(&request.name).map_err(|err| error::aspect_name_refusal(&err))?; let schema = schema_from_request(&request)?; let result = store.declare(&request.name, &schema).await; drop(store); @@ -274,4 +282,41 @@ mod tests { drop(store); assert!(frames_under(dir.path()).is_empty(), "no frame was written anywhere"); } + + /// X-NAME: a rule only a name that is not in use yet can satisfy is enforced at declare, and + /// reported through `code` and `details`. The rules every existing name already satisfies are + /// not tightened here, and an aspect whose name carries a capital from an older version is + /// still addressed, sealed and read exactly as before. + #[tokio::test] + async fn declare_refuses_a_name_only_a_new_aspect_must_keep() { + let dir = TempDir::new().unwrap(); + let store = Arc::new(SegmentStore::open(dir.path().join("store")).await.expect("opens store")); + let state = AppState::new().with_store(store.clone()); + for name in ["Price", "PRICE", "CPU_load"] { + let body = serde_json::json!({ "name": name, "physical_type": "f64", "timestamp_unit": "seconds" }); + let (status, body) = post_json(app_with_state(state.clone()), "/api/v1/storage/aspects", &body).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{name:?} must not be creatable; body: {body}"); + assert_eq!(body["code"], "invalid_aspect_name", "body: {body}"); + assert_eq!(body["details"]["aspect"], name, "body: {body}"); + assert!(body["details"]["reason"].as_str().unwrap().contains("uppercase"), "the reason says which rule: {body}"); + } + let declared = store.list_declared_aspects().await.unwrap(); + assert!(declared.is_empty(), "a refused declare writes nothing: {declared:?}"); + // A `:` stays ordinary on Unix, in the creation rules as in the use rules. + if cfg!(unix) { + let body = serde_json::json!({ "name": "job:rate5m", "physical_type": "f64", "timestamp_unit": "seconds" }); + let (status, body) = post_json(app_with_state(state.clone()), "/api/v1/storage/aspects", &body).await; + assert_eq!(status, StatusCode::CREATED, "a `:` is an ordinary character on Unix: {body}"); + } + // A name an older version put in the catalog directly is grandfathered: the stricter rule + // is only ever the creation rule. + let schema = weft_physical_type::AspectSchema::new(weft_physical_type::PhysicalType::F64, "0".parse().unwrap(), weft_physical_type::TimeUnit::Seconds); + store.catalog().declare(store.database(), store.subject(), "Price", &schema).await.expect("raw declare"); + let points = serde_json::json!({ "points": [{ "timestamp": 0, "value": "1" }] }); + let (status, body) = post_json(app_with_state(state.clone()), "/api/v1/storage/Price/points", &points).await; + assert_eq!(status, StatusCode::CREATED, "an aspect that already has a capital in its name is still writable: {body}"); + assert_eq!(body["row_count"].as_u64(), Some(1), "the sealed frame is described: {body}"); + drop(state); + drop(store); + } } diff --git a/weftdb/src/types/aspect_name.rs b/weftdb/src/types/aspect_name.rs index efab24ee..a42fe9f8 100644 --- a/weftdb/src/types/aspect_name.rs +++ b/weftdb/src/types/aspect_name.rs @@ -52,6 +52,14 @@ //! in uppercase hex, so two names that differ in case or normalisation encode to file //! names that differ under case folding and normalisation too. Maintenance outputs take //! these names from slice S8 on; seals from S10. +//! +//! A name that is being **created** passes [`validate_new`] as well, which adds the rules that +//! can only be applied while a name is still new — no ASCII uppercase letter, so `price` and +//! `PRICE` cannot both be created and then take one file name on a case-insensitive filesystem. +//! The other candidate rule, refusing on every platform the characters only Windows forbids +//! (`< > : " | ? *`), is deliberately *not* taken while the aspect name still appears literally +//! in the legacy `-` name: `job:rate5m` is a name in use on Unix, and the +//! cross-platform problem is fixed one layer down, where [`encode`] escapes `:`. use std::fmt; @@ -100,6 +108,10 @@ pub enum AspectNameReason { /// On Windows, the name contains a character Windows does not allow in a file name /// (`<`, `>`, `:`, `"`, `|`, `?` or `*`). WindowsForbiddenCharacter, + /// The name contains an ASCII uppercase letter. Only [`validate_new`] refuses this: an + /// aspect already declared with such a name keeps it, but a new one could collide with + /// another aspect's file name on a case-insensitive or case-folding filesystem. + UppercaseAsciiLetter, } impl fmt::Display for AspectNameReason { @@ -115,6 +127,7 @@ impl fmt::Display for AspectNameReason { Self::TrailingDotOrSpace => f.write_str("it ends with `.` or a space"), Self::ReservedDeviceName => f.write_str("it is a Windows reserved device name (CON, PRN, AUX, NUL, CONIN$, CONOUT$, COM0-9, LPT0-9)"), Self::WindowsForbiddenCharacter => f.write_str("it contains a character Windows does not allow in a file name (< > : \" | ? *)"), + Self::UppercaseAsciiLetter => f.write_str("it contains an uppercase ASCII letter, which could take another aspect's file name on a case-insensitive filesystem"), } } } @@ -159,6 +172,8 @@ impl std::error::Error for InvalidAspectName {} /// Check that `name` is safe to use as an aspect name, which also names the aspect's /// files on disk. See the [module documentation](self) for the rules. /// +/// A name being **created** is checked by [`validate_new`] as well. +/// /// # Errors /// /// An [`InvalidAspectName`] naming the first rule `name` breaks. @@ -166,6 +181,34 @@ pub fn validate(name: &str) -> Result<(), InvalidAspectName> { reason(name, cfg!(windows)).map_or(Ok(()), |reason| Err(InvalidAspectName { name: name.to_string(), reason })) } +/// Check that `name` is one this store will **create**, not merely one it can address. +/// +/// Every rule [`validate`] applies, plus the ones that can only be enforced while the name is +/// still new — a rule that a name already on disk would break strands an aspect whose data is +/// already written, so those rules live here rather than in [`validate`]. Today there is one: +/// no ASCII uppercase letter. `price` and `PRICE` are one file name on a case-insensitive or +/// case-folding filesystem (NTFS, APFS by default, ext4 with `casefold`), and the legacy +/// `-.weftseg` scheme writes the aspect name into that name literally, so the two +/// aspects cannot both be addressed everywhere a store may be copied. +/// +/// Declaring a name is where one is born, so that is the only caller that must satisfy this; +/// every later operation on an aspect calls [`validate`], so an aspect declared before a +/// new-name rule exists keeps working. 1.0 plan X-NAME. +/// +/// # Errors +/// +/// An [`InvalidAspectName`] naming the first rule `name` breaks, [`validate`]'s rules first. +pub fn validate_new(name: &str) -> Result<(), InvalidAspectName> { + match reason(name, cfg!(windows)) { + Some(reason) => Err(InvalidAspectName { name: name.to_string(), reason }), + // After `reason`, so a name that both escapes a directory and carries a capital is + // refused for the escape: the more dangerous of the two, and the one an operator would + // otherwise notice last. + None if name.chars().any(|c: char| c.is_ascii_uppercase()) => Err(InvalidAspectName { name: name.to_string(), reason: AspectNameReason::UppercaseAsciiLetter }), + None => Ok(()), + } +} + /// Whether `byte` stands for itself in [`encode`]'s output: `[a-z0-9_]`. const fn is_literal(byte: u8) -> bool { byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_' @@ -430,4 +473,45 @@ mod tests { assert_eq!(err.name(), "../x"); assert_eq!(err.to_string(), "invalid aspect name \"../x\": it starts with `.`"); } + + /// X-NAME: the rules that apply only to a name that is not in use yet. `validate` keeps its + /// lenience on purpose — a new rule must not strand an aspect whose data is already written. + #[test] + fn a_new_name_may_not_collide_with_another_in_case() { + for name in ["Price", "PRICE", "CPU_load", "Temp1", "ΔT PRICE"] { + assert_eq!(validate(name), Ok(()), "{name:?} is addressable, so an aspect declared with it before this rule keeps working"); + assert_eq!(validate_new(name).expect_err(name).reason(), AspectNameReason::UppercaseAsciiLetter, "{name:?} may not be created"); + } + for name in ["a", "price", "temp_1", "cpu.usage_idle", "load-avg", "Δt", "温度", "con-x", "\u{394}\u{3a4}"] { + assert_eq!(validate_new(name), Ok(()), "{name:?} should be creatable"); + } + } + + /// A non-ASCII capital is not an ASCII one, and [`encode`] escapes it rather than folding it, + /// so it is not part of the collision class this rule closes either. + #[test] + fn only_ascii_case_is_refused() { + // "ΔΤ": Greek capital delta and Greek capital tau. It reads as "DT", and a human typing it + // after looking at a `dt` aspect will not be refused — but nothing folds it onto `dt`, + // because NTFS, APFS and ext4 `casefold` fold ASCII only. + assert_eq!(validate_new("\u{394}\u{3a4}"), Ok(())); + // The rule keys on the ASCII letter, whatever stands next to it. + assert_eq!(validate_new("\u{394}T").expect_err("refused").reason(), AspectNameReason::UppercaseAsciiLetter); + assert_eq!(validate_new("PRICE"), Err(InvalidAspectName { name: "PRICE".into(), reason: AspectNameReason::UppercaseAsciiLetter })); + assert!(validate_new("PRICE").expect_err("refused").to_string().contains("uppercase"), "the message says which rule: {}", validate_new("PRICE").expect_err("refused")); + } + + /// The new rule must not bury the refusals that matter more: the first rule a name breaks is + /// still the one [`reason`] finds first. + #[test] + fn the_new_rule_never_hides_a_more_dangerous_one() { + for name in ["../Price", "a/b/Temp", "C:\\Price", ".CON", "a ", "", "price.", "CON"] { + assert_eq!(validate_new(name).expect_err(name).reason(), validate(name).expect_err(name).reason(), "{name:?}"); + } + // `:` stays a Unix-ordinary character in the creation rules too: it is the frame name that + // needs escaping, and `encode` escapes it. + if cfg!(unix) { + assert_eq!(validate_new("job:rate5m"), Ok(())); + } + } } From d70496437171fbb8d262d476e346dff0ba48d0f6 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 06:11:14 -0500 Subject: [PATCH 02/12] =?UTF-8?q?docs(roadmap):=20tick=20X-NAME=20?= =?UTF-8?q?=E2=80=94=20the=20name=20a=20new=20aspect=20may=20not=20have,?= =?UTF-8?q?=20checked=20where=20the=20name=20is=20born?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 784fc1e8..79498e5f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1156,7 +1156,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [x] Store lane: **D-S8** Write-once maintenance I: generational outputs, frame journal, swap, reaper, reader pins — #72 - [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::()` 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-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)* + - [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 `-.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 - [ ] **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) From 54f277c69acc0dfd8d91ff5a60b5108a633a7420 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 07:00:34 -0500 Subject: [PATCH 03/12] feat(weft-server): end the process when a control-plane write panics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 21 +++++- weft-server/src/lib.rs | 1 + weft-server/src/panic_hook.rs | 81 +++++++++++++++++++++ weft-server/src/settings.rs | 5 +- weft-server/src/startup/hooks.rs | 19 ++++- weft-server/src/startup/mod.rs | 5 +- weft-server/tests/control_plane_panic.rs | 92 ++++++++++++++++++++++++ weftdb/src/types/exec.rs | 8 ++- 8 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 weft-server/src/panic_hook.rs create mode 100644 weft-server/tests/control_plane_panic.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d681b99..f719ef20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`, @@ -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 diff --git a/weft-server/src/lib.rs b/weft-server/src/lib.rs index 421ec5b8..1e6d6558 100644 --- a/weft-server/src/lib.rs +++ b/weft-server/src/lib.rs @@ -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; diff --git a/weft-server/src/panic_hook.rs b/weft-server/src/panic_hook.rs new file mode 100644 index 00000000..3cb63ee0 --- /dev/null +++ b/weft-server/src/panic_hook.rs @@ -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::() { + 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, + } +} diff --git a/weft-server/src/settings.rs b/weft-server/src/settings.rs index b3f9001c..ac68d31f 100644 --- a/weft-server/src/settings.rs +++ b/weft-server/src/settings.rs @@ -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)" }, @@ -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 @@ -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"); } diff --git a/weft-server/src/startup/hooks.rs b/weft-server/src/startup/hooks.rs index d8be6578..fadd14a3 100644 --- a/weft-server/src/startup/hooks.rs +++ b/weft-server/src/startup/hooks.rs @@ -1,10 +1,14 @@ -//! Startup phase 1 — the process-wide hooks: the tracing subscriber, and the OTLP -//! exporter when the operator configured a collector. +//! Startup phase 1 — the process-wide hooks: the tracing subscriber, the OTLP exporter +//! when the operator configured a collector, and the panic hook. //! //! Installed before anything else runs, so every later startup phase and every //! request is observable. Installing a subscriber cannot fail the server: a //! `try_init` is a no-op when one is already installed, and a collector that //! cannot be reached falls back to the `fmt`-only path. +//! +//! The panic hook is installed **after** the subscriber because it reports through it: +//! [`std::panic::set_hook`] replaces the installed hook and hands the replaced one back, so +//! the order decides who prints the panic. See [`init_panic_hook`]. use weft_server::SERVICE; @@ -49,6 +53,17 @@ pub(crate) fn init_tracing() -> Option anyhow::Result<()> { // early would tear down the batch span processor and stop export. It is shut down on // a clean exit to flush any buffered spans. let otel_provider = hooks::init_tracing(); + // Immediately after tracing, so the panic hook reports through the subscriber it chains. + hooks::init_panic_hook(); let addr = config::bind_addr()?; let interpolate = config::interpolate_config()?; diff --git a/weft-server/tests/control_plane_panic.rs b/weft-server/tests/control_plane_panic.rs new file mode 100644 index 00000000..04322cd0 --- /dev/null +++ b/weft-server/tests/control_plane_panic.rs @@ -0,0 +1,92 @@ +//! Robustness track **ROB-2** (release plan C12, C22): a panic while the process is +//! writing the control plane ends the process instead of being answered. +//! +//! [`weftdb::exec::control_plane_write`] marks the region — weftdb's own test +//! `every_op_runs_inside_the_control_plane_write_scope` proves every op of every +//! segment-index transaction runs inside it — and [`weft_server::panic_hook`] is the hook +//! that reads the mark and aborts. This test checks both halves from outside the library: +//! a panic **inside** the region kills the process with a signal a supervisor can see and +//! says why on stderr, and a panic **outside** it still only ends its own task, because +//! ROB-3's catch layer (wave 6) is the only layer that catches panics and unwinding must +//! stay reachable for it. +//! +//! The abort ends whatever process runs it, so each case runs in a child: this test binary +//! re-executed with one test selected, exactly as `poison_exit.rs` does. +//! +//! Run with `cargo test -p weft-server --test control_plane_panic`. + +use weft_server::panic_hook::install; + +/// Set only in the child: which panic to raise. +const CHILD_MODE: &str = "WEFT_TEST_PANIC_CHILD_MODE"; +/// The child panics inside a control-plane write. +const INSIDE: &str = "inside"; +/// The child panics in a task that is writing nothing. +const OUTSIDE: &str = "outside"; +/// What the child panics with, inside the region. +const INJECTED_INSIDE: &str = "injected panic in the middle of a control-plane write"; +/// What the child panics with, outside it. +const INJECTED_OUTSIDE: &str = "injected panic in a task that is not writing the control plane"; +/// What the child prints when a panic outside the region left it running. +const SURVIVED: &str = "panic child: still running after the panic"; + +/// The child's body: install the server's panic hook, then panic in one of the two places. +/// In a normal test run the variable is unset and this does nothing. +#[tokio::test] +async fn control_plane_panic_child() { + let mode = std::env::var(CHILD_MODE).unwrap_or_default(); + if mode != INSIDE && mode != OUTSIDE { + return; + } + install(); + if mode == INSIDE { + // The same future the store's writes run in: yielded through, so the mark is one + // the hook reads from a later poll on a possibly different worker thread. + weftdb::exec::control_plane_write(async { + tokio::task::yield_now().await; + panic!("{INJECTED_INSIDE}"); + }) + .await; + return; + } + let task = tokio::spawn(async { panic!("{INJECTED_OUTSIDE}") }); + let error = task.await.expect_err("the task aborts").into_panic(); + let shown = error.downcast_ref::().cloned().unwrap_or_else(|| format!("{error:?}")); + assert!(shown.contains(INJECTED_OUTSIDE), "the panic that ended the task: {shown}"); + println!("{SURVIVED}"); +} + +/// Run [`control_plane_panic_child`] in a child process panicking in `mode`. +async fn run_child(mode: &str) -> std::process::Output { + let exe = std::env::current_exe().expect("finds the test binary"); + let mut child = tokio::process::Command::new(exe); + child.args(["control_plane_panic_child", "--exact", "--nocapture", "--test-threads=1"]).env(CHILD_MODE, mode); + child.output().await.expect("runs the child") +} + +#[tokio::test] +async fn a_panic_inside_a_control_plane_write_aborts_the_process() { + let out = run_child(INSIDE).await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!out.status.success(), "the child did not die: {out:?}"); + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt as _; + assert_eq!(out.status.signal(), Some(6), "it died by SIGABRT (6), not by a status a supervisor might mistake for a clean exit: {out:?}"); + } + assert!(stderr.contains(INJECTED_INSIDE), "the panic itself is on stderr: {stderr}"); + assert!(stderr.contains("the control plane was being written when this panic happened") && stderr.contains("aborting the process"), "and so is the reason it died: {stderr}"); + assert!(!stdout.contains(SURVIVED), "the child never got past the panic: {stdout}"); +} + +#[tokio::test] +async fn a_panic_outside_a_control_plane_write_only_ends_its_task() { + let out = run_child(OUTSIDE).await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "the process lives, so ROB-3 can still catch this case: {out:?}"); + assert!(stderr.contains(INJECTED_OUTSIDE), "the panic is still reported by the chained hook: {stderr}"); + assert!(!stderr.contains("the control plane was being written when this panic happened"), "the abort path is not taken: {stderr}"); + assert!(stdout.contains(SURVIVED), "the process kept running: {stdout}"); +} diff --git a/weftdb/src/types/exec.rs b/weftdb/src/types/exec.rs index 1a1d3b4c..297a2efa 100644 --- a/weftdb/src/types/exec.rs +++ b/weftdb/src/types/exec.rs @@ -8,8 +8,12 @@ //! `CONTROL_PLANE_WRITE.try_with(|f| f.get())` (or [`in_control_plane_write`]). Every //! segment-index transaction (`IndexTxn`, the store's whole `segment_index.db` write //! path) runs inside it; a hook that finds the flag set write-poisons the process -//! ([`poison_global`](crate::durable::poison_global)) instead of letting it go on writing -//! through that pager. +//! ([`poison_global`](crate::durable::poison_global)) and stops the process from going on +//! writing through that pager — `weft-server`'s hook does exactly that and aborts (ROB-2). +//! The scope is a task-local, not a thread-local, so it follows the task across `await` +//! points and worker threads. It is **not** inherited by `tokio::spawn`, which is why the +//! region wraps whole operations rather than individual statements: work a transaction +//! spawned out of the region would run unmarked, and the tests below check that. use std::{cell::Cell, future::Future}; From df21a2e18ba6fc97e42d6071bb3e1d74980618b2 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 07:14:39 -0500 Subject: [PATCH 04/12] =?UTF-8?q?docs(roadmap):=20tick=20ROB-2=20=E2=80=94?= =?UTF-8?q?=20a=20panic=20in=20the=20middle=20of=20a=20write=20ends=20the?= =?UTF-8?q?=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 79498e5f..d022587a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -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::()` 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 `-.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 From 7e31d8a252306211afb1ba7b14066e6f4956eef5 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 08:12:24 -0500 Subject: [PATCH 05/12] build(deps): stop the libraries choosing their users' allocator and GPU stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PAC-3. turso's `mimalloc` feature installs a `#[global_allocator]` in turso's own crate root, so the workspace's turso line made `weftdb` — a published library — decide the process allocator of every program that links it, including one with an allocator of its own or none at all. splimes' default `gpu` feature did the same one layer over: `gpu` -> `wgpu` -> the Vulkan, Metal and DX12 backends, in the dependency graph of five library crates. Our own binaries lose nothing. `weft-server` and `weft-tui` now name mimalloc 0.1.52 in their own roots, which is the edit the 1.0 plan's L-MAIN lane already schedules (docs/release/1.0-plan.md:126, "Later main.rs edits, one per wave: PAC-3 (W2, `#[global_allocator]`). `weft-server` and `weft-bench` ask splimes for `gpu` directly, and `weftdb`'s interpolation benches get it as a dev-dependency, so every GPU path this repository compiles and exercises is still compiled and exercised. splimes' `serde` stays on, because `weftdb::BatchedMeasurement` serialises a `splimes::Resolution`; turso's other default, `fts`, stays off as it already was, since it is full-text search and pulling it in grows the graph with tantivy, tantivy-columnar and zstd-sys. `weftdb`'s tracing-subscriber moves to dev-dependencies — only test code installs a subscriber. The non-dev dependency graphs, counted as distinct packages before and after: weft-line-protocol and weft-reduce 77 -> 25, weftdb 266 -> 227, weft-orchestration 267 -> 231. Two crate-doc lines in weftdb/src/lib.rs named splimes' GPU-only `calibrate` and are reworded, since the doc job denies broken intra-doc links. --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ Cargo.lock | 3 ++- Cargo.toml | 21 +++++++++++++++++++-- weft-bench/Cargo.toml | 5 ++++- weft-server/Cargo.toml | 8 +++++++- weft-server/src/main.rs | 8 ++++++++ weft-tui/Cargo.toml | 3 +++ weft-tui/src/main.rs | 7 +++++++ weftdb/Cargo.toml | 6 +++++- weftdb/src/lib.rs | 10 +++++++--- 10 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f719ef20..2bdad7c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -273,9 +273,42 @@ only the control plane, not the segment frames. still part of the file name — `job:rate5m` is a name in use — and that cross-platform hazard is closed one layer down instead, where `aspect_name::encode` escapes `:` for the write-once frame names (D-S10). +- **Our executables say what allocator they run on** (1.0 plan PAC-3). `weft-server` and + `weft-tui` each declare `#[global_allocator] static ALLOCATOR: mimalloc::MiMalloc` in their own + `main.rs`, through a direct `mimalloc` dependency rather than one inherited from a library. + Same allocator, same version, same measured behaviour — the decision is just made in the place + that owns the process (see the dependency entry under Changed). ### Changed +- **Library crates stop deciding their users' allocator and GPU stack** (1.0 plan PAC-3). No + library in this workspace enables turso's `mimalloc` feature or splimes' default `gpu` feature + any more, because both are decisions a dependency makes for the whole process. turso 0.8.1 + installs a `#[global_allocator]` in its own crate root when `mimalloc` is on, so every program + linking `weftdb` — an embedder with its own allocator, a program that wants the system one — was + switched to mimalloc without being asked; `gpu` drags `wgpu`, and with it the Vulkan, Metal and + DX12 backends, into the graph of everything that depends on splimes. Counting the distinct + packages in each crate's non-dev dependency graph, before and after: `weft-line-protocol` and + `weft-reduce` 77 → 25, `weftdb` 266 → 227, `weft-orchestration` 267 → 231. Our own GPU users — + `weft-server` and `weft-bench`, which call `prewarm_gpu`, `calibrate` and `gpu_pool_stats` — + enable `gpu` themselves, as do `weftdb`'s interpolation benches through a dev-dependency, so + nothing this repository runs lost its GPU path. A library user who wants either asks for it: an + allocator by declaring one in their own binary, the GPU by enabling `splimes/gpu`. ⚠️ Rust code + that reached the GPU *through* `weftdb` — a `splimes::Backend::Gpu` while depending on `weftdb` + alone — names `splimes` itself now. One default stays on deliberately: splimes' `serde`, because + `weftdb::BatchedMeasurement` derives `Serialize`/`Deserialize` over a `splimes::Resolution` field. + turso's other default, `fts`, stays off, as it already was: it is turso's full-text search, and + turning it on grows the graph with tantivy, tantivy-columnar, zstd-sys and the rest of that tree, + which the first attempt at this dependency line showed in the lock diff. + One number to be aware of: `cargo bench -p weftdb` is criterion, not test code, so it used to + inherit mimalloc through turso and now runs on the system allocator — a `weftdb` bench figure + from before this release is not comparable with one taken after. `weft-bench` never linked + `weftdb` and never had mimalloc, so the Weft-Bench series, including the nightly regression + track, does not move. +- **`tracing-subscriber` is a dev-dependency of `weftdb`, not a dependency.** No code in the + library installs a subscriber outside `#[cfg(test)]`, so every program linking `weftdb` was + building registry, fmt and env-filter code it never called. + - **Statuses split where the §4 table names a different condition** (1.0 plan X-ERR). A batch that will not seal under an aspect's declared schema no longer answers `400` whatever the reason: a length mismatch between the two columns, a zero `rows_per_page`, and a backwards diff --git a/Cargo.lock b/Cargo.lock index 44e5cb6b..5efe79b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4647,7 +4647,6 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7de8cbeace6385bf3fcb346d4aace8d9ba0de5cffe5e5a0ed210beebd671c208" dependencies = [ - "mimalloc", "thiserror 2.0.21", "tracing", "tracing-subscriber", @@ -5222,6 +5221,7 @@ dependencies = [ "axum", "bigdecimal", "chrono", + "mimalloc", "opentelemetry", "opentelemetry-otlp", "opentelemetry_sdk", @@ -5252,6 +5252,7 @@ dependencies = [ "crossterm", "dirs", "futures", + "mimalloc", "num-traits", "ratatui", "splimes", diff --git a/Cargo.toml b/Cargo.toml index 29dced62..65853f77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,18 @@ rand = "0.10" rayon = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" -turso = { version = "0.8", default-features = false, features = ["mimalloc"] } +# The allocator our own executables run on, named by each of them rather than inherited: see +# `weft-server/src/main.rs` and `weft-tui/src/main.rs`. PAC-3. +mimalloc = "0.1" +# `mimalloc` is deliberately not enabled here: turso's `mimalloc` feature installs a +# process-wide `#[global_allocator]` from its own crate root, so asking for it through a +# library replaces the allocator of every program that merely links that library — including one +# that sets its own, or wants the system default. Nothing in this workspace enables it any more; +# an executable that wants mimalloc depends on it and declares it in its own root, and a library +# user who wants it does the same. turso's other default, `fts`, was already off in this workspace +# and stays off — it is the full-text-search feature, and turning it on here would pull tantivy, +# tantivy-columnar, zstd-sys and the rest of that tree into every build. PAC-3. +turso = { version = "0.8", default-features = false } tempfile = "3" thiserror = "2" tokio = { version = "1", features = ["full"] } @@ -60,7 +71,13 @@ crossterm = "0.29" sysinfo = "0.39" weftdb = { path = "./weftdb", version = "0.1.0" } # Developed in its own repository (github.com/basic-automation/splimes), on its own roadmap. -splimes = "1" +# `default-features = false` because splimes' defaults turn on its `gpu` backend, which pulls +# `wgpu` — Vulkan, Metal, DX12 and a shader compiler — into every crate that links a library +# in this workspace, including the published `weftdb`. The GPU path is the executable's +# decision: `weft-server` and `weft-bench` enable `splimes/gpu`, and `weft-orchestration` +# exposes it as its own opt-in `gpu-codecs` feature. `serde` stays on, because +# `weftdb::BatchedMeasurement`'s derived serde needs splimes' own implementations. PAC-3. +splimes = { version = "1", default-features = false, features = ["serde"] } weft-line-protocol = { path = "./weft-line-protocol", version = "0.1.0" } weft-physical-type = { path = "./weft-physical-type", version = "0.1.0" } weft-reduce = { path = "./weft-reduce", version = "0.1.0" } diff --git a/weft-bench/Cargo.toml b/weft-bench/Cargo.toml index 09835685..fa01ce8d 100644 --- a/weft-bench/Cargo.toml +++ b/weft-bench/Cargo.toml @@ -33,7 +33,10 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } rand = { workspace = true } rand_chacha = { workspace = true } -splimes = { workspace = true, features = ["tokio"] } +# The benchmark's engine metadata and its gpu-memory workload call splimes' GPU entry points +# (`prewarm_gpu`, `calibrate`, `gpu_pool_stats`), so it opts into the backend that the library +# crates no longer carry. PAC-3. +splimes = { workspace = true, features = ["tokio", "gpu"] } sysinfo = { workspace = true } tokio = { workspace = true, features = ["full"] } diff --git a/weft-server/Cargo.toml b/weft-server/Cargo.toml index 41ab159b..4f01dcc3 100644 --- a/weft-server/Cargo.toml +++ b/weft-server/Cargo.toml @@ -42,6 +42,10 @@ axum = { workspace = true } bigdecimal = { workspace = true } chrono = { workspace = true, features = ["serde"] } weftdb = { workspace = true } # control-plane segment store backing the storage query endpoints +# The process allocator this binary chooses, stated here rather than inherited from a library +# dependency (PAC-3); see `main.rs`. The same mimalloc that every benchmark of this server, and +# every release build of it, has measured. +mimalloc = { workspace = true } weft-arrow = { workspace = true } # reconstructed-series -> Arrow IPC / Parquet bytes for the interpolate endpoint (arrow types stay inside weft-arrow) weft-arrow-store = { workspace = true } # stored range -> Arrow IPC bytes (keeps the arrow-* tree off the lean core) weft-line-protocol = { workspace = true } @@ -49,7 +53,9 @@ weft-physical-type = { workspace = true } # AspectSchema / PhysicalType / TimeU weft-reduce = { workspace = true } # canonical downsample reduction (Aggregation / reduce / bucket_start) serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -splimes = { workspace = true, features = ["tokio"] } +# The GPU backend is this binary's decision, not the libraries': `weft-server/src/gpu.rs` +# calibrates splimes' backends at startup. PAC-3. +splimes = { workspace = true, features = ["tokio", "gpu"] } tokio = { workspace = true, features = ["full"] } tracing = { workspace = true } # per-request engine spans on the compute path (Phase 3 instrumentation) tracing-subscriber = { workspace = true } # RUST_LOG-driven span/event subscriber (env-filter from the workspace dep) diff --git a/weft-server/src/main.rs b/weft-server/src/main.rs index 34a76808..d0dbbfb9 100644 --- a/weft-server/src/main.rs +++ b/weft-server/src/main.rs @@ -8,6 +8,14 @@ mod startup; +// The process allocator is this binary's choice, not a side effect of what a library it links +// happens to depend on (PAC-3). turso used to carry it: its `mimalloc` feature installs a +// `#[global_allocator]` in turso's own crate root, so any program linking `weftdb` got mimalloc +// whether it wanted it or not. `weftdb` no longer asks for it, and an executable that wants it — +// every one of ours, on every machine we benchmark and ship — says so here. +#[global_allocator] +static ALLOCATOR: mimalloc::MiMalloc = mimalloc::MiMalloc; + #[tokio::main] async fn main() -> anyhow::Result<()> { startup::run().await diff --git a/weft-tui/Cargo.toml b/weft-tui/Cargo.toml index 98e7329c..9216d283 100644 --- a/weft-tui/Cargo.toml +++ b/weft-tui/Cargo.toml @@ -18,6 +18,9 @@ publish = false ratatui = { workspace = true } crossterm = { workspace = true } weftdb = { workspace = true } +# The process allocator this binary chooses, stated here rather than inherited from a library +# dependency (PAC-3); see `main.rs`. +mimalloc = { workspace = true } splimes = { workspace = true } tokio = { workspace = true } anyhow = { workspace = true } diff --git a/weft-tui/src/main.rs b/weft-tui/src/main.rs index 48d2141f..7b62a6ad 100644 --- a/weft-tui/src/main.rs +++ b/weft-tui/src/main.rs @@ -1,6 +1,13 @@ #![warn(clippy::pedantic, clippy::nursery, clippy::all)] #![allow(clippy::multiple_crate_versions, clippy::used_underscore_binding, clippy::similar_names, clippy::module_name_repetitions, clippy::module_inception)] +// The process allocator is this binary's choice, not a side effect of what a library it links +// happens to depend on (PAC-3). turso used to carry it: its `mimalloc` feature installs a +// `#[global_allocator]` in turso's own crate root, so any program linking `weftdb` got mimalloc +// whether it wanted it or not. `weftdb` no longer asks for it; this binary says so itself. +#[global_allocator] +static ALLOCATOR: mimalloc::MiMalloc = mimalloc::MiMalloc; + use std::fs::OpenOptions; use tracing_subscriber::{fmt, prelude::*, EnvFilter}; diff --git a/weftdb/Cargo.toml b/weftdb/Cargo.toml index 4ab3d3ba..ca4afb8c 100644 --- a/weftdb/Cargo.toml +++ b/weftdb/Cargo.toml @@ -50,13 +50,17 @@ serde_json = { workspace = true } md5 = { workspace = true } sysinfo = { workspace = true } tracing = { workspace = true } -tracing-subscriber = { workspace = true } [dev-dependencies] csv = { workspace = true } criterion = { workspace = true , features = ["html_reports"] } tempfile = { workspace = true } serial_test = { workspace = true } +# The interpolation benches compare splimes' CPU, rayon and GPU backends, so they need the GPU +# path that the library deliberately does not enable for its users. +splimes = { workspace = true, features = ["gpu"] } +# Only the tests install a subscriber; the library logs through the `tracing` facades. +tracing-subscriber = { workspace = true } [target.'cfg(unix)'.dev-dependencies] # Only to keep the fault tests' deliberately aborted child from dumping core. diff --git a/weftdb/src/lib.rs b/weftdb/src/lib.rs index 26bf0f7f..910589e0 100644 --- a/weftdb/src/lib.rs +++ b/weftdb/src/lib.rs @@ -207,9 +207,11 @@ //! - **Advanced Interpolation**: Linear, cubic, quadratic, and polynomial spline methods //! (splimes 1.0), run on tokio's blocking pool so they never occupy an async worker //! - **Interpolation Backends**: splimes picks one CPU thread or the rayon pool by output -//! grid size, and the GPU (`wgpu`) only above the thresholds `splimes::calibrate()` +//! grid size, and its GPU backend only above thresholds its own `calibrate` function //! measures; a program calls it once at startup (`weft-server` does), and without it -//! interpolation stays on the CPU +//! interpolation stays on the CPU. That backend is splimes' opt-in `gpu` feature, which +//! this crate does **not** enable: enabling it here would compile Vulkan, Metal and DX12 +//! support into every program that links `weftdb`, whether or not it has a GPU. //! - **MVCC Concurrency**: Concurrent writes with `BEGIN CONCURRENT` transactions //! - **Pattern Recognition**: Extract and match recurring patterns in time-series data //! - **Event Detection**: Detect significant occurrences and predict future events @@ -255,7 +257,9 @@ //! - **Rust Version**: 1.95 or later, on stable //! - **Hardware**: `x86_64`, ARM64 architectures //! - **GPU Support**: Optional, through splimes' `wgpu` backend (Vulkan, Metal, DX12), -//! used once `splimes::calibrate()` has run +//! used once splimes' `calibrate` function has run. It is opt-in at the dependency level: +//! `weftdb` does not enable splimes' `gpu` feature, so a build that wants the GPU asks +//! for it explicitly. #![recursion_limit = "1024"] #![warn(clippy::pedantic, clippy::nursery, clippy::all)] From dfa4134fc124116cc5c1c03075040871a5ac08ef Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 08:23:44 -0500 Subject: [PATCH 06/12] =?UTF-8?q?docs(roadmap):=20tick=20PAC-3=20=E2=80=94?= =?UTF-8?q?=20libraries=20stop=20choosing=20their=20users'=20allocator=20a?= =?UTF-8?q?nd=20GPU=20stack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #93. Two decisions were being made for every program that links these crates: turso's mimalloc feature installs a #[global_allocator] in turso's own crate root, so asking for it on the published library weftdb chose the allocator of every embedder's process; and splimes' default features pull wgpu, which sat in the normal graph of four libraries that never call a GPU API. The allocator now belongs to the executables, which is where the plan puts this slice (docs/release/1.0-plan.md:126). GPU stays opt-in at the dependency level. Distinct packages in the normal graph: weft-line-protocol and weft-reduce 77 -> 25, weft-orchestration 267 -> 231, weftdb 266 -> 227. Recorded in the note: the two keeps (splimes serde for BatchedMeasurement, turso fts left off as it already was), and that cargo bench -p weftdb is no longer allocator-comparable across this commit. --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index d022587a..a7e0b21d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1158,7 +1158,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [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::()` 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 `-.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`.* - [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 + - [x] **PAC-3** Library crate hygiene: no global allocator or wgpu features imposed on library users — *#93: two decisions were being made for every program that links these crates. `turso` 0.8.1 puts a `#[global_allocator]` in its own crate root behind its `mimalloc` feature, so the workspace's `features = ["mimalloc"]` on a **published library** (`weftdb/Cargo.toml`) chose the allocator of every embedder's process; and `splimes`' default features pull `wgpu`, so `wgpu` sat in the normal graph of `weftdb`, `weft-line-protocol`, `weft-reduce` and `weft-orchestration` — 52 packages of Vulkan, Metal and DX12 code for two crates that never call a GPU API. The allocator moved to the place that owns the process: `weft-server/src/main.rs` and `weft-tui/src/main.rs` each declare `static ALLOCATOR: mimalloc::MiMalloc` through a direct `mimalloc` dependency, so the same allocator on the same version still runs everything we benchmark and ship, and `weftdb` now carries no allocator feature at all — the plan's own lane row names this slice at `main.rs` (`docs/release/1.0-plan.md:126`). GPU is opt-in the same way: the workspace asks splimes for `default-features = false, features = ["serde"]`, `weft-server` and `weft-bench` add `gpu` themselves, and `weftdb`'s interpolation benches take it as a dev-dependency. Distinct packages in the normal graph: `weft-line-protocol` and `weft-reduce` 77 → 25, `weft-orchestration` 267 → 231, `weftdb` 266 → 227; `cargo tree -p weftdb -i wgpu -e normal` and `-i mimalloc` now return nothing. Two keeps are deliberate: splimes' `serde` stays on because `weftdb::BatchedMeasurement` holds a `splimes::Resolution` and derives `Serialize`/`Deserialize`, and turso's other default, `fts`, stays off exactly as it already was — an attempt to re-add it pulled `tantivy`, `zstd`, `datasketches` and friends, +435 lines of `Cargo.lock`, for full-text search nothing in this workspace uses. `tracing-subscriber` became a dev-dependency of `weftdb` (only test code installs a subscriber). One consequence for our own numbers: `cargo bench -p weftdb` used to inherit mimalloc and now runs on the system allocator, so those nine criterion benches are not comparable across this commit; the nightly regression track (EVI-5) uses `weft-bench`, which never linked `weftdb` and never had mimalloc, so no baseline moves there.* - [ ] 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 - [ ] **PAC-2** Portable Linux binaries on a glibc 2.28 baseline From eaeff33a84e005edbe55ffc3c375ca3020d0e550 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 11:33:09 -0500 Subject: [PATCH 07/12] Name frame versions once, and read a frame from what the file says The plan's X-FRAME row (docs/release/1.0-plan.md:142) asks for a frame registry, the v7 envelope, the read_frame*/describe_frame family and the FrameFault classes, absorbing FRE-4, TAG-2 and ROB-8's fault_class naming. weft_physical_type::frame_registry is now the only place that says which frame version means what: 5 single-block, 6 paged, 7 envelope, and the rule in the module doc that a number is never reused or renumbered once it has shipped. The envelope is a header and a trailer around an existing frame, WEFTSEG | u16 7 | u16 body_version | extension table | frame | crc32, and it stores that frame verbatim, its own magic, version, body and CRC intact. That is a deliberate constraint: segment_store.rs promises an unconfigured deployment's bytes are byte-for-byte what previous versions wrote, and the byte-pinning tests stay green because a v5 or v6 body is untouched. A legacy reader pointed at an envelope still says UnsupportedVersion { found: 7 } rather than misparsing it. An extension carries a critical flag. Unknown and critical means refusal: the body was written on the assumption that the extension would be applied, so decoding without it serves data under a contract nobody honoured. Unknown and optional means ignored. KNOWN_EXTENSIONS is empty on purpose - the envelope ships with no extensions, and the numbers are a permanent namespace. SegmentStore chose its decoder from the index row's format_version, so anything that was not 6 - including the unwrap_or(0) in segment_index.rs - sent a paged frame to the single-block reader. It now asks the file: the six decode sites go through weftdb/src/types/frame_read.rs, which probes the header, calls the matching read_frame_*, and returns failures as weftdb::FrameFaultError in the anyhow chain under the unchanged "decoding [paged ]segment : ..." sentence. weft-server maps the class by type: a frame ahead of this build is 503 incompatible_store, any other frame fault is 500 frame_undecodable, the code X-ERR gave a wire sentence but no producer. No writer opts into 7 yet: seals still write 5 and 6. This slice is the mechanism and the classes; the opt-in and the C5 quarantine behaviour belong to D-S12 and the ingest lanes. Tests: weft-physical-type 306 lib (14 new: envelope round-trip, a legacy reader refusing an envelope, critical refusal, optional ignore, header/count disagreement caught by CRC, describe_frame on a future version, fault classification, the read family over a wrapped frame); weftdb frame_read 6 new; weft-server --test frame_fault 2 new, corrupting a real sealed frame and asserting 500 frame_undecodable with details.fault "integrity" and 503 incompatible_store with "newer_format" after re-stamping the trailer CRC. --- CHANGELOG.md | 25 + weft-physical-type/src/frame_registry.rs | 239 +++++++++ weft-physical-type/src/lib.rs | 6 +- weft-physical-type/src/weftseg.rs | 603 ++++++++++++++++++++++- weft-server/src/error.rs | 26 + weft-server/tests/frame_fault.rs | 115 +++++ weftdb/src/types/frame_read.rs | 266 ++++++++++ weftdb/src/types/mod.rs | 2 + weftdb/src/types/segment_store.rs | 56 +-- 9 files changed, 1304 insertions(+), 34 deletions(-) create mode 100644 weft-physical-type/src/frame_registry.rs create mode 100644 weft-server/tests/frame_fault.rs create mode 100644 weftdb/src/types/frame_read.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bdad7c2..3b76807e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -278,6 +278,31 @@ only the control plane, not the segment frames. `main.rs`, through a direct `mimalloc` dependency rather than one inherited from a library. Same allocator, same version, same measured behaviour — the decision is just made in the place that owns the process (see the dependency entry under Changed). +- **Frame versions are a registry, and a frame says what it is** (1.0 plan X-FRAME; absorbs FRE-4, + TAG-2 and ROB-8's fault-class naming). `weft_physical_type::frame_registry` names the frame + layouts — `5` single-block, `6` paged, `7` envelope — on the rule stated there that a number is + never reused or renumbered once it has shipped. The v7 envelope is a header and a trailer around + a frame stored **verbatim**: `WEFTSEG | u16 7 | u16 body_version | extension table | frame | + crc32`, 16 bytes of overhead (a 12-byte header plus the trailer), so a v5 or v6 body keeps every + byte-for-byte expectation a released server already holds while a reader gains somewhere to put + material in front of a body it is not allowed to change. An extension is `{id, critical, + payload}`: a reader that does not know a **critical** one refuses the frame, because that body + was written on the assumption that the extension would be applied, and an optional one is + ignored. The set this build implements (`KNOWN_EXTENSIONS`) is deliberately empty — the envelope + ships with no extensions, and a number invented before there is a feature behind it can never be + taken back. `describe_frame`, `frame_layout_of`, `wrap_frame_envelope`/`unwrap_frame_envelope` + and the layout-aware readers `read_frame_segment`, `read_frame_paged_segment`, `read_frame_point`, + `read_frame_points`, `read_frame_range` and `read_frame_range_physical` read a frame without the + caller knowing how it is framed, and `frame_fault_of` gives the class an operator acts on (plan + C5): `Integrity`, the bytes are damaged, or `Undecodable { newer_format }`, this binary is behind + the file. `SegmentStore` picks its reader from the frame's own header rather than the + `format_version` it recorded in the index row, where any value that was not `6` used to send a + paged frame into the single-block decoder; the decode failures it returns carry + `weftdb::FrameFaultError` in the `anyhow` chain under the `decoding [paged ]segment : …` + sentence they have always produced. Over HTTP that is C5: a frame this build has no reader for is + `503 incompatible_store`, and any other frame that will not decode is `500 frame_undecodable`, + the code X-ERR gave a wire sentence but no producer. **No writer opts into the envelope yet** — + seals keep writing 5 and 6 — so this is the mechanism, not a format change. ### Changed diff --git a/weft-physical-type/src/frame_registry.rs b/weft-physical-type/src/frame_registry.rs new file mode 100644 index 00000000..dc4b561a --- /dev/null +++ b/weft-physical-type/src/frame_registry.rs @@ -0,0 +1,239 @@ +//! The version registry for `.weftseg` frame layouts, and the fault classes a frame +//! read can fail with. +//! +//! Every `.weftseg` file names its own layout: the seven magic bytes, then a `u16` +//! little-endian format version. That number is the whole of what a reader needs to +//! know how to decode what follows, so it is a shared, permanent name — three tracks +//! of this project allocated frame versions independently and would have shipped +//! different meanings for the same number (1.0 plan X-FRAME). This module is where +//! the names live: [`FRAME_SINGLE_BLOCK`] and [`FRAME_PAGED`] are the two layouts +//! WeftDB has always written, [`FRAME_ENVELOPE`] is the v7 wrapper that lets a frame +//! carry extension blocks in front of a body whose bytes are already frozen. +//! +//! **A number that has shipped is never reused, redefined, or retired**, so a file +//! written by an old server still means the same thing to a new one, and a file +//! written by a new server is refused rather than misread. The list is append-only: +//! the next layout takes 8. +//! +//! # Fault classes +//! +//! A frame read can fail in two ways that the operator should not treat the same way +//! (1.0 plan resolution C5). [`FrameFault::Integrity`] means the bytes disagree with +//! their own checksum, length, or magic: the file is damaged, and the quarantine +//! machinery is allowed to purge it. [`FrameFault::Undecodable`] means the file is +//! internally consistent but this build cannot make sense of it — a version it has +//! never heard of, an extension it cannot honour, a codec it was compiled without. +//! Those files are never damaged, and deleting one destroys data that a different +//! build could read: they go suspect, not corrupt, and `newer_format: true` is the +//! sub-case where a server should also say on `/ready` that the store on disk is a +//! format it cannot open. +//! +//! The quarantine machinery itself is D-S12's (verified by TAG-9 and ROB-22); what +//! lives here is the vocabulary every layer agrees on in the meantime. + +/// The single-block frame layout WeftDB has always written: one block per column, the +/// whole aspect's rows in one file. +pub const FRAME_SINGLE_BLOCK: u16 = 5; +/// The paged frame layout: fixed-size pages, each independently decodable, so a range +/// read touches only the pages it needs. +pub const FRAME_PAGED: u16 = 6; +/// The envelope: this version number's own layout, a header naming the body layout +/// plus an extension table, then a complete frame of that layout. +/// +/// The envelope exists because a body layout, once written by a released server, is +/// frozen: adding a field inside it would silently shift every byte after it. A frame +/// that needs something new puts it in front of the frozen body instead, where an +/// older reader can still refuse the file cleanly rather than decode garbage from a +/// moved offset. +pub const FRAME_ENVELOPE: u16 = 7; + +/// The highest frame version this build can read. Anything above it came from a newer +/// server, which is the difference between "this file is broken" and "this file is +/// ahead of me" — see [`FrameFault::Undecodable`]. +pub const FRAME_VERSION_MAX_KNOWN: u16 = FRAME_ENVELOPE; + +/// Which layout a frame body uses. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FrameLayout { + /// [`FRAME_SINGLE_BLOCK`]. + SingleBlock, + /// [`FRAME_PAGED`]. + Paged, +} + +/// The layout a body version names, or `None` for a version this build has no reader +/// for. +/// +/// The version in a frame's own header is not necessarily a layout this build knows; +/// the caller decides whether that is an error worth reporting. +#[must_use] +pub const fn layout_of(version: u16) -> Option { + match version { + FRAME_SINGLE_BLOCK => Some(FrameLayout::SingleBlock), + FRAME_PAGED => Some(FrameLayout::Paged), + _ => None, + } +} + +/// An entry in a v7 envelope's extension table. +/// +/// `critical` is the whole of the compatibility contract: an extension the reader has +/// never heard of is skipped when it is informational, and refuses the frame when it +/// is critical, because "critical" is the writer's way of saying *the body below is +/// only correct if you applied me*. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FrameExtension { + /// The extension's number. Extension numbers are a permanent namespace, on the + /// same rule as frame versions: never reuse one, and never renumber one that has + /// shipped. + pub id: u32, + /// Whether a reader that does not know this extension must refuse the frame. + pub critical: bool, + /// The extension's own bytes. The envelope does not interpret them; the extension + /// defines its own payload format, versioned by its own number. + pub payload: Vec, +} + +/// An extension number this build implements, and what it means. +/// +/// The description is not decoration: it is what the reader prints when a frame asks +/// for something the binary does not have, and an operator cannot decide between +/// "upgrade the server" and "this file is broken" without it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct FrameExtensionSpec { + /// The extension's number. + pub id: u32, + /// What the extension does, in the sentence an operator reads. + pub name: &'static str, +} + +/// The extension numbers this build implements. +/// +/// Append-only, on exactly the rule stated for frame versions at the top of this +/// module: an extension that has shipped keeps its number, and a new one takes the +/// next free number. It is empty because the envelope ships with no extensions — the +/// point of X-FRAME is the *mechanism* that lets a later slice put a block in front of +/// a frozen body without moving a byte inside it, and a number invented before there +/// is a writer for it is a number wasted. +pub const KNOWN_EXTENSIONS: &[FrameExtensionSpec] = &[]; + +/// Whether this build knows what to do with extension `id`. +/// +/// A frame that names an extension absent from [`KNOWN_EXTENSIONS`] is a frame written +/// by a newer server, not a damaged one: the answer feeds +/// [`FrameFault::Undecodable`], and the file is kept. +#[must_use] +pub fn is_known_extension(id: u32) -> bool { + KNOWN_EXTENSIONS.iter().any(|spec| spec.id == id) +} + +/// What a frame is, without decoding it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FrameInfo { + /// The version in the frame's own header: one of [`FRAME_SINGLE_BLOCK`], + /// [`FRAME_PAGED`], [`FRAME_ENVELOPE`], or a number this build has no reader for. + pub version: u16, + /// The version of the body bytes themselves — the same as [`Self::version`] for a + /// plain v5/v6 frame, and the wrapped layout's number for an envelope. + pub body_version: u16, + /// The body's layout, or `None` if this build has no reader for it. + pub layout: Option, + /// The envelope's extension table; empty for a plain frame. + pub extensions: Vec, + /// Set when the frame carries a **critical** extension this build does not + /// implement: the body is not safe to serve, because it was written on the + /// assumption that a reader would apply what is missing. + pub unknown_critical: bool, +} + +impl FrameInfo { + /// Whether the frame is safe to decode with this build: the body layout is known + /// and no critical extension is being ignored. + #[must_use] + pub const fn decodable(&self) -> bool { + self.layout.is_some() && !self.unknown_critical + } +} + +/// Why a frame could not be read, in the two classes the plan's C5 resolution defines. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FrameFault { + /// The bytes disagree with their own magic, checksum or length: the file is + /// damaged. Quarantine and, under the store's own policy, TTL purge are allowed. + Integrity, + /// The file is internally consistent but this build cannot read it. Its contents + /// are not damaged, so it is never purged automatically. + Undecodable { + /// The cause is a frame version this build predates, so the store holds data + /// written by a newer server. `/ready` reports `incompatible_store` for this + /// case and only this case. + newer_format: bool, + }, +} + +impl FrameFault { + /// The fault class name, for logs and for the operator-facing quarantine reason: + /// `integrity`, `newer_format`, or `undecodable`. + #[must_use] + pub const fn fault_class(&self) -> &'static str { + match self { + Self::Integrity => "integrity", + Self::Undecodable { newer_format: true } => "newer_format", + Self::Undecodable { newer_format: false } => "undecodable", + } + } + + /// Whether the fault means the file itself is damaged. + #[must_use] + pub const fn is_integrity(&self) -> bool { + matches!(self, Self::Integrity) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_registry_numbers_are_the_ones_the_formats_use() { + // Pinned against the constants the writers use, so a renumber here and a + // renumber there cannot drift apart. + assert_eq!(FRAME_SINGLE_BLOCK, crate::segment::SEGMENT_FORMAT_VERSION); + assert_eq!(FRAME_PAGED, crate::page::PAGED_SEGMENT_FORMAT_VERSION); + assert_eq!(FRAME_ENVELOPE, 7); + assert_eq!(FRAME_VERSION_MAX_KNOWN, FRAME_ENVELOPE); + } + + #[test] + fn layout_of_names_only_what_this_build_can_read() { + assert_eq!(layout_of(FRAME_SINGLE_BLOCK), Some(FrameLayout::SingleBlock)); + assert_eq!(layout_of(FRAME_PAGED), Some(FrameLayout::Paged)); + for unknown in [0, 1, 2, 3, 4, FRAME_ENVELOPE, 8, u16::MAX] { + assert_eq!(layout_of(unknown), None, "{unknown} has no reader in this build"); + } + } + + #[test] + fn no_extension_number_is_claimed_yet() { + // The envelope mechanism is what X-FRAME ships; the extensions come later, one + // per slice that needs a block in front of a frozen body. This assertion is the + // place where a new extension becomes visible instead of incidental. + assert!(KNOWN_EXTENSIONS.is_empty()); + for id in 0..=16 { + assert!(!is_known_extension(id), "extension {id} has no implementation in this build"); + } + // Numbers are a permanent namespace: a table may only ever gain distinct ids. + for (i, spec) in KNOWN_EXTENSIONS.iter().enumerate() { + assert!(!KNOWN_EXTENSIONS[i + 1..].iter().any(|later| later.id == spec.id), "extension number {} claimed twice", spec.id); + } + } + + #[test] + fn fault_classes_are_the_three_c5_names() { + assert_eq!(FrameFault::Integrity.fault_class(), "integrity"); + assert!(FrameFault::Integrity.is_integrity()); + assert_eq!(FrameFault::Undecodable { newer_format: true }.fault_class(), "newer_format"); + assert_eq!(FrameFault::Undecodable { newer_format: false }.fault_class(), "undecodable"); + assert!(!FrameFault::Undecodable { newer_format: false }.is_integrity()); + } +} diff --git a/weft-physical-type/src/lib.rs b/weft-physical-type/src/lib.rs index 08a7f0ac..7dd62854 100644 --- a/weft-physical-type/src/lib.rs +++ b/weft-physical-type/src/lib.rs @@ -106,6 +106,9 @@ pub mod catalog; pub mod column; #[cfg(feature = "experimental-codecs")] pub mod floatcodec; +/// The `.weftseg` frame version registry, and the two classes of frame-fault a read +/// can fail with (1.0 plan X-FRAME, resolution C5). +pub mod frame_registry; pub mod nulls; pub mod page; pub mod schema; @@ -122,6 +125,7 @@ pub use catalog::{SegmentDescriptor, SegmentIndex}; pub use column::{encode_column, recommend_encoding, CascadeInner, ColumnEncodeError, ColumnEncoding, DeltaCascadePlan}; #[cfg(feature = "experimental-codecs")] pub use floatcodec::{best_f64_bytes, best_f64_codec, chimp128_f64_bytes, chimp128_f64_decode, chimp128_f64_encode, chimp_f64_bytes, chimp_f64_decode, chimp_f64_encode, elf_f64_bytes, elf_f64_decode, elf_f64_encode, xor_f64_bytes, xor_f64_decode, xor_f64_encode}; +pub use frame_registry::{is_known_extension, layout_of, FrameExtension, FrameExtensionSpec, FrameFault, FrameInfo, FrameLayout, FRAME_ENVELOPE, FRAME_PAGED, FRAME_SINGLE_BLOCK, FRAME_VERSION_MAX_KNOWN, KNOWN_EXTENSIONS}; pub use nulls::{NullMask, NullMaskError}; pub use page::{Page, PagedSegment, DEFAULT_ROWS_PER_PAGE, PAGED_SEGMENT_FORMAT_VERSION}; pub use schema::{AspectSchema, SealError}; @@ -132,7 +136,7 @@ pub use tags::{SeriesKey, SeriesSelector, TagError, TagSet, MAX_SERIES_KEY_BYTES pub use timestamp::{bitpack_bytes, bitpack_decode, bitpack_encode, bitpack_width, decode_delta, decode_delta_of_delta, encode_delta, encode_delta_of_delta, first_order_violation, rle_decode, rle_encode, rle_varint_bytes, uvarint_len, zigzag_varint_bytes, zigzag_varint_len, DeltaColumn, DeltaOfDeltaColumn, TimeUnit}; #[cfg(feature = "experimental-codecs")] pub use timestamp::{fire_estimated_bytes, fire_reconstruct, fire_residuals}; -pub use weftseg::{crc32, frame_value_codec, read_paged_segment, read_segment, write_paged_segment, write_segment, write_segment_with, ByteReader, ByteWriter, FrameOptions, WeftSegError}; +pub use weftseg::{crc32, describe_frame, frame_fault_of, frame_layout_of, frame_value_codec, frame_version, read_frame_paged_segment, read_frame_point, read_frame_points, read_frame_range, read_frame_range_physical, read_frame_segment, read_paged_segment, read_segment, unwrap_frame_envelope, wrap_frame_envelope, write_paged_segment, write_segment, write_segment_with, ByteReader, ByteWriter, FrameOptions, WeftSegError}; /// A schema-declared physical encoding for an aspect's numeric values. /// diff --git a/weft-physical-type/src/weftseg.rs b/weft-physical-type/src/weftseg.rs index ea61e05f..c782579a 100644 --- a/weft-physical-type/src/weftseg.rs +++ b/weft-physical-type/src/weftseg.rs @@ -31,7 +31,13 @@ //! by construction, because both count the same varint widths. /// Why a `.weftseg` byte stream could not be read. +/// +/// The variants are open: a new frame layout or a new decode restriction adds one +/// without breaking a caller's `match`. Pair a failure with +/// [`frame_fault_of`] to get the class an operator +/// acts on rather than the byte that tripped. #[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] pub enum WeftSegError { /// The reader ran out of bytes before a value was fully decoded. UnexpectedEof { @@ -89,6 +95,25 @@ pub enum WeftSegError { /// The cargo feature of `weft-physical-type` that compiles the codec in. feature: &'static str, }, + /// A v7 envelope carries a **critical** extension this build does not implement, so + /// the body it wraps cannot be trusted: "critical" is the writer's way of saying the + /// body below is only correct once a reader has applied this. The file is not + /// damaged — a build that implements extension `id` reads it — so it is quarantined + /// as suspect and never purged (1.0 plan resolution C5). + UnknownCriticalExtension { + /// The extension number the frame requires. + id: u32, + }, + /// A v7 envelope declares a body layout that the frame inside it does not name. Both + /// fields are covered by the same checksum, so no writer produces this: the file was + /// edited, or a writer has a bug. It is counted as damage rather than as an unknown + /// format, because there is no build that could read it. + EnvelopeBodyMismatch { + /// The layout the envelope's `body_version` field declares. + declared: u16, + /// The version the wrapped frame names in its own header. + found: u16, + }, } impl std::fmt::Display for WeftSegError { @@ -105,6 +130,8 @@ impl std::fmt::Display for WeftSegError { Self::TrailingBytes { remaining } => write!(f, "{remaining} trailing bytes after segment frame"), Self::InvalidNullMask(source) => write!(f, "invalid quality column: {source}"), Self::CodecNotEnabled { codec, feature } => write!(f, "segment uses the {codec} codec, which this build does not include; rebuild with the `{feature}` cargo feature (weft-physical-type; forwarded by weftdb and weft-server) to read it"), + Self::UnknownCriticalExtension { id } => write!(f, "frame carries critical extension {id}, which this build does not implement; the frame is left intact because a build that implements it can read it"), + Self::EnvelopeBodyMismatch { declared, found } => write!(f, "v7 envelope declares a {declared} body but the frame inside it names version {found}; the envelope's own checksum is correct, so this file was written or edited inconsistently"), } } } @@ -2205,7 +2232,7 @@ fn read_dod_codec(r: &mut ByteReader, count: usize) -> Result<(Vec, Option< // --------------------------------------------------------------------------- use crate::{ - nulls::NullMask, page::{Page, PagedSegment, PAGED_SEGMENT_FORMAT_VERSION}, segment::SegmentStats, Segment, SEGMENT_FORMAT_VERSION + frame_registry::{layout_of, FrameExtension, FrameFault, FrameInfo, FrameLayout, FRAME_ENVELOPE, FRAME_VERSION_MAX_KNOWN}, nulls::NullMask, page::{Page, PagedSegment, PAGED_SEGMENT_FORMAT_VERSION}, segment::SegmentStats, Segment, SEGMENT_FORMAT_VERSION }; /// The magic prefix every `.weftseg` frame starts with. @@ -2319,6 +2346,352 @@ fn read_segment_stats(r: &mut ByteReader) -> Result Ok(SegmentStats { row_count, null_count, time_sorted, min_ts, max_ts, min_value, max_value }) } +// --------------------------------------------------------------------------- +// The v7 envelope: a header in front of a body whose bytes are already frozen. +// --------------------------------------------------------------------------- + +/// The layout version a frame names in its own header, without touching anything +/// after it. The cheapest thing a server can do with a file it is about to open. +/// +/// # Errors +/// +/// [`WeftSegError::UnexpectedEof`] for a file too short to carry a header, +/// [`WeftSegError::BadMagic`] for bytes that are not a `.weftseg` frame at all, and +/// never a version error — this reports the version rather than judging it. +pub fn frame_version(bytes: &[u8]) -> Result { + if bytes.len() < MAGIC.len() + 2 { + return Err(WeftSegError::UnexpectedEof { needed: MAGIC.len() + 2, remaining: bytes.len() }); + } + if !is_known_magic(&bytes[..MAGIC.len()]) { + return Err(WeftSegError::BadMagic); + } + Ok(u16::from_le_bytes([bytes[MAGIC.len()], bytes[MAGIC.len() + 1]])) +} + +/// Which reader a frame needs, read from its header alone. +/// +/// This is a routing decision, not a verdict: nothing past the version fields is +/// trusted or verified here, and the reader this chooses checks every byte that +/// follows. That is what makes it safe to route on the number in the file rather +/// than the number copied into an index row when the frame was written — a frame +/// whose header was damaged lands in whichever reader the damaged byte suggests and +/// fails there on its own checksum, which is the honest answer either way. +/// +/// An envelope's answer is the layout of the frame it wraps, so a caller can pick a +/// layout-specific entry point without knowing whether the envelope is in use. +#[must_use] +pub fn frame_layout_of(bytes: &[u8]) -> Result { + let version = frame_version(bytes)?; + if let Some(layout) = layout_of(version) { + return Ok(layout); + } + if version != FRAME_ENVELOPE { + return Err(WeftSegError::UnsupportedVersion { found: version }); + } + // The envelope's own version is followed by the version of the frame inside it. + if bytes.len() < MAGIC.len() + 4 { + return Err(WeftSegError::UnexpectedEof { needed: MAGIC.len() + 4, remaining: bytes.len() }); + } + let body_version = ByteReader::new(&bytes[MAGIC.len() + 2..]).read_u16_le()?; + layout_of(body_version).ok_or(WeftSegError::UnsupportedVersion { found: body_version }) +} + +/// A frame taken apart: header, extension table, and the body bytes a reader of +/// `body_version` can be handed as they stand. +struct FrameParts<'a> { + /// The version in the frame's own header. + version: u16, + /// The version of the body layout. + body_version: u16, + /// The extension table, empty for a plain frame. + extensions: Vec, + /// A critical extension this build does not implement was named as one. + unknown_critical: bool, + /// The complete frame of `body_version`: magic, version, body and that frame's own + /// CRC trailer. For a plain v5/v6 frame this is the whole input, which is what + /// every existing reader in this file expects to be handed. + body: &'a [u8], +} + +/// Split a frame into its header, extension table and body, checking the frame's own +/// trailing CRC on the way. +/// +/// A v5 or v6 frame comes back with `body == bytes`: the envelope pass changes nothing +/// about how the two historical layouts are read. A v7 frame is checked end to end — +/// the outer CRC covers the header, the table and the wrapped frame including that +/// frame's own CRC — and its wrapped frame is returned as the body, verbatim. +fn frame_parts(bytes: &[u8]) -> Result, WeftSegError> { + let version = frame_version(bytes)?; + if bytes.len() < MAGIC.len() + 6 { + // Header plus a CRC-32 trailer is the smallest frame that could ever be valid. + return Err(WeftSegError::UnexpectedEof { needed: MAGIC.len() + 6, remaining: bytes.len() }); + } + let (framed, trailer) = bytes.split_at(bytes.len() - 4); + let stored = u32::from_le_bytes([trailer[0], trailer[1], trailer[2], trailer[3]]); + let computed = crc32(framed); + if stored != computed { + return Err(WeftSegError::ChecksumMismatch { stored, computed }); + } + if version != FRAME_ENVELOPE { + return Ok(FrameParts { version, body_version: version, extensions: Vec::new(), unknown_critical: false, body: bytes }); + } + // Skip the magic and the envelope's own version; what follows is body_version, + // then the extension table, then the wrapped frame. + let rest = &framed[MAGIC.len() + 2..]; + let mut r = ByteReader::new(rest); + let body_version = r.read_u16_le()?; + let count = r.read_uvarint()?; + // The count is not trusted as a size hint: entries are parsed one at a time and + // each needs bytes that must actually be present, so a count invented by damage + // stops at the first entry that is not there instead of reserving anything. + let mut extensions = Vec::new(); + let mut unknown_critical = false; + for _ in 0..count { + let id = u32::try_from(r.read_uvarint()?).map_err(|_| WeftSegError::VarintTooLong)?; + let critical = match r.read_u8()? { + 0 => false, + 1 => true, + value => return Err(WeftSegError::InvalidTag { kind: "extension critical flag", value }), + }; + let payload = r.read_bytes()?.to_vec(); + if critical && !crate::frame_registry::is_known_extension(id) { + unknown_critical = true; + } + extensions.push(FrameExtension { id, critical, payload }); + } + let body = &rest[r.pos..]; + // The wrapped frame carries its own header and CRC, and it has to name the layout the + // envelope says it is. Both fields sit under the same checksum, so a writer cannot + // produce a pair that disagrees: this is a file that was edited, or a writer with a + // bug. The check lives here rather than in each caller so that every reader sees the + // same answer, and the error names what was found rather than what was claimed — a + // version error here would be a lie about a version this build has never heard of. + let found = frame_version(body)?; + if found != body_version { + return Err(WeftSegError::EnvelopeBodyMismatch { declared: body_version, found }); + } + Ok(FrameParts { version, body_version, extensions, unknown_critical, body }) +} + +/// Wrap a complete frame in a v7 envelope carrying `extensions`. +/// +/// The frame inside is stored **verbatim** — magic, version and its own CRC included — +/// and the envelope adds its header plus a CRC over everything. That costs 16 bytes — +/// a 12-byte header (magic, envelope version, body version, entry count) plus a 4-byte +/// trailer — +/// and buys the envelope's whole point: not one byte inside a v5 or v6 frame moves, so +/// the byte-for-byte expectations that released servers and on-disk stores already +/// hold keep holding, while a reader that understands envelopes can put new material +/// in front of a body it is not allowed to change. +/// +/// # Errors +/// +/// [`WeftSegError::BadMagic`] or [`WeftSegError::UnsupportedVersion`] if `inner` is not +/// a frame of a layout this build can read — an envelope must wrap something real. +pub fn wrap_frame_envelope(inner: &[u8], extensions: &[FrameExtension]) -> Result, WeftSegError> { + let body_version = frame_version(inner)?; + if layout_of(body_version).is_none() { + return Err(WeftSegError::UnsupportedVersion { found: body_version }); + } + let extra = extensions.iter().map(|e| e.payload.len() + 6).sum::(); + let mut w = ByteWriter::with_capacity(inner.len() + extra + 32); + w.put_raw(MAGIC); + w.put_u16_le(FRAME_ENVELOPE); + w.put_u16_le(body_version); + w.put_uvarint(extensions.len() as u64); + for e in extensions { + w.put_uvarint(u64::from(e.id)); + w.put_u8(u8::from(e.critical)); + w.put_bytes(&e.payload); + } + w.put_raw(inner); + let checksum = crc32(w.as_slice()); + w.put_u32_le(checksum); + Ok(w.into_vec()) +} + +/// The complete frame inside a v7 envelope, with the envelope's CRC verified and its +/// extension table checked. +/// +/// A plain v5/v6 frame is returned unchanged, so a caller that reads frames through +/// this function does not have to know which layout it was given. +/// +/// # Errors +/// +/// [`WeftSegError::UnknownCriticalExtension`] if the frame carries a critical extension +/// this build does not implement — the body was written on the assumption that a reader +/// would apply it, so decoding the body without it would serve data the writer never +/// meant. Also the CRC, magic, length and varint errors of the envelope check itself. +pub fn unwrap_frame_envelope(bytes: &[u8]) -> Result<&[u8], WeftSegError> { + let parts = frame_parts(bytes)?; + if parts.unknown_critical { + // Named separately from the generic refusal so the message can say which number + // the operator's server has to grow. + if let Some(e) = parts.extensions.iter().find(|e| e.critical && !crate::frame_registry::is_known_extension(e.id)) { + return Err(WeftSegError::UnknownCriticalExtension { id: e.id }); + } + } + // `frame_parts` has already checked that a wrapped frame names the layout its + // envelope declares, and the legacy readers below re-check its CRC over exactly + // these bytes. + Ok(parts.body) +} + +/// Describe a frame without decoding it: which layout it is, which extensions it +/// carries, and whether this build can serve it. +/// +/// This is what a server answers for a frame it cannot read, and what an operator +/// script reads to decide whether a file is damaged or simply ahead of the software. +/// The answer is already classified: a `FrameFault` says which of the two, and +/// `FrameInfo::decodable()` says whether the body may be served at all. +/// +/// # Errors +/// +/// The [`FrameFault`] class of whatever went wrong — see [`frame_fault_of`]. +pub fn describe_frame(bytes: &[u8]) -> Result { + match frame_parts(bytes) { + Ok(parts) => Ok(FrameInfo { version: parts.version, body_version: parts.body_version, layout: layout_of(parts.body_version), extensions: parts.extensions, unknown_critical: parts.unknown_critical }), + Err(error) => Err(frame_fault_of(&error)), + } +} + +/// The operator-facing class of a frame read failure, per the plan's C5 resolution. +/// +/// **Integrity** is damage: the bytes disagree with their own magic, checksum or +/// length, and the quarantine machinery may purge the file. Everything else is +/// **undecodable**: the file is self-consistent, and the only reason it failed is that +/// this build has no reader for it — a version it predates, a codec feature it was +/// compiled without, an extension it does not implement, or a tag it does not +/// recognise. Those files hold data that some other build can read, so they are never +/// purged automatically. +/// +/// `newer_format` is set when the cause is a frame version above +/// [`FRAME_VERSION_MAX_KNOWN`], which is the case where the store as a whole should +/// report `incompatible_store` rather than blame one file. +#[must_use] +pub const fn frame_fault_of(error: &WeftSegError) -> FrameFault { + match error { + WeftSegError::UnsupportedVersion { found } => FrameFault::Undecodable { newer_format: *found > FRAME_VERSION_MAX_KNOWN }, + WeftSegError::CodecNotEnabled { .. } | WeftSegError::UnknownCriticalExtension { .. } => FrameFault::Undecodable { newer_format: false }, + // BadMagic, ChecksumMismatch, UnexpectedEof, VarintTooLong, TrailingBytes, + // InvalidUtf8, InvalidDecimal, InvalidNullMask, EnvelopeBodyMismatch: the file + // does not agree with itself. InvalidTag is counted with them because a tag this + // build does not know arrives with a new frame or body version, not inside a + // version it already understands. + _ => FrameFault::Integrity, + } +} + +/// The body layout of a frame this build can read, envelope or not, together with the +/// body bytes that reader should be handed. +/// +/// # Errors +/// +/// [`WeftSegError::UnsupportedVersion`] for a body version with no reader (including a +/// frame from a newer server), and [`WeftSegError::UnknownCriticalExtension`] for a +/// critical extension this build does not implement. +fn read_frame_body(bytes: &[u8]) -> Result<(&[u8], FrameLayout), WeftSegError> { + let parts = frame_parts(bytes)?; + if parts.unknown_critical { + if let Some(e) = parts.extensions.iter().find(|e| e.critical && !crate::frame_registry::is_known_extension(e.id)) { + return Err(WeftSegError::UnknownCriticalExtension { id: e.id }); + } + } + let body = if parts.version == FRAME_ENVELOPE { + // `frame_parts` already proved the wrapped frame names the layout declared in the + // envelope header, so the layout below is the layout of these bytes. + parts.body + } else { + bytes + }; + match layout_of(parts.body_version) { + Some(layout) => Ok((body, layout)), + None => Err(WeftSegError::UnsupportedVersion { found: parts.body_version }), + } +} + +/// Read a single-block frame, whether or not it is wrapped in a v7 envelope. +/// +/// The envelope-aware form of [`read_segment`], for a caller that does not know how the +/// file was framed. +/// +/// # Errors +/// +/// The [`WeftSegError`]s of [`unwrap_frame_envelope`] and [`read_segment`]. Use +/// [`frame_fault_of`] to turn one into the class the operator acts on. +pub fn read_frame_segment(bytes: &[u8]) -> Result { + read_segment(unwrap_frame_envelope(bytes)?) +} + +/// Read a paged frame, whether or not it is wrapped in a v7 envelope. +/// +/// The envelope-aware form of [`read_paged_segment`]. +/// +/// # Errors +/// +/// The [`WeftSegError`]s of [`unwrap_frame_envelope`] and [`read_paged_segment`]. +pub fn read_frame_paged_segment(bytes: &[u8]) -> Result { + read_paged_segment(unwrap_frame_envelope(bytes)?) +} + +/// Read one point from a frame of either layout, wrapped or not. +/// +/// The envelope-aware form of [`read_segment_point`] and [`read_paged_segment_point`], +/// dispatching on the layout the file itself names rather than on what a caller +/// believes it wrote. +/// +/// # Errors +/// +/// [`WeftSegError::UnsupportedVersion`] for a body layout this build has no point +/// reader for, otherwise the errors of the two underlying readers. +pub fn read_frame_point(bytes: &[u8], t: i64) -> Result, WeftSegError> { + let (body, layout) = read_frame_body(bytes)?; + match layout { + FrameLayout::SingleBlock => read_segment_point(body, t), + FrameLayout::Paged => read_paged_segment_point(body, t), + } +} + +/// Read several points from a frame of either layout, wrapped or not. +/// +/// # Errors +/// +/// As [`read_frame_point`]. +pub fn read_frame_points(bytes: &[u8], ts: &[i64]) -> Result>, WeftSegError> { + let (body, layout) = read_frame_body(bytes)?; + match layout { + FrameLayout::SingleBlock => read_segment_points(body, ts), + FrameLayout::Paged => read_paged_segment_points(body, ts), + } +} + +/// Read a timestamp range from a frame of either layout, wrapped or not. +/// +/// # Errors +/// +/// As [`read_frame_point`]. +pub fn read_frame_range(bytes: &[u8], start: i64, end: i64) -> Result<(Vec, Vec>), WeftSegError> { + let (body, layout) = read_frame_body(bytes)?; + match layout { + FrameLayout::SingleBlock => read_segment_range(body, start, end), + FrameLayout::Paged => read_paged_segment_range(body, start, end), + } +} + +/// Read a timestamp range as physical values from a frame of either layout, wrapped or +/// not. +/// +/// # Errors +/// +/// As [`read_frame_point`]. +pub fn read_frame_range_physical(bytes: &[u8], start: i64, end: i64) -> Result<(Vec, Vec>), WeftSegError> { + let (body, layout) = read_frame_body(bytes)?; + match layout { + FrameLayout::SingleBlock => read_segment_range_physical(body, start, end), + FrameLayout::Paged => read_paged_segment_range_physical(body, start, end), + } +} + /// Encode a [`Segment`] into a complete, self-describing, checksummed `.weftseg` /// byte frame. /// @@ -2696,6 +3069,7 @@ pub fn read_paged_segment(bytes: &[u8]) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::frame_registry::{FrameExtension, FrameFault, FrameLayout, FRAME_ENVELOPE, FRAME_PAGED, FRAME_SINGLE_BLOCK, FRAME_VERSION_MAX_KNOWN}; #[test] fn crc32_slicing_by_16_matches_the_byte_at_a_time_crc() { @@ -4174,6 +4548,233 @@ mod tests { assert_eq!(read_segment(&bytes), Err(WeftSegError::UnsupportedVersion { found: SEGMENT_FORMAT_VERSION + 1 })); } + // --------------------------------------------------------------------- + // The v7 envelope, the frame registry, and the two fault classes C5 names + // --------------------------------------------------------------------- + + /// Re-stamp a frame's trailing CRC-32 over everything before it, so a body edit can + /// be made with the trailer telling the truth about the damaged bytes. + fn restamp(frame: &[u8]) -> Vec { + let mut bytes = frame.to_vec(); + let body_len = bytes.len() - 4; + let new_crc = crc32(&bytes[..body_len]).to_le_bytes(); + bytes[body_len..].copy_from_slice(&new_crc); + bytes + } + + /// Rewrite the u16 version at `at` and re-stamp the trailer: a frame that is + /// internally consistent apart from the number that was changed, which is how a + /// version this build has never seen arrives from a future release. + fn reversion(frame: &[u8], at: usize, version: u16) -> Vec { + let mut bytes = frame.to_vec(); + bytes[at..at + 2].copy_from_slice(&version.to_le_bytes()); + restamp(&bytes) + } + + /// A single-block frame with enough rows that point and range reads do real work. + fn single_block_frame() -> Vec { + write_segment(&single_block_segment()) + } + + fn single_block_segment() -> Segment { + let ts: Vec = (0..400).map(|i| 1_600_000_000 + i * 60).collect(); + let vs: Vec = (0..400).map(|i| BigDecimal::from(i * 3)).collect(); + Segment::build(&ts, &vs, TimeUnit::Seconds, &BigDecimal::from(0)).expect("builds") + } + + /// A paged frame spanning several pages. + fn paged_frame() -> Vec { + write_paged_segment(&paged_segment()) + } + + fn paged_segment() -> PagedSegment { + let ts: Vec = (0..400).map(|i| 1_600_000_000 + i * 60).collect(); + let vs: Vec = (0..400).map(|i| BigDecimal::from(i * 3)).collect(); + let seg = PagedSegment::build(&ts, &vs, TimeUnit::Seconds, &BigDecimal::from(0), 64).expect("builds"); + assert!(seg.page_count() > 1, "the point of these tests is more than one page"); + seg + } + + #[test] + fn an_envelope_stores_the_frame_it_wraps_without_moving_a_byte() { + let inner = single_block_frame(); + let wrapped = wrap_frame_envelope(&inner, &[]).expect("wraps"); + // magic(7) + envelope version(2) + body version(2) + entry count(1) = 12 bytes of + // header, then the inner frame entire, then the envelope's own CRC. + let header = MAGIC.len() + 2 + 2 + 1; + assert_eq!(&wrapped[header..header + inner.len()], &inner[..], "the wrapped frame keeps its own magic, version and CRC untouched"); + assert_eq!(wrapped.len(), inner.len() + 16, "an envelope with an empty extension table costs 16 bytes"); + assert_eq!(frame_version(&wrapped).expect("version"), FRAME_ENVELOPE); + assert_eq!(unwrap_frame_envelope(&wrapped).expect("unwraps"), &inner[..]); + } + + #[test] + fn the_frame_readers_answer_the_same_for_a_wrapped_frame() { + for plain in [single_block_frame(), paged_frame()] { + let wrapped = wrap_frame_envelope(&plain, &[]).expect("wraps"); + // A plain frame has to go through the same entry points too, or the family would + // be a second format a caller has to detect rather than a superset of the first. + assert_eq!(read_frame_point(&wrapped, 1_600_000_060).expect("wrapped point"), read_frame_point(&plain, 1_600_000_060).expect("plain point")); + assert_eq!(read_frame_points(&wrapped, &[1_600_000_060, 1_600_001_200]).expect("wrapped points"), read_frame_points(&plain, &[1_600_000_060, 1_600_001_200]).expect("plain points")); + assert_eq!(read_frame_range(&wrapped, 1_600_000_000, 1_600_003_000).expect("wrapped range"), read_frame_range(&plain, 1_600_000_000, 1_600_003_000).expect("plain range")); + assert_eq!(read_frame_range_physical(&wrapped, 1_600_000_000, 1_600_003_000).expect("wrapped physical"), read_frame_range_physical(&plain, 1_600_000_000, 1_600_003_000).expect("plain physical")); + } + // The layout-specific entry points keep working too, wrapped or not. + let single = single_block_frame(); + let single_wrapped = wrap_frame_envelope(&single, &[]).expect("wraps"); + assert_eq!(read_frame_segment(&single_wrapped).expect("segment"), read_segment(&single).expect("segment")); + let paged = paged_frame(); + let paged_wrapped = wrap_frame_envelope(&paged, &[]).expect("wraps"); + assert_eq!(read_frame_paged_segment(&paged_wrapped).expect("paged"), read_paged_segment(&paged).expect("paged")); + } + + #[test] + fn a_critical_extension_this_build_lacks_refuses_the_frame() { + let inner = single_block_frame(); + let wrapped = wrap_frame_envelope(&inner, &[FrameExtension { id: 7, critical: true, payload: vec![1, 2, 3] }]).expect("wraps"); + // Decoding the body without applying the extension would serve data the writer + // never meant, so every reader in the family refuses instead. + assert_eq!(read_frame_point(&wrapped, 1_600_000_060), Err(WeftSegError::UnknownCriticalExtension { id: 7 })); + assert_eq!(read_frame_range(&wrapped, 1_600_000_000, 1_600_003_000), Err(WeftSegError::UnknownCriticalExtension { id: 7 })); + assert_eq!(unwrap_frame_envelope(&wrapped), Err(WeftSegError::UnknownCriticalExtension { id: 7 })); + // Describing it is still allowed: the fault class is what an operator acts on, and + // the file is undecodable-but-intact, not damaged. + let info = describe_frame(&wrapped).expect("describes"); + assert_eq!(info.version, FRAME_ENVELOPE); + assert_eq!(info.body_version, FRAME_SINGLE_BLOCK); + assert!(info.unknown_critical); + assert!(!info.decodable()); + assert_eq!(info.extensions.len(), 1); + assert_eq!(info.extensions[0].id, 7); + assert!(info.extensions[0].critical); + let error = read_frame_point(&wrapped, 1_600_000_060).expect_err("refuses"); + assert_eq!(frame_fault_of(&error), FrameFault::Undecodable { newer_format: false }); + // A build that has never heard of the envelope refuses it too, rather than reading + // the extension table as if it were a segment header. + assert_eq!(read_segment(&wrapped), Err(WeftSegError::UnsupportedVersion { found: FRAME_ENVELOPE })); + } + + #[test] + fn an_informational_extension_is_carried_and_ignored() { + let inner = single_block_frame(); + let wrapped = wrap_frame_envelope(&inner, &[FrameExtension { id: 3, critical: false, payload: b"written-by/version".to_vec() }]).expect("wraps"); + assert_eq!(read_frame_range(&wrapped, 1_600_000_000, 1_600_003_000).expect("range"), read_frame_range(&inner, 1_600_000_000, 1_600_003_000).expect("range")); + let info = describe_frame(&wrapped).expect("describes"); + assert!(!info.unknown_critical); + assert!(info.decodable()); + assert_eq!(info.extensions[0].payload, b"written-by/version"); + } + + #[test] + fn a_frame_from_a_newer_build_is_the_newer_format_fault() { + let renamed = reversion(&single_block_frame(), MAGIC.len(), FRAME_VERSION_MAX_KNOWN + 1); + // Describing is not decoding: the shape is legible, there is just no reader. + let info = describe_frame(&renamed).expect("describes"); + assert_eq!(info.version, FRAME_VERSION_MAX_KNOWN + 1); + assert_eq!(info.layout, None, "no reader in this build is named for it"); + assert!(!info.decodable()); + let error = read_frame_point(&renamed, 1_600_000_060).expect_err("refuses"); + assert_eq!(error, WeftSegError::UnsupportedVersion { found: FRAME_VERSION_MAX_KNOWN + 1 }); + // The whole store is suspect in this case, not one file: a build that wrote it is + // ahead of this one, which is what /ready reports as incompatible_store. + assert_eq!(frame_fault_of(&error), FrameFault::Undecodable { newer_format: true }); + // Writing one is refused too: an envelope has to wrap something this build can read. + assert_eq!(wrap_frame_envelope(&renamed, &[]), Err(WeftSegError::UnsupportedVersion { found: FRAME_VERSION_MAX_KNOWN + 1 })); + } + + #[test] + fn an_older_version_is_not_reported_as_a_newer_build() { + // The distinction matters because the two faults get different treatment: a frame + // this old was written by something that could not have known better, so blaming + // the store's version would send an operator hunting for a server that does not + // exist. + let renamed = reversion(&single_block_frame(), MAGIC.len(), FRAME_SINGLE_BLOCK - 1); + let error = read_frame_point(&renamed, 1_600_000_060).expect_err("refuses"); + assert_eq!(error, WeftSegError::UnsupportedVersion { found: FRAME_SINGLE_BLOCK - 1 }); + assert_eq!(frame_fault_of(&error), FrameFault::Undecodable { newer_format: false }); + } + + #[test] + fn an_envelope_that_disagrees_with_its_body_is_damage() { + let wrapped = wrap_frame_envelope(&single_block_frame(), &[]).expect("wraps"); + // The envelope's body_version sits after the magic and its own version. + let contradicted = reversion(&wrapped, MAGIC.len() + 2, FRAME_PAGED); + assert_eq!(read_frame_segment(&contradicted), Err(WeftSegError::EnvelopeBodyMismatch { declared: FRAME_PAGED, found: FRAME_SINGLE_BLOCK })); + assert_eq!(read_frame_point(&contradicted, 1_600_000_060), Err(WeftSegError::EnvelopeBodyMismatch { declared: FRAME_PAGED, found: FRAME_SINGLE_BLOCK })); + // Both numbers sit under the same checksum, so nothing wrote this file honestly: + // it is damage, not a format this build merely predates. + assert_eq!(describe_frame(&contradicted), Err(FrameFault::Integrity)); + } + + #[test] + fn an_inner_frame_keeps_its_own_checksum_under_the_envelope() { + let inner = single_block_frame(); + let wrapped = wrap_frame_envelope(&inner, &[]).expect("wraps"); + let header = MAGIC.len() + 2 + 2 + 1; + // Damage in the body, with the envelope's trailer honestly recomputed over it: the + // outer check passes and the inner frame's own CRC still catches the byte. + let mut damaged = wrapped.clone(); + // A byte in the middle of the wrapped frame's column data: not the magic, not the + // version, nothing the envelope inspects for itself. + damaged[header + inner.len() / 2] ^= 0xFF; + let stamped = restamp(&damaged); + assert_eq!(describe_frame(&stamped).expect("describes").version, FRAME_ENVELOPE, "the envelope itself reads as intact"); + let error = read_frame_segment(&stamped).expect_err("the body does not"); + assert!(matches!(error, WeftSegError::ChecksumMismatch { .. }), "the wrapped frame's own trailer is what catches it: {error:?}"); + assert!(frame_fault_of(&error).is_integrity()); + // The same byte without re-stamping is caught one layer earlier, and still classifies + // the same way, so the two layers cannot tell an operator two different stories. + let error = read_frame_segment(&damaged).expect_err("refuses"); + assert!(matches!(error, WeftSegError::ChecksumMismatch { .. }), "{error:?}"); + assert!(frame_fault_of(&error).is_integrity()); + } + + #[test] + fn describe_frame_names_the_layout_of_every_frame() { + let single = single_block_frame(); + let paged = paged_frame(); + let info = describe_frame(&single).expect("v5"); + assert_eq!((info.version, info.body_version, info.layout), (FRAME_SINGLE_BLOCK, FRAME_SINGLE_BLOCK, Some(FrameLayout::SingleBlock))); + assert!(info.extensions.is_empty() && !info.unknown_critical && info.decodable()); + let info = describe_frame(&paged).expect("v6"); + assert_eq!((info.version, info.body_version, info.layout), (FRAME_PAGED, FRAME_PAGED, Some(FrameLayout::Paged))); + let info = describe_frame(&wrap_frame_envelope(&paged, &[]).expect("wraps")).expect("v7 of v6"); + assert_eq!((info.version, info.body_version, info.layout), (FRAME_ENVELOPE, FRAME_PAGED, Some(FrameLayout::Paged))); + // Damage is described as damage, including a frame too short to hold a trailer. + assert_eq!(describe_frame(&[]), Err(FrameFault::Integrity)); + assert_eq!(describe_frame(&[0; 3]), Err(FrameFault::Integrity)); + } + + #[test] + fn an_envelope_wraps_a_checkpointed_frame() { + // The opt-in layouts are exactly the frames whose bytes the envelope exists to + // protect, so a wrapped checkpointed frame has to read back identically. + let ts: Vec = (0..400).map(|i| 1_600_000_000 + i * 60).collect(); + let vs: Vec = (0..400).map(|i| BigDecimal::from(i * 3)).collect(); + let _ = (&ts, &vs); + let seg = single_block_segment(); + let plain = write_segment_checkpointed(&seg, 64); + let wrapped = wrap_frame_envelope(&plain, &[]).expect("wraps"); + assert_eq!(read_frame_segment(&wrapped).expect("segment"), read_segment(&plain).expect("segment")); + let paged = paged_segment(); + let plain = write_paged_segment_checkpointed(&paged, 64); + let wrapped = wrap_frame_envelope(&plain, &[]).expect("wraps"); + assert_eq!(read_frame_paged_segment(&wrapped).expect("paged"), read_paged_segment(&plain).expect("paged")); + assert_eq!(read_frame_range(&wrapped, 1_600_000_000, 1_600_003_000).expect("range"), read_paged_segment_range(&plain, 1_600_000_000, 1_600_003_000).expect("legacy range")); + } + + #[test] + fn the_envelope_rejects_a_frame_it_was_handed_that_is_not_a_frame() { + // Not a .weftseg at all, correctly checksummed: refused by magic, and classed as + // damage because there is no build that could read it. + let mut junk = b"NOTSEG!".to_vec(); + junk.extend_from_slice(&crc32(&junk).to_le_bytes()); + assert_eq!(unwrap_frame_envelope(&junk), Err(WeftSegError::BadMagic)); + assert_eq!(describe_frame(&junk), Err(FrameFault::Integrity)); + // A truncated envelope: the header does not even fit. + assert!(matches!(unwrap_frame_envelope(&[b'W', b'E', b'F', b'T', b'S', b'E', b'G', 7, 0, 5, 0]), Err(WeftSegError::UnexpectedEof { .. }))); + } + #[test] fn realized_frame_decodes_to_the_original_data() { // End-to-end: raw data -> segment -> bytes -> segment -> raw data. diff --git a/weft-server/src/error.rs b/weft-server/src/error.rs index 4d55cf20..e5ac641c 100644 --- a/weft-server/src/error.rs +++ b/weft-server/src/error.rs @@ -571,6 +571,15 @@ pub(crate) fn classify_typed(err: &anyhow::Error) -> Option { if let Some(e) = err.downcast_ref::() { return Some(aspect_name_refusal(e)); } + // A frame on disk that this build could not decode. The fault class decides the status, + // not the decoder's words: a frame written by a newer build is a store this binary must + // refuse (C5 sends `incompatible_store`, and the operator's next step is the binary, not + // the file), while damage or a frame needing an extension or codec this build lacks is + // `frame_undecodable`. Neither is ever derived from the message, which is kept + // byte-for-byte what these sites have always logged. + if let Some(e) = err.downcast_ref::() { + return Some(from_frame_fault(e)); + } // A store on disk this binary cannot open: exit `65` for a CLI, `503 incompatible_store` // here, and the same code for every reason (newer layout, unreadable `STORE_FORMAT`). if let Some(e) = err.downcast_ref::() { @@ -588,6 +597,23 @@ pub(crate) fn classify_typed(err: &anyhow::Error) -> Option { None } +/// A frame whose bytes this build could not turn into rows. +/// +/// `503 incompatible_store` only for `newer_format`, because that is the one case where the +/// file is fine and the running binary is the thing that is wrong: the same store answers once +/// the server is updated, so it is reported as a store/version condition and `/ready` says so +/// too. Damage stays `500 frame_undecodable` — the request was not the problem, but the store as +/// a whole is not unusable either, and C5's quarantine of the file happens in the storage layer. +/// A frame needing a critical extension or an off-by-default codec also stays `500`: the build is +/// behind on a feature, not on the format, and the message names what to enable. +pub fn from_frame_fault(e: &weftdb::FrameFaultError) -> ApiError { + use weft_physical_type::FrameFault; + + let fault = e.fault(); + let code = if matches!(fault, FrameFault::Undecodable { newer_format: true }) { Code::IncompatibleStore } else { Code::FrameUndecodable }; + ApiError::server(code, e.to_string()).with_details(serde_json::json!({ "fault": fault.fault_class() })) +} + /// A batch that will not seal under the aspect's declared schema. /// /// This is where the old code's `400 bad_request` for every seal failure splits apart, as the diff --git a/weft-server/tests/frame_fault.rs b/weft-server/tests/frame_fault.rs new file mode 100644 index 00000000..977fe26a --- /dev/null +++ b/weft-server/tests/frame_fault.rs @@ -0,0 +1,115 @@ +//! What a client gets when a frame on disk cannot be decoded. +//! +//! X-ERR gave `frame_undecodable` a sentence and X-FRAME gave it a producer, and this file is +//! where the two meet. It cannot be a unit test: `weftdb::FrameFaultError` has no public +//! constructor and `WeftSegError` is `#[non_exhaustive]`, precisely so that a status cannot be +//! invented here — the only way to reach these branches is a real file the store cannot read. +//! +//! The two cases are the two halves of plan `C5`. Damage is the store's problem and not the +//! caller's, so it is `500 frame_undecodable` with the `integrity` class: the request was fine, +//! the file is not, and quarantining it is a storage-layer decision that belongs to the slice +//! which verifies frames on read. A frame written by a newer build is the *binary's* problem, so +//! it is `503 incompatible_store` with the `newer_format` class — the same store answers as soon +//! as the server is updated, which is why the code is the one `/ready` already uses to say "this +//! store needs a newer server". Neither answer is derived from the decoder's wording: the class +//! comes from the fault object, and the sentence stays the fixed one X-ERR chose. + +use std::{ + fs, path::{Path, PathBuf}, sync::Arc +}; + +use axum::{ + body::Body, http::{Request, StatusCode} +}; +use bigdecimal::BigDecimal; +use tower::ServiceExt; +use weft_physical_type::{crc32, AspectSchema, PhysicalType, TimeUnit, FRAME_VERSION_MAX_KNOWN}; +use weft_server::{app_with_state, AppState}; +use weftdb::SegmentStore; + +fn schema() -> AspectSchema { + AspectSchema::new(PhysicalType::F64, BigDecimal::from(0), TimeUnit::Seconds) +} + +/// A store holding one sealed frame of three points, closed at the end of the call. +/// +/// The store is dropped on purpose: a frame read while it is still open can be served from a +/// cache, and the whole point is to make the decoder look at damaged bytes. +async fn seal_one_frame(dir: &Path) { + let store = SegmentStore::open(dir).await.expect("opens the store"); + store.declare("price", &schema()).await.expect("declares the aspect"); + store.seal("price", &schema(), &[10, 20, 30], &[BigDecimal::from(1), BigDecimal::from(2), BigDecimal::from(3)]).await.expect("seals the frame"); + drop(store); +} + +/// The single `.weftseg` the store wrote. +fn only_frame(dir: &Path) -> PathBuf { + let segments = dir.join("segments"); + let mut names = fs::read_dir(&segments).expect("the segment directory exists").map(|entry| entry.expect("a directory entry").path()).filter(|path| path.extension().is_some_and(|ext| ext == "weftseg")).collect::>(); + names.sort(); + assert_eq!(names.len(), 1, "one seal writes one frame, found {names:?}"); + names.remove(0) +} + +/// Re-write `path` with `bytes`, re-stamping nothing: callers that change the body recompute the +/// trailer themselves so that each case says which checksum it expects to pass. +fn write_frame(path: &Path, bytes: &[u8]) { + fs::write(path, bytes).expect("writes the frame back"); +} + +/// `GET` the range that covers the sealed points, and return the status plus the JSON body. +async fn read_range(dir: &Path) -> (StatusCode, serde_json::Value) { + let store = SegmentStore::open(dir).await.expect("re-opens the store"); + let router = app_with_state(AppState::new().with_store(Arc::new(store))); + let response = router.oneshot(Request::builder().method("GET").uri("/api/v1/storage/price/points?start=0&end=100").body(Body::empty()).expect("builds the request")).await.expect("answers"); + let status = response.status(); + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX).await.expect("reads the body"); + (status, serde_json::from_slice(&bytes).expect("a JSON body")) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_frame_that_fails_its_checksum_answers_500_frame_undecodable() { + let dir = tempfile::TempDir::new().expect("tempdir"); + seal_one_frame(dir.path()).await; + let frame = only_frame(dir.path()); + + // Flip a byte in the middle of the body and leave the trailer alone: the checksum does not + // match, which is the plain `integrity` case. + let mut bytes = fs::read(&frame).expect("reads the frame"); + let middle = bytes.len() / 2; + bytes[middle] ^= 0xff; + write_frame(&frame, &bytes); + + let (status, body) = read_range(dir.path()).await; + assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR, "damaged data on disk is a server fault, not a bad request: {body}"); + assert_eq!(body["code"], "frame_undecodable", "{body}"); + assert_eq!(body["details"]["fault"], "integrity", "the fault class is what the caller branches on: {body}"); + assert_eq!(body["error"], "the server could not decode data it has on disk", "the sentence is X-ERR's fixed one, not the decoder's: {body}"); + // The decoder's own words are kept for the log, and the envelope's `error` field is not the + // place they leak: this asserts the sanitisation rule holds on this path too. + assert!(!body["error"].as_str().unwrap_or_default().contains("weftseg"), "the path does not belong in the body: {body}"); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_frame_from_a_newer_build_answers_503_incompatible_store() { + let dir = tempfile::TempDir::new().expect("tempdir"); + seal_one_frame(dir.path()).await; + let frame = only_frame(dir.path()); + + // Claim a version this build has never heard of, and re-stamp the trailer so the checksum + // passes: the only thing wrong with the file is that it is newer than the server. The frame + // header is `MAGIC | u16 version`, and the trailer is `crc32` over everything before it, so + // both have to move together or the test would only be proving the case above. + let mut bytes = fs::read(&frame).expect("reads the frame"); + let newer = FRAME_VERSION_MAX_KNOWN + 1; + bytes[7..9].copy_from_slice(&newer.to_le_bytes()); + let end = bytes.len() - 4; + let stamped = crc32(&bytes[..end]).to_le_bytes(); + bytes[end..].copy_from_slice(&stamped); + write_frame(&frame, &bytes); + + let (status, body) = read_range(dir.path()).await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "a store this binary cannot read is a store/version condition: {body}"); + assert_eq!(body["code"], "incompatible_store", "{body}"); + assert_eq!(body["details"]["fault"], "newer_format", "the class says the binary is what is behind: {body}"); +} diff --git a/weftdb/src/types/frame_read.rs b/weftdb/src/types/frame_read.rs new file mode 100644 index 00000000..29d4bfaf --- /dev/null +++ b/weftdb/src/types/frame_read.rs @@ -0,0 +1,266 @@ +//! Reading a frame by what the **frame** says it is, and keeping its fault class +//! attached to the failure (1.0 plan X-FRAME). +//! +//! Every frame-decoding call site in [`crate::SegmentStore`] used to choose its reader +//! from `descriptor.format_version`, a u16 copied into the index row when the frame was +//! sealed. The frame registry (5 = single-block, 6 = paged, 7 = envelope) makes the file +//! the authority on its own layout, and the index copy is at best a cache of what the +//! writer decided years ago. It is also the least protected number in the path: +//! `types/segment_index.rs` reads it out of a damaged row with `unwrap_or(0)`, which +//! quietly routes a paged frame to the single-block reader and reports a corrupt file as +//! a version problem. +//! +//! So the layout is probed from the frame's own header — magic plus the version fields, +//! nothing else — and the reader that gets chosen verifies every byte after that. A +//! header that has been damaged still fails, and fails on the frame's own checksum, +//! which is the honest answer whichever way the routing guess went. +//! +//! What travels out of here is deliberately two things, not one: +//! +//! - the **operator-visible sentence**, byte-for-byte what these sites have always +//! produced (`decoding paged segment : `). Corrupt-frame triage +//! is done by reading logs, and a slice that changes how a fault is *classified* has no +//! business also changing what an operator greps for. When the header cannot even say +//! which layout it is, the sentence uses the unmarked word `segment`, which is what the +//! not-paged case has always said. +//! - a **typed fault** underneath it ([`FrameFaultError`]), carrying the fault class the +//! plan's C5 policy is written against, so `weft-server` can answer with the status the +//! condition deserves rather than deriving one from the sentence — which is the rule +//! X-ERR established: read the type under the `anyhow` context, not the context's words. + +use std::fmt; + +use anyhow::Result; +use bigdecimal::BigDecimal; +use weft_physical_type::{frame_fault_of, frame_layout_of, read_frame_paged_segment, read_frame_point, read_frame_points, read_frame_range, read_frame_range_physical, read_frame_segment, FrameFault, FrameLayout, PagedSegment, PhysicalValue, Segment, WeftSegError}; + +/// A frame that this build could not decode, tagged with why in the vocabulary the +/// storage policy is written against. +/// +/// The class, not the decoder's phrasing, is what the storage layer's behaviour and the +/// HTTP status follow: +/// +/// - [`FrameFault::Integrity`] — damage: a checksum that does not match, a length that +/// does not add up, an envelope that contradicts the frame inside it. The file is +/// corrupt and stays corrupt in every build this one could be updated to. +/// - [`FrameFault::Undecodable`] — this build is the problem, not the file. Either it is +/// older than the frame (`newer_format`, which is a reason to refuse the store rather +/// than to quarantine a file), or the frame needs something this build does not +/// implement, such as a critical extension or a codec behind an off-by-default feature. +/// +/// It sits under an `anyhow::Context` whose sentence is the historical one, so nothing +/// that reads the message changes while the type becomes available to anyone downcasting. +#[derive(Debug)] +pub struct FrameFaultError { + /// The C5 fault class this failure belongs to. + fault: FrameFault, + /// The decoder's own complaint, kept so the log line and `source()` stay informative. + source: WeftSegError, +} + +impl FrameFaultError { + /// The class a caller acts on. + #[must_use] + pub fn fault(&self) -> FrameFault { + self.fault + } + + /// The decoder's own complaint. + #[must_use] + pub fn source(&self) -> &WeftSegError { + &self.source + } +} + +impl fmt::Display for FrameFaultError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + // The class leads, because this is the error a caller reaches for when the + // surrounding context has already been read past. + write!(formatter, "{} frame could not be decoded: {}", self.fault.fault_class(), self.source) + } +} + +impl std::error::Error for FrameFaultError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.source) + } +} + +/// A whole frame, decoded, in whichever layout the frame itself declared. +#[derive(Debug)] +pub(crate) enum FullFrame { + /// Layout 5. + SingleBlock(Segment), + /// Layout 6. + Paged(PagedSegment), +} + +impl FullFrame { + /// Rows per page, for the index columns that only mean something to a paged frame. + pub(crate) fn rows_per_page(&self) -> Option { + match self { + Self::SingleBlock(_) => None, + Self::Paged(segment) => Some(segment.rows_per_page), + } + } + + /// Every row in the frame, nulls kept. + pub(crate) fn decode_nullable(self) -> (Vec, Vec>) { + match self { + Self::SingleBlock(segment) => segment.decode_nullable(), + Self::Paged(segment) => segment.decode_nullable(), + } + } +} + +/// Turns a decoder failure into the chain the store has always returned, with the typed +/// fault added underneath it. +fn decode_error(error: WeftSegError, path: &str, paged: bool) -> anyhow::Error { + // Kept before `error` moves: the historical sentence is the point of this function. + let shown = error.to_string(); + let fault = frame_fault_of(&error); + let word = if paged { "paged segment" } else { "segment" }; + anyhow::Error::new(FrameFaultError { fault, source: error }).context(format!("decoding {word} {path}: {shown}")) +} + +/// Whether the frame calls itself paged, for the sentence in its error path. +fn frame_is_paged(bytes: &[u8]) -> bool { + matches!(frame_layout_of(bytes), Ok(FrameLayout::Paged)) +} + +/// Decode a frame and return the physical (encoded) values in a time range. +pub(crate) fn read_range_physical(path: &str, bytes: &[u8], start: i64, end: i64) -> Result<(Vec, Vec>)> { + let paged = frame_is_paged(bytes); + read_frame_range_physical(bytes, start, end).map_err(|error| decode_error(error, path, paged)) +} + +/// Decode a frame and return the values in a time range. +pub(crate) fn read_range(path: &str, bytes: &[u8], start: i64, end: i64) -> Result<(Vec, Vec>), anyhow::Error> { + let paged = frame_is_paged(bytes); + read_frame_range(bytes, start, end).map_err(|error| decode_error(error, path, paged)) +} + +/// Decode a frame and look up one timestamp. +pub(crate) fn read_point(path: &str, bytes: &[u8], t: i64) -> Result, anyhow::Error> { + let paged = frame_is_paged(bytes); + read_frame_point(bytes, t).map_err(|error| decode_error(error, path, paged)) +} + +/// Decode a frame and look up several timestamps. +pub(crate) fn read_points(path: &str, bytes: &[u8], timestamps: &[i64]) -> Result>, anyhow::Error> { + let paged = frame_is_paged(bytes); + read_frame_points(bytes, timestamps).map_err(|error| decode_error(error, path, paged)) +} + +/// Decode a whole frame, keeping its layout so the caller can report rows per page. +pub(crate) fn read_full(path: &str, bytes: &[u8]) -> Result { + match frame_layout_of(bytes) { + Ok(FrameLayout::Paged) => read_frame_paged_segment(bytes).map(FullFrame::Paged).map_err(|error| decode_error(error, path, true)), + Ok(FrameLayout::SingleBlock) => read_frame_segment(bytes).map(FullFrame::SingleBlock).map_err(|error| decode_error(error, path, false)), + // The header could not name a layout this build knows. There is no paged/non-paged + // answer to make, and the unmarked word is the historical default. + Err(error) => Err(decode_error(error, path, false)), + } +} + +#[cfg(test)] +mod tests { + use bigdecimal::BigDecimal; + use weft_physical_type::{crc32, wrap_frame_envelope, write_paged_segment, FrameExtension, FrameFault, PagedSegment, TimeUnit, FRAME_VERSION_MAX_KNOWN}; + + use super::*; + + /// A ten-row paged frame in the layout most stores actually use. + fn paged_frame() -> PagedSegment { + let timestamps: Vec = (0..10).collect(); + let values: Vec = (0..10).map(BigDecimal::from).collect(); + PagedSegment::build(×tamps, &values, TimeUnit::Nanos, &BigDecimal::from(0), 4).expect("ten rows in pages of four build") + } + + /// The bytes of that frame, unwrapped. + fn paged_bytes() -> Vec { + write_paged_segment(&paged_frame()) + } + + /// Rewrite a frame's own version field and re-stamp its trailer, so a reader reaches + /// the version check instead of tripping the checksum on the way to it. + fn stamp_version(bytes: &mut [u8], version: u16) { + // `MAGIC` is 7 bytes and the version is the u16 straight after it; the trailer is + // the last 4 bytes and covers everything before it. + bytes[7..9].copy_from_slice(&version.to_le_bytes()); + let body = bytes.len() - 4; + let checksum = crc32(&bytes[..body]); + bytes[body..].copy_from_slice(&checksum.to_le_bytes()); + } + + #[test] + fn a_frame_is_read_from_what_the_file_says() { + let body = paged_bytes(); + let wrapped = wrap_frame_envelope(&body, &[]).expect("an envelope around a real frame"); + let frame = read_full("price-1.weftseg", &wrapped).expect("the envelope's own header says which reader to use"); + assert_eq!(frame.rows_per_page(), Some(4), "the wrapped frame is still the paged one"); + let (timestamps, values) = frame.decode_nullable(); + assert_eq!(timestamps.len(), 10); + assert!(values.iter().all(Option::is_some), "no row was lost to the envelope"); + } + + #[test] + fn damage_keeps_the_operator_sentence_and_adds_a_fault_class() { + let mut bytes = paged_bytes(); + // One byte in the middle: away from the magic, away from the trailer, so the + // checksum is what complains. + let middle = bytes.len() / 2; + bytes[middle] ^= 0xff; + let error = read_range("/var/lib/weft/segments/price-1.weftseg", &bytes, i64::MIN, i64::MAX).expect_err("a damaged frame does not decode"); + let fault = error.downcast_ref::().expect("the fault class travels under the context"); + assert!(matches!(fault.fault(), FrameFault::Integrity), "damage is damage: {fault:?}"); + // The sentence an operator greps for is the one these sites have always printed. + assert!(error.to_string().starts_with("decoding paged segment /var/lib/weft/segments/price-1.weftseg: "), "{error}"); + } + + #[test] + fn a_frame_written_by_a_newer_build_is_the_binary_being_behind() { + let mut bytes = paged_bytes(); + stamp_version(&mut bytes, FRAME_VERSION_MAX_KNOWN + 1); + let error = read_range("price-1.weftseg", &bytes, i64::MIN, i64::MAX).expect_err("this build has no reader for that version"); + let fault = error.downcast_ref::().expect("the fault class travels under the context"); + assert!(matches!(fault.fault(), FrameFault::Undecodable { newer_format: true }), "a newer frame is not a corrupt file: {fault:?}"); + } + + #[test] + fn an_envelope_wrapping_a_body_this_build_cannot_read_is_still_newer_format() { + // The case the envelope exists for: a later server wraps a body this build has no + // reader for. `read_full` must not then double-check the CRC of a frame it has + // already refused, which would report the same file as damaged. + let mut envelope = wrap_frame_envelope(&paged_bytes(), &[]).expect("an envelope around a real frame"); + let outer = envelope.len() - 4; + let inner = envelope[1..outer].windows(7).position(|w| w == b"WEFTSEG").expect("an envelope wraps a frame") + 1; + let bumped = FRAME_VERSION_MAX_KNOWN + 1; + envelope[7..9].copy_from_slice(&bumped.to_le_bytes()); + envelope[inner + 7..inner + 9].copy_from_slice(&bumped.to_le_bytes()); + let inner_crc = crc32(&envelope[inner..outer]); + envelope[outer - 4..outer].copy_from_slice(&inner_crc.to_le_bytes()); + let outer_crc = crc32(&envelope[..outer]); + envelope[outer..].copy_from_slice(&outer_crc.to_le_bytes()); + let error = read_full("price-9.weftseg", &envelope).expect_err("the body layout has no reader here"); + let fault = error.downcast_ref::().expect("the fault class travels under the context"); + assert!(matches!(fault.fault(), FrameFault::Undecodable { newer_format: true }), "{fault:?}"); + } + + #[test] + fn a_critical_extension_this_build_lacks_is_a_refusal_not_damage() { + // The frame is intact and this build is simply not allowed to serve it: decoding + // the body without the extension would answer with data the writer never meant. + let wrapped = wrap_frame_envelope(&paged_bytes(), &[FrameExtension { id: 9001, critical: true, payload: vec![7] }]).expect("the envelope does not require a number this build knows"); + let error = read_full("price-1.weftseg", &wrapped).expect_err("a critical extension this build cannot apply stops the read"); + let fault = error.downcast_ref::().expect("the fault class travels under the context"); + assert!(matches!(fault.fault(), FrameFault::Undecodable { newer_format: false }), "{fault:?}"); + } + + #[test] + fn a_non_critical_extension_is_ignored_and_the_frame_reads() { + let wrapped = wrap_frame_envelope(&paged_bytes(), &[FrameExtension { id: 9002, critical: false, payload: vec![7, 7] }]).expect("an extension this build does not know may still be written"); + let frame = read_full("price-1.weftseg", &wrapped).expect("an optional extension does not stop a read"); + assert_eq!(frame.decode_nullable().0.len(), 10); + } +} diff --git a/weftdb/src/types/mod.rs b/weftdb/src/types/mod.rs index 8dd44fa2..05d46115 100644 --- a/weftdb/src/types/mod.rs +++ b/weftdb/src/types/mod.rs @@ -16,6 +16,7 @@ pub use dataset::{Dataset, DatasetId}; pub use dictionary::{Dictionary, DictionaryConstraints, DictionaryId, DictionaryMetadata, Steps, Variability, VariablilityType}; pub use dictionary_name::InvalidDictionaryName; pub use event::{Event, EventID, EventName, Events, Manifestation, ManifestationId}; +pub use frame_read::FrameFaultError; pub use input_measurement::InputMeasurement; pub use measurement::{Measurement, MeasurementId}; pub use measurement_vector::MeasurementVector; @@ -55,6 +56,7 @@ pub mod error; pub mod event; pub mod exec; pub(crate) mod frame_name; +pub(crate) mod frame_read; pub(crate) mod index_txn; pub mod input_measurement; pub mod interpolate_range; diff --git a/weftdb/src/types/segment_store.rs b/weftdb/src/types/segment_store.rs index 8bb99383..bb0016e6 100644 --- a/weftdb/src/types/segment_store.rs +++ b/weftdb/src/types/segment_store.rs @@ -84,14 +84,14 @@ use anyhow::{bail, Context, Result}; use bigdecimal::BigDecimal; use chrono::{DateTime, Utc}; use splimes::{Point, Resolution}; -use weft_physical_type::{merge_newer_wins, split_index, AspectSchema, FrameOptions, PagedSegment, PhysicalValue, Segment, SegmentDescriptor, SplitDecision, SplitPolicy, TimeUnit, PAGED_SEGMENT_FORMAT_VERSION}; +use weft_physical_type::{merge_newer_wins, split_index, AspectSchema, FrameOptions, PagedSegment, PhysicalValue, Segment, SegmentDescriptor, SplitDecision, SplitPolicy, TimeUnit}; use weft_reduce::{Aggregation, Bucket, PartialReduction}; use crate::{ aspect_name, types::{ aspect_locks::{AspectLocks, AspectState, CommitGuard, MaintGuard}, durable::{ create_dir_all_durable, fault::{self, FaultPoint}, poison_global, write_new_durable, LockHolder, RealFs, RootLock, RootLockError, StoreFs, SyncPolicy, WritePoints - }, frame_name, index_txn::{IndexOp, IndexRow, IndexTxn, IndexTxnError, TxnApplied, TxnErrorKind, TxnPoints}, migrations, reaper::{LiveFrames, ReadPin, Reaper}, store_format::{self, StoreFormat, StoreScope} + }, frame_name, frame_read, index_txn::{IndexOp, IndexRow, IndexTxn, IndexTxnError, TxnApplied, TxnErrorKind, TxnPoints}, migrations, reaper::{LiveFrames, ReadPin, Reaper}, store_format::{self, StoreFormat, StoreScope} }, AspectCatalog, AspectMetadata, AspectMetadataStore, CatalogStore, PartialSidecar, PartialSidecarPolicy, SegmentIndexStore, SnapshotReport, StoreError, CONTROL_PLANE_FILES, SIDECAR_AGGREGATIONS, SUPPORTED_LAYOUT }; @@ -289,7 +289,7 @@ fn scaled_rows(ts: &[i64], vs: &[Option], start: i64, end: i64, u fn segment_partial(descriptor: &SegmentDescriptor, bytes: &[u8], start: i64, end: i64, unit: TimeUnit, resolution: Resolution, aggregations: &[Aggregation]) -> Result> { // Read the window in its physical encoding, so a ScaledI64 column can be reduced on its // stored mantissas without building a BigDecimal per row. - let (ts, vs) = if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { weft_physical_type::weftseg::read_paged_segment_range_physical(bytes, start, end).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))? } else { weft_physical_type::weftseg::read_segment_range_physical(bytes, start, end).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))? }; + let (ts, vs) = frame_read::read_range_physical(descriptor.path.as_str(), bytes, start, end)?; // Integer-native path: every present in-window value is ScaledI64 at one scale and every // requested reduction is a streaming one. `reduce_partial_scaled` then builds the same // mergeable partial `reduce_partial` would, from integer accumulators. @@ -1921,16 +1921,10 @@ impl SegmentStore { bail!("segment {} does not end in the CRC its row was committed with ({expected:#010x})", descriptor.path); } } - if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { - let segment = PagedSegment::read_from(&bytes).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))?; - let rows_per_page = segment.rows_per_page; - let (timestamps, values) = segment.decode_nullable(); - Ok(DecodedFrame { rows_per_page: Some(rows_per_page), timestamps, values }) - } else { - let segment = Segment::read_from(&bytes).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))?; - let (timestamps, values) = segment.decode_nullable(); - Ok(DecodedFrame { rows_per_page: None, timestamps, values }) - } + let frame = frame_read::read_full(descriptor.path.as_str(), &bytes)?; + let rows_per_page = frame.rows_per_page(); + let (timestamps, values) = frame.decode_nullable(); + Ok(DecodedFrame { rows_per_page, timestamps, values }) } /// Read the frames of `members` (ascending by id), checking each is the frame its row @@ -3054,8 +3048,9 @@ impl SegmentStore { /// /// # Errors /// - /// Propagates a libSQL prune failure, a filesystem read error, or a - /// [`weft_physical_type::weftseg::WeftSegError`] for a corrupt/unreadable `.weftseg`. + /// Propagates a libSQL prune failure, a filesystem read error, or a frame-decode + /// failure carrying a [`crate::FrameFaultError`] — the C5 fault class (`integrity`, + /// `newer_format` or `undecodable`) for a corrupt or unreadable `.weftseg`. pub async fn read_time_range(&self, aspect: &str, start: i64, end: i64) -> Result<(Vec, Vec>)> { aspect_name::validate(aspect)?; let _pin = self.pin_reads(); @@ -3070,7 +3065,7 @@ impl SegmentStore { // index range + per-present-row value read) — the paged variant additionally skips whole // pages disjoint from the window — instead of the whole segment; any other shape falls // back to a full decode. Already filtered to `[start, end]`. - let (ts, vs) = if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { weft_physical_type::weftseg::read_paged_segment_range(&bytes, start, end).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))? } else { weft_physical_type::weftseg::read_segment_range(&bytes, start, end).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))? }; + let (ts, vs) = frame_read::read_range(descriptor.path.as_str(), &bytes, start, end)?; for (t, v) in ts.into_iter().zip(vs) { if start <= t && t <= end { timestamps.push(t); @@ -3193,8 +3188,9 @@ impl SegmentStore { /// /// # Errors /// - /// Propagates a libSQL prune failure, a filesystem read error, or a - /// [`weft_physical_type::weftseg::WeftSegError`] for a corrupt/unreadable `.weftseg`. + /// Propagates a libSQL prune failure, a filesystem read error, or a frame-decode + /// failure carrying a [`crate::FrameFaultError`] — the C5 fault class (`integrity`, + /// `newer_format` or `undecodable`) for a corrupt or unreadable `.weftseg`. pub async fn read_point(&self, aspect: &str, t: i64) -> Result> { aspect_name::validate(aspect)?; let _pin = self.pin_reads(); @@ -3205,7 +3201,7 @@ impl SegmentStore { // Streaming single-value read: prunes/skips the pages and value-column blocks a point // lookup does not touch, unpacking only the one block covering `t` on a per-block codec // (roadmap Phase 4/6). Equal to `…read_from(&bytes)?.value_at(t)` for every frame. - let hit = if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { weft_physical_type::weftseg::read_paged_segment_point(&bytes, t).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))? } else { weft_physical_type::weftseg::read_segment_point(&bytes, t).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))? }; + let hit = frame_read::read_point(descriptor.path.as_str(), &bytes, t)?; if hit.is_some() { found = hit; } @@ -3228,8 +3224,9 @@ impl SegmentStore { /// /// # Errors /// - /// Propagates a libSQL prune failure, a filesystem read error, or a - /// [`weft_physical_type::weftseg::WeftSegError`] for a corrupt/unreadable `.weftseg`. + /// Propagates a libSQL prune failure, a filesystem read error, or a frame-decode + /// failure carrying a [`crate::FrameFaultError`] — the C5 fault class (`integrity`, + /// `newer_format` or `undecodable`) for a corrupt or unreadable `.weftseg`. pub async fn read_points(&self, aspect: &str, ts: &[i64]) -> Result>> { aspect_name::validate(aspect)?; let _pin = self.pin_reads(); @@ -3242,7 +3239,7 @@ impl SegmentStore { let mut found = vec![None; ts.len()]; for descriptor in &descriptors { let bytes = self.read_frame(descriptor).await?; - let hits = if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { weft_physical_type::weftseg::read_paged_segment_points(&bytes, ts).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))? } else { weft_physical_type::weftseg::read_segment_points(&bytes, ts).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))? }; + let hits = frame_read::read_points(descriptor.path.as_str(), &bytes, ts)?; // Later (higher seal-id) segments override earlier ones per instant — last-writer-wins. for (slot, hit) in found.iter_mut().zip(hits) { if hit.is_some() { @@ -4239,8 +4236,9 @@ impl SegmentStore { /// /// # Errors /// - /// Propagates a libSQL read failure, a filesystem read error, or a - /// [`weft_physical_type::weftseg::WeftSegError`] for a corrupt/unreadable `.weftseg`. + /// Propagates a libSQL read failure, a filesystem read error, or a frame-decode + /// failure carrying a [`crate::FrameFaultError`] — the C5 fault class (`integrity`, + /// `newer_format` or `undecodable`) for a corrupt or unreadable `.weftseg`. pub async fn read_value_range(&self, aspect: &str, lo: &BigDecimal, hi: &BigDecimal) -> Result<(Vec, Vec)> { aspect_name::validate(aspect)?; let _pin = self.pin_reads(); @@ -4265,13 +4263,7 @@ impl SegmentStore { /// rows aligned `(timestamps, values)` with `None` at every null row. async fn decode_all(&self, descriptor: &SegmentDescriptor) -> Result<(Vec, Vec>)> { let bytes = self.read_frame(descriptor).await?; - if descriptor.format_version == PAGED_SEGMENT_FORMAT_VERSION { - let segment = PagedSegment::read_from(&bytes).map_err(|e| anyhow::anyhow!("decoding paged segment {}: {e}", descriptor.path))?; - Ok(segment.decode_nullable()) - } else { - let segment = Segment::read_from(&bytes).map_err(|e| anyhow::anyhow!("decoding segment {}: {e}", descriptor.path))?; - Ok(segment.decode_nullable()) - } + Ok(frame_read::read_full(descriptor.path.as_str(), &bytes)?.decode_nullable()) } /// Number of sealed segments recorded for `aspect`. @@ -4692,7 +4684,7 @@ mod tests { use serial_test::serial; use tempfile::TempDir; - use weft_physical_type::{timestamp::TimeUnit, PhysicalType}; + use weft_physical_type::{timestamp::TimeUnit, PhysicalType, PAGED_SEGMENT_FORMAT_VERSION}; use super::*; use crate::{types::index_txn::TxnPoints, STORE_FORMAT_FILE}; From 14847d2855d0a2c0207778da959c4790885a64dd Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 12:31:43 -0500 Subject: [PATCH 08/12] Tick X-FRAME: the frame versions are named in one place (#94) --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index a7e0b21d..bc079094 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1159,7 +1159,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [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 `-.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`.* - [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.* - [x] **PAC-3** Library crate hygiene: no global allocator or wgpu features imposed on library users — *#93: two decisions were being made for every program that links these crates. `turso` 0.8.1 puts a `#[global_allocator]` in its own crate root behind its `mimalloc` feature, so the workspace's `features = ["mimalloc"]` on a **published library** (`weftdb/Cargo.toml`) chose the allocator of every embedder's process; and `splimes`' default features pull `wgpu`, so `wgpu` sat in the normal graph of `weftdb`, `weft-line-protocol`, `weft-reduce` and `weft-orchestration` — 52 packages of Vulkan, Metal and DX12 code for two crates that never call a GPU API. The allocator moved to the place that owns the process: `weft-server/src/main.rs` and `weft-tui/src/main.rs` each declare `static ALLOCATOR: mimalloc::MiMalloc` through a direct `mimalloc` dependency, so the same allocator on the same version still runs everything we benchmark and ship, and `weftdb` now carries no allocator feature at all — the plan's own lane row names this slice at `main.rs` (`docs/release/1.0-plan.md:126`). GPU is opt-in the same way: the workspace asks splimes for `default-features = false, features = ["serde"]`, `weft-server` and `weft-bench` add `gpu` themselves, and `weftdb`'s interpolation benches take it as a dev-dependency. Distinct packages in the normal graph: `weft-line-protocol` and `weft-reduce` 77 → 25, `weft-orchestration` 267 → 231, `weftdb` 266 → 227; `cargo tree -p weftdb -i wgpu -e normal` and `-i mimalloc` now return nothing. Two keeps are deliberate: splimes' `serde` stays on because `weftdb::BatchedMeasurement` holds a `splimes::Resolution` and derives `Serialize`/`Deserialize`, and turso's other default, `fts`, stays off exactly as it already was — an attempt to re-add it pulled `tantivy`, `zstd`, `datasketches` and friends, +435 lines of `Cargo.lock`, for full-text search nothing in this workspace uses. `tracing-subscriber` became a dev-dependency of `weftdb` (only test code installs a subscriber). One consequence for our own numbers: `cargo bench -p weftdb` used to inherit mimalloc and now runs on the system allocator, so those nine criterion benches are not comparable across this commit; the nightly regression track (EVI-5) uses `weft-bench`, which never linked `weftdb` and never had mimalloc, so no baseline moves there.* - - [ ] L-FMT: **X-FRAME** Frame registry, v7 envelope, `read_frame*`/`describe_frame`, `FrameFault` classes (absorbs FRE-4, TAG-2) + - [x] L-FMT: **X-FRAME** Frame registry, v7 envelope, `read_frame*`/`describe_frame`, `FrameFault` classes (absorbs FRE-4, TAG-2) — *#94: the three frame versions are named in one place, `weft_physical_type::frame_registry` — 5 single-block, 6 paged, 7 envelope, `layout_of`, `FrameExtension`, `FrameInfo`, `FrameFault { Integrity, Undecodable { newer_format } }` and `fault_class()`, with `KNOWN_EXTENSIONS` deliberately **empty** because an extension id is a permanent namespace. The envelope (`wrap_frame_envelope` in `weft-physical-type/src/weftseg.rs`) stores the older frame verbatim, magic, version and CRC included, for 16 bytes of overhead, so every byte-for-byte pin in this repo still holds and a legacy reader still answers `UnsupportedVersion { found: 7 }`; `frame_parts` verifies that CRC before it trusts a single header field, and `EnvelopeBodyMismatch` catches a header that disagrees with a body which is already under the checksum. Reading is now routed by **what the file says**: `weftdb/src/types/frame_read.rs` probes the header (`frame_layout_of`) instead of trusting `descriptor.format_version`, and its `FrameFaultError` reproduces the historical `decoding paged segment {path}: {sentence}` byte-for-byte, at all six call sites. Nothing opts into v7 yet — writers keep emitting 5 and 6, because `weftdb/src/types/segment_store.rs:7022` promises an unconfigured deployment bytes that are historical. `error::Code::FrameUndecodable` finally has a producer: a frame from a newer version is 503 `incompatible_store` and everything undecodable-but-current is 500, asserted in `weft-server/tests/error_contract.rs` by corrupting a real `.weftseg` (one byte, and a re-stamped-CRC version bump), which also shows the store surviving a damaged frame and the other aspect still reading. 306 `weft-physical-type` and 400 `weftdb` lib tests pass.* - [ ] Legacy lane: **FRE-14** Legacy data-directory marker, numbered migrations and measurement upgrade test - [ ] **PAC-2** Portable Linux binaries on a glibc 2.28 baseline - [ ] **EVI-5** Nightly benchmark-regression workflow and `weft-bench compare` From b5d2a78b62991c1e3702c2ee60ee6f29860c6b35 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 13:47:50 -0500 Subject: [PATCH 09/12] Refuse a 0.1.x data directory instead of opening it as an empty store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A WeftDB 0.1.x data directory keeps one database per aspect: `metadata.db` at the top, then `//measurements.db` two levels down. That top `metadata.db` is one of `CONTROL_PLANE_FILES`, so `control_plane_present` said yes and the open went ahead: it created the three control-plane databases it was missing, made `segments/`, wrote a `STORE_FORMAT` marker beside the old data, ran the layout-1 migrations, and returned an empty store. Every measurement vanished without an error, and the marker then described a tree that was not there. The probe for the shape runs as step 4a of `open_on`: after the marker gate, before any database is opened, and only when the root has no marker — which is exactly where a 0.1.x root sits, since markers begin at layout 2. It reports the `/` pairs it found in `StoreError::RowsModeDataDirectory`, which X-ERR already maps to 503 `incompatible_store` and CLI exit 65, so no new status code is involved. By then the open has taken `LOCK` and written `LOCK.holder`, and the variant's doc says those two files are all it touches: the test asserts the rest of the directory is unchanged, that no marker is added, and that a second open refuses the same way. A refusal that annotated the directory would be read by the next version as understanding it, which is the lie this closes. The other half of the box is the upgrade test, and it compares against the release rather than against this build: the fixture carries v0.1.0's own `aspect_stats.txt` and `stats.json`, so the migrated store has to count 3/725/14 bytes-of-rows for `price` over `0..4070900000`, 2/72/7 for `temp`, 1/80/0 for `flow`, and 877 rows / 21 nulls / 3220 bytes store-wide. Then the directory is moved with `rename` and reopened: no migration runs again, the same integers come back, and the six frames under `segments/` are still six. `ordinal()` is the whole of the registry's ordering, and its `unwrap_or(0)` is silent: an id that does not start with a number is ordinal 0, so it is neither pending nor after and simply never runs. Nothing pinned the format, so the registry test now requires four digits, an underscore, and a name. The earlier note for this slice said `migrations_lock.rs` holds a `MAX_MIGRATION_ID` that should be tied to the registry; that file and constant do not exist in this tree, and the note is corrected rather than the phantom wired up. Reading the rows themselves is FRE-14b, and it is a slice of its own: 0.1.x wrote eight databases per aspect, and `correlations` and `patterns` have no frame equivalent at all. --- CHANGELOG.md | 12 +++ weftdb/src/types/error.rs | 15 +++ weftdb/src/types/migrations.rs | 11 ++- weftdb/src/types/segment_store.rs | 11 +++ weftdb/src/types/store_format.rs | 107 +++++++++++++++++++++ weftdb/tests/compat_v010.rs | 149 +++++++++++++++++++++++++++++- 6 files changed, 300 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b76807e..8466b72a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -303,6 +303,18 @@ only the control plane, not the segment frames. `503 incompatible_store`, and any other frame that will not decode is `500 frame_undecodable`, the code X-ERR gave a wire sentence but no producer. **No writer opts into the envelope yet** — seals keep writing 5 and 6 — so this is the mechanism, not a format change. +- **A WeftDB 0.1.x data directory is refused, not quietly emptied** (1.0 plan FRE-14, first half). + A 0.1.x root keeps one database per aspect — `metadata.db` at the top and + `//measurements.db` two levels down — and that `metadata.db` is one of + `weftdb::CONTROL_PLANE_FILES`, so opening such a root used to **succeed**: it found a control + plane, created the three databases it was missing, added `segments/`, wrote a `STORE_FORMAT` + marker beside the old data, and reported an **empty** store with no error anywhere. Before any + database is opened, `SegmentStore::open` now recognises the shape and returns + `StoreError::RowsModeDataDirectory { aspects }`, naming each `/` pair in path + order. It rides X-ERR's existing mapping — `503 incompatible_store`, CLI exit 65 — so there is no + new status code. The refusal writes nothing but `LOCK` and `LOCK.holder`: the test asserts the + file set is otherwise unchanged, that no marker is added, and that a second open refuses the same + way. Turning those rows into frames is the second half of FRE-14 and is still open. ### Changed diff --git a/weftdb/src/types/error.rs b/weftdb/src/types/error.rs index fe81b816..35c02e0c 100644 --- a/weftdb/src/types/error.rs +++ b/weftdb/src/types/error.rs @@ -151,4 +151,19 @@ pub enum StoreError { /// The aspects with such frames, in name order, each once. aspects: Vec, }, + /// The directory opened is not a store but a **WeftDB 0.1.x data directory**: it keeps a + /// database per aspect at `//measurements.db`, two levels below a + /// top-level `metadata.db`. No reader for that layout exists yet, and opening such a directory as a + /// store would be quietly wrong rather than loud: 0.1.x named its top-level database + /// `metadata.db`, which is one of this build's [`CONTROL_PLANE_FILES`](crate::types::backup::CONTROL_PLANE_FILES), + /// so the open would find a control plane, create the three databases missing from it, add + /// `segments/` and a `STORE_FORMAT` marker next to the old data, and report an empty store + /// with no error at all. It is refused before any database opens and before any of those + /// files exist; only `LOCK` and `LOCK.holder` are touched, as with every other refusal. + #[error("this is a WeftDB 0.1.x data directory, keeping a database per aspect ({}); this version reads stores, and nothing here was changed (only LOCK and LOCK.holder were written)", aspects.iter().map(|aspect| format!("{aspect:?}")).collect::>().join(", "))] + RowsModeDataDirectory { + /// The `/` directories holding a `measurements.db`, in path order, + /// each once. + aspects: Vec, + }, } diff --git a/weftdb/src/types/migrations.rs b/weftdb/src/types/migrations.rs index 815a85d7..f29b7d67 100644 --- a/weftdb/src/types/migrations.rs +++ b/weftdb/src/types/migrations.rs @@ -105,7 +105,9 @@ pub(crate) struct Floors { pub min_write: u32, } -/// Every registered migration, in id order. +/// Every registered migration, in id order. An id is four digits, an underscore, and a +/// lower snake_case name: the digits are the ordinal that `pending` and `after` select on, +/// so the numbering rule is part of the registry's contract and the test below asserts it. pub(crate) static REGISTRY: &[Migration] = &[Migration { id: "0001_baseline", layout: 1, min_read_after: 1, min_write_after: 1, older_writers_safe: false, precheck: None, apply: baseline, rederive: None }, Migration { id: "0002_s6_s7", layout: 2, min_read_after: 2, min_write_after: 2, older_writers_safe: false, precheck: Some(refuse_unsafe_legacy_paths), apply: s6_s7, rederive: None }]; /// The ordinal of a migration id: its leading digits (`0002_s6_s7` → 2). An id without @@ -487,6 +489,13 @@ mod tests { let ordinals: Vec = REGISTRY.iter().map(|migration| ordinal(migration.id)).collect(); assert!(ordinals.windows(2).all(|pair| pair[0] < pair[1]), "{ordinals:?}"); for migration in REGISTRY { + // `ordinal()` takes the digits before the first `_` and falls back to 0 without + // complaint, so an id that does not begin with the numbering rule is ordinal 0: + // never pending, never after, and it silently never runs. The format is what the + // ordering is built from, so it is asserted here rather than left to review. + let (digits, name) = migration.id.split_once('_').unwrap_or((migration.id, "")); + assert!(digits.len() == 4 && digits.bytes().all(|byte| byte.is_ascii_digit()), "{}: a migration id must begin with four digits, which is its ordinal", migration.id); + assert!(!name.is_empty() && name.bytes().all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_'), "{}: a migration id's name follows the digits and is lower snake_case", migration.id); assert!(migration.min_read_after <= migration.layout && migration.min_write_after <= migration.layout, "{}: a floor above its own layout", migration.id); assert!(migration.layout <= crate::SUPPORTED_LAYOUT, "{}: a layout this build does not support", migration.id); if migration.min_write_after < migration.layout { diff --git a/weftdb/src/types/segment_store.rs b/weftdb/src/types/segment_store.rs index bb0016e6..86726970 100644 --- a/weftdb/src/types/segment_store.rs +++ b/weftdb/src/types/segment_store.rs @@ -1455,6 +1455,17 @@ impl SegmentStore { if let Some(marker) = found.as_ref().filter(|marker| !marker.writable_by(SUPPORTED_LAYOUT)) { return Err(anyhow::Error::new(StoreError::IncompatibleLayout { min_write: marker.min_write_layout, supported: SUPPORTED_LAYOUT }).context(opening())); } + // 4a. A root with no marker is not necessarily an empty directory: the 0.1.x line was + // a data directory with one database per aspect, and its top-level database carries + // this build's control-plane name. Opening it as a store would build the rest of a + // store beside that data and report it empty with no error, so it is refused here, + // before any database opens and before any file of ours exists in the directory. + if found.is_none() { + let aspects = store_format::rows_mode_aspects(&root).await; + if !aspects.is_empty() { + return Err(anyhow::Error::new(StoreError::RowsModeDataDirectory { aspects }).context(opening())); + } + } // 7, for a root that has databases: they open now, before anything is written to the // root. segment_index.db opens first and refuses a write floor its store_meta records // newer than this build (the gate of a root without a marker, 4) before anything is diff --git a/weftdb/src/types/store_format.rs b/weftdb/src/types/store_format.rs index a7c92398..feffd7c7 100644 --- a/weftdb/src/types/store_format.rs +++ b/weftdb/src/types/store_format.rs @@ -185,6 +185,49 @@ pub async fn write_marker(fs: &dyn StoreFs, root: &Path, format: &StoreFormat) - Ok(()) } +/// The `/` pairs under `root` that show a **WeftDB 0.1.x data directory** +/// rather than a store, and nothing for anything else. +/// +/// Shape decides, not a name: 0.1.x kept one database per aspect two levels down +/// (`//measurements.db`, the layout `Database::aspect_measurements_db_path` +/// writes — a private helper on the legacy rows-mode type, so named in prose here rather +/// than linked). That top-level name is the hazard — it is one of [`CONTROL_PLANE_FILES`](crate::types::backup::CONTROL_PLANE_FILES), +/// so a data directory looks like a store whose control plane is incomplete, and a layout-1 +/// store root has the same file. What separates the two is the second level: a store's +/// subdirectories hold frames and backup trees, never an aspect's own database. +/// +/// The caller runs this only for a root with no `STORE_FORMAT` marker, so a store at the +/// current layout never pays for it; the probe costs one directory read at the top and one +/// per child directory. +#[must_use] +pub(crate) async fn rows_mode_aspects(root: &Path) -> Vec { + if !tokio::fs::try_exists(root.join("metadata.db")).await.unwrap_or(false) { + return Vec::new(); + } + // A root that cannot be read is not reported as a data directory: the open goes on to + // fail on its own, with its own context, rather than blaming a shape it did not see. + let Ok(mut subjects) = tokio::fs::read_dir(root).await else { return Vec::new() }; + let mut found = Vec::new(); + while let Ok(Some(subject)) = subjects.next_entry().await { + // A symlink's type describes the link, so links are passed over with the files here + // rather than followed into somebody else's tree. + if !subject.file_type().await.is_ok_and(|kind| kind.is_dir()) { + continue; + } + let Ok(mut aspects) = tokio::fs::read_dir(subject.path()).await else { continue }; + while let Ok(Some(aspect)) = aspects.next_entry().await { + if !aspect.file_type().await.is_ok_and(|kind| kind.is_dir()) { + continue; + } + if tokio::fs::try_exists(aspect.path().join("measurements.db")).await.unwrap_or(false) { + found.push(format!("{}/{}", subject.file_name().to_string_lossy(), aspect.file_name().to_string_lossy())); + } + } + } + found.sort(); + found +} + #[cfg(test)] mod tests { use super::*; @@ -220,6 +263,70 @@ mod tests { assert!(format.writable_by(SUPPORTED_LAYOUT) && !format.writable_by(1)); } + /// A helper for the shape probes: a file at `path`, with its parents created. + fn touch(path: &Path) { + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the parent directory"); + std::fs::write(path, b"").expect("the placeholder file"); + } + + #[tokio::test] + async fn a_rows_mode_root_reports_its_aspect_databases_in_order() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + // 0.1.x laid a control-plane `metadata.db` at the root and one database per aspect + // two levels down. The order matters: the refusal names them, and a `read_dir` order + // would make the sentence (and any test of it) move with the filesystem. + touch(&root.join("metadata.db")); + touch(&root.join("plant/temp/measurements.db")); + touch(&root.join("plant/price/measurements.db")); + // A directory under the same parent that is not a measurement store stays unnamed: + // `dictionaries/` and its siblings were 0.1.x plumbing, not a series to migrate. + touch(&root.join("plant/dictionaries/words.db")); + assert_eq!(rows_mode_aspects(root).await, ["plant/price", "plant/temp"]); + } + + #[tokio::test] + async fn a_store_root_is_never_mistaken_for_a_rows_mode_root() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + // The shape that matters is the one the migration path meets: a pre-marker store, + // whose `metadata.db` is a control-plane file, keeps its frames under `segments/`. + // Flagging that root would refuse the very store FRE-14 exists to upgrade. + for file in ["metadata.db", "segment_index.db", "aspect_catalog.db", "catalog.db"] { + touch(&root.join(file)); + } + touch(&root.join("segments/price-0.weftseg")); + // The same shape but not the same file name, and one level deeper than the probe + // looks: neither is a measurement store, because the probe asks for + // `measurements.db` exactly, at `//`. + touch(&root.join("segments/price/measurements.dbx")); + touch(&root.join("plant/price/nested/measurements.db")); + let found = rows_mode_aspects(root).await; + assert_eq!(found, Vec::::new(), "a storage-v2 root with a `segments/` tree is a store, not a data directory"); + // The same root once the marker exists is equally uninteresting, and an empty root + // — the case every new store takes — must not pay more than one directory read. + std::fs::write(root.join(STORE_FORMAT_FILE), r#"{"kind":"weftdb-store","layout_version":2,"min_read_layout":1,"min_write_layout":1,"last_written_layout":2,"store_uuid":"u","scope":{"database":"d","subject":"s"}}"#).expect("a marker"); + assert_eq!(rows_mode_aspects(root).await, Vec::::new(), "the same root once marked is a store too"); + let empty = tempfile::tempdir().unwrap(); + assert_eq!(rows_mode_aspects(empty.path()).await, Vec::::new(), "an empty root — the case every new store takes — finds nothing"); + } + + #[tokio::test] + #[cfg(unix)] + async fn a_rows_mode_probe_does_not_follow_a_symlink() { + // The probe is a refusal, so the worst it may do is name a shape. Following a link + // would let a data directory outside the root decide whether an open is refused. + // Unix only: `std::os::unix::fs::symlink` is the whole point of the test, and + // Windows' equivalent needs developer mode or elevation, so CI would skip it anyway. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let outside = tempfile::tempdir().unwrap(); + touch(&outside.path().join("price/measurements.db")); + touch(&root.join("metadata.db")); + std::os::unix::fs::symlink(outside.path(), root.join("plant")).expect("a symlink"); + assert_eq!(rows_mode_aspects(root).await, Vec::::new(), "a symlinked database directory is not reported"); + } + #[tokio::test] async fn a_marker_that_is_not_one_is_unreadable() { let dir = tempfile::tempdir().unwrap(); diff --git a/weftdb/tests/compat_v010.rs b/weftdb/tests/compat_v010.rs index f892588d..d8e26956 100644 --- a/weftdb/tests/compat_v010.rs +++ b/weftdb/tests/compat_v010.rs @@ -19,16 +19,18 @@ //! - what a v0.1.0 backup actually contains, which is **not** the segment data. That is a //! finding, not an aspiration: see [`v010_backup_covers_the_control_plane_only`]. //! -//! The rows-mode tree is asserted by shape only. Opening it is FRE-14's job (the legacy -//! data-directory marker and numbered migrations), and the tree is here so that slice is -//! driven by what the release wrote instead of by a fixture someone invented. +//! The rows-mode tree is refused, not read: an open of it names the aspect databases and +//! writes nothing (FRE-14a, [`a_v010_rows_mode_directory_is_refused_and_writes_nothing`]). +//! Turning those rows into frames is the other half of FRE-14 and is still open; the tree is +//! here so that slice is driven by what the release wrote instead of by a fixture someone +//! invented. use std::{ collections::BTreeMap, path::{Path, PathBuf} }; use weft_physical_type::{PhysicalType, TimeUnit}; -use weftdb::SegmentStore; +use weftdb::{SegmentStore, StoreError}; /// The committed fixture set, relative to this crate. fn fixture() -> PathBuf { @@ -272,3 +274,142 @@ async fn v010_legacy_tree_is_a_rows_mode_database_of_the_expected_shape() { let grid_nulls = std::fs::read_to_string(fixture().join("rows.csv")).expect("the shared grid is part of the fixture").lines().filter(|line| line.ends_with(",")).count(); assert_eq!(grid_nulls, 13, "the shared grid carries 13 null rows for Storage-v2 to seal; the grid changed"); } + +/// The shape 0.1.x left behind when it kept one database per aspect: a root with a +/// control-plane `metadata.db` and `//measurements.db` below it. This +/// build reads stores, and opening that root would otherwise succeed: `metadata.db` is one +/// of `[`weftdb::CONTROL_PLANE_FILES`]`, so the open finds a control plane, creates the three +/// databases it is missing, adds `segments/`, writes a marker beside the old data, and +/// reports an **empty** store. The refusal has to be loud, and it has to leave nothing. +#[tokio::test] +async fn a_v010_rows_mode_directory_is_refused_and_writes_nothing() { + let source = fixture().join("legacy/v010_legacy"); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant-data"); + copy_tree(&source, &root); + let before = files_under(&root); + + let error = match SegmentStore::open(&root).await { + Ok(_) => panic!("a rows-mode directory is not a store, and saying so is the whole point"), + Err(error) => error, + }; + let typed = error.downcast_ref::().unwrap_or_else(|| panic!("the refusal is typed, not a sentence: {error:#}")); + let StoreError::RowsModeDataDirectory { aspects } = typed else { + panic!("the refusal names the rows-mode shape: {error:#}"); + }; + assert_eq!(aspects.as_slice(), ["plant/flow", "plant/price", "plant/temp"], "the refusal names every aspect database it found, in path order"); + // The sentence is what an operator reads, so it is asserted rather than sampled: it says + // what the directory is, what was not changed, and which files the open did touch. It is + // read off the typed error because the surrounding `anyhow` context replaces `Display` + // with its own sentence — `{error:#}` below is what a CLI prints. + assert_eq!(typed.to_string(), format!("this is a WeftDB 0.1.x data directory, keeping a database per aspect ({}); this version reads stores, and nothing here was changed (only LOCK and LOCK.holder were written)", ["\"plant/flow\"", "\"plant/price\"", "\"plant/temp\""].join(", "))); + assert!(format!("{error:#}").starts_with(&format!("opening segment store {}", root.display())), "the refusal still says which open failed: {error:#}"); + + // Nothing was created but the two lock files. The three databases a store needs, the + // `segments/` directory, and the `STORE_FORMAT` marker are the writes this refusal exists + // to prevent, and their absence is what makes the refusal safe to retry. + let mut created = files_under(&root).into_iter().filter(|name| !before.contains(name)).collect::>(); + created.sort(); + assert_eq!(created, ["LOCK", "LOCK.holder"], "a refused open leaves the data directory exactly as it found it"); + assert!(!before.iter().any(|name| name == "STORE_FORMAT"), "the fixture never had a marker, and the refusal does not add one"); + + // A second open refuses the same way: the refusal is not a state transition. + let again = match SegmentStore::open(&root).await { + Ok(_) => panic!("the same directory is refused twice"), + Err(error) => error, + }; + assert!(format!("{again:#}").contains("a WeftDB 0.1.x data directory"), "{again:#}"); +} + +/// One aspect's measured facts, as the integers a comparison is made of: segment count, +/// rows, nulls, bytes, unsorted segments, overlapping segments, then the timestamp span. +async fn measured(store: &SegmentStore, aspect: &str) -> Vec { + let stats = store.aspect_stats(aspect).await.expect("the aspect is declared"); + let meta = store.aspect_metadata(aspect).await.expect("the aspect has metadata"); + let (lo, hi) = stats.time_range.expect("a sealed aspect covers a span"); + vec![stats.segment_count as u64, stats.total_rows, meta.total_nulls, stats.total_bytes, stats.unsorted_segments as u64, stats.overlapping_segments as u64, lo as u64, hi as u64] +} + +/// The same eight facts as v0.1.0 wrote them into `aspect_stats.txt`. +fn recorded(recorded: &serde_json::Value, keys: [&str; 6]) -> Vec { + let span = recorded["time_range"].as_array().expect("a recorded timestamp span"); + let numbers = |value: &serde_json::Value| value.as_u64().expect("a recorded count"); + let mut out = keys.map(|key| numbers(&recorded[key])).to_vec(); + out.extend(span.iter().map(numbers)); + out +} + +/// The store-wide totals, from the same build of integers: aspects, segments, rows, nulls, +/// bytes, unsorted segments, span. +async fn measured_store(store: &SegmentStore) -> Vec { + let stats = store.store_stats().await.expect("the store reports itself"); + let (lo, hi) = stats.time_range.expect("a store with segments covers a span"); + vec![stats.aspect_count as u64, stats.segment_count as u64, stats.total_rows, stats.total_nulls, stats.total_bytes, stats.unsorted_segments as u64, lo as u64, hi as u64] +} + +/// The X-FX gate the plan asks for in one place: the measurements a v0.1.0 release counted +/// are the measurements this build counts after the layout-1 upgrade, and after the store +/// has been moved to another path. `aspect_stats.txt` and `stats.json` are v0.1.0's own +/// numbers, so the comparison is with the release rather than with this build talking to +/// itself. Bytes are part of the comparison on purpose: nothing in this build's migration +/// re-encodes a frame, so a total that moves is a frame that changed. +#[tokio::test] +async fn the_migrated_measurements_are_the_ones_v010_counted() { + let stats_text = std::fs::read_to_string(fixture().join("aspect_stats.txt")).expect("v0.1.0 recorded its own per-aspect stats"); + let per_aspect: BTreeMap = stats_text + .lines() + .filter(|line| !line.starts_with('#') && !line.trim().is_empty()) + .map(|line| { + let value: serde_json::Value = serde_json::from_str(line.trim()).expect("a recorded stats line"); + (value["aspect"].as_str().expect("a named stats row").to_owned(), value) + }) + .collect(); + assert_eq!(per_aspect.keys().collect::>(), ["flow", "price", "temp"], "the fixture records stats for exactly the three aspects it wrote"); + let store_totals: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(fixture().join("stats.json")).expect("v0.1.0 recorded its own store totals")).expect("the store totals are JSON"); + + let dir = tempfile::tempdir().unwrap(); + let from = dir.path().join("original"); + let to = dir.path().join("relocated"); + copy_tree(&fixture().join("store"), &from); + + let store = SegmentStore::open(&from).await.expect("a v0.1.0 store opens"); + let report = store.open_report(); + assert_eq!(report.migrated_from, Some(1), "the store was upgraded from a pre-marker layout"); + assert!(!report.applied.is_empty(), "the upgrade ran here, so it is named: {:?}", report.applied); + for aspect in ["flow", "price", "temp"] { + assert_eq!(measured(&store, aspect).await, recorded(&per_aspect[aspect], ["segment_count", "total_rows", "total_nulls", "total_bytes", "unsorted_segments", "overlapping_segments"]), "{aspect}: this build counts what v0.1.0 counted"); + } + assert_eq!(measured_store(&store).await, recorded(&store_totals, ["aspect_count", "segment_count", "total_rows", "total_nulls", "total_bytes", "unsorted_segments"]), "the store totals are the ones v0.1.0 printed"); + // Bytes per point is a division rather than a count, so it is compared loosely: the + // recorded value is v0.1.0's float, and a re-derived frame would move it by more than + // this while a different rounding of the same quotient cannot. + for aspect in ["flow", "price", "temp"] { + let recorded = per_aspect[aspect]["bytes_per_point"].as_f64().expect("a recorded ratio"); + let measured = store.aspect_stats(aspect).await.expect("the aspect is declared").bytes_per_point; + assert!((recorded - measured).abs() < 1e-9, "{aspect}: {measured} against the recorded {recorded}"); + } + // The value span is recorded as scaled decimal text, which is what makes it comparable + // through `canonical`: 100.2500 and 100.25 are the same measurement. + for aspect in ["flow", "price", "temp"] { + let want = (canonical(per_aspect[aspect]["value_range"][0].as_str().expect("a recorded low")), canonical(per_aspect[aspect]["value_range"][1].as_str().expect("a recorded high"))); + let got = store.aspect_metadata(aspect).await.expect("the aspect has metadata").value_range.map(|(lo, hi)| (canonical(&lo.to_string()), canonical(&hi.to_string()))); + assert_eq!(got.as_ref(), Some(&want), "{aspect}: the value range survives the upgrade"); + } + + // The upgrade settles: it ran once, and the tree it left behind is what a later start + // reads without running anything again. + drop(store); + std::fs::rename(&from, &to).expect("the store moves"); + let moved = SegmentStore::open(&to).await.expect("the same store opens at a new path"); + let report = moved.open_report(); + assert_eq!(report.migrated_from, None, "a migrated store does not migrate again"); + assert!(report.applied.is_empty(), "nothing ran on the second open: {:?}", report.applied); + for aspect in ["flow", "price", "temp"] { + assert_eq!(measured(&moved, aspect).await, recorded(&per_aspect[aspect], ["segment_count", "total_rows", "total_nulls", "total_bytes", "unsorted_segments", "overlapping_segments"]), "{aspect}: a moved store counts the same measurements"); + } + assert_eq!(measured_store(&moved).await, recorded(&store_totals, ["aspect_count", "segment_count", "total_rows", "total_nulls", "total_bytes", "unsorted_segments"]), "the moved store's totals are still v0.1.0's"); + // The frames themselves: the segment files under the moved root are the ones the fixture + // shipped, which is why the byte totals above can be trusted as identity. + let frames = files_under(&to).into_iter().filter(|name| name.ends_with(".weftseg")).collect::>(); + assert_eq!(frames.iter().filter(|name| name.starts_with("segments/")).count(), 6, "the upgrade moved six frames forward and did not rewrite or add any: {frames:?}"); +} From 96e62afbdd5e1f5b087fcee057b69060f9cbda0a Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Wed, 21 Oct 2026 20:35:28 -0500 Subject: [PATCH 10/12] Read a 0.1.x rows-mode tree into a store, and record that it was read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 14a closed the door on a 0.1.x data directory: the open now refuses a root whose series sit at `//measurements.db` instead of creating the three control-plane databases it was missing over the top of the operator's measurements and reporting an empty store. The other half of that box is a leg that reads the tree, because a refusal that is also the end of the road leaves the measurements stranded in a format nothing current can open. `weftdb::rows_mode` is that leg: `discover` reports the series a tree holds and lists, with reasons, everything it will not carry — the `metadata.db` catalog, the databases 0.1.x kept beside the rows (`ingestion.db`, `pipeline.db`, `correlations.db`, `patterns.db`, `processed_*.db`, `unprocessed_*.db`) and the `dictionaries/` directories. It opens each series database and checks its shape before any store is opened, so a directory that is not what 0.1.x wrote fails there rather than half-way through a write. An aspect's name comes from its directory, and its value column has to be `value`. 0.1.x's own server created `measurements (id TEXT NOT NULL, dataset_id TEXT NOT NULL, timestamp INTEGER NOT NULL, value TEXT NOT NULL)` (`v0.1.0:weftdb/src/types/aspect.rs:475`, repeated at `:1195`) and writes nothing into that file that names the series; the fixture's page images print the same DDL. An earlier draft of this reader also took "whichever column is named after the aspect", on a story about a 2322 migration renaming the column — there is no such migration in the tag, and the story was mine rather than the source's. It was also worse than unattested: an aspect named `id`, `dataset_id` or `timestamp` had been read out of the column holding its identifiers — the release writes `tx_id.as_uuid()` into `id` and the dataset's UUID into `dataset_id` (`v0.1.0:weftdb/src/types/database/inputs.rs:136-142`) — and every row of that import would have been an identifier passing for a measurement, with nothing afterwards able to say so. `pick_value_column` requires the pair the DDL created and refuses the rest, naming the columns it found. `a_series_database_that_is_not_the_shape_0_1_x_wrote_is_refused` pins the refusal, and `an_aspect_named_after_an_identifier_column_is_still_read_from_value` pins the side that is legal: an aspect actually called `dataset_id` is a name 0.1.x allowed, named by its directory, read from `value` and from nowhere else. The timestamp unit is declared, never inferred, and the fixture is why that is not a theoretical rule. rows-mode stamped `timestamp_millis()` into the `INTEGER` (`v0.1.0:weftdb/src/types/database/inputs.rs:142`, `:187`) whatever the aspect's recorded `resolution` claimed — that column is the sampling grid, not a unit — while 0.1.x's batch path kept seconds, which is why the *other* tree in the same fixture, from the same release and the same `rows.csv`, is second-scale and migrates to a store that declares `TimeUnit::Seconds`. Same grid, two trees, two scales, a factor of a thousand apart: a reader that guessed from the size of the numbers had a plausible wrong answer in both directions, and the wrong answer looks like a date. `TIMESTAMP_UNIT`/`TIMESTAMP_TIME_UNIT` say millis with their citation, the fixture test asserts the import declares millis while the migrated store declares seconds. The warning could not go in the fixture's own README: every file under `compat/fixtures/` is hash-pinned in `MANIFEST.sha256`, and a fixture set is append-only — the release's bytes are what the compatibility gates compare, prose included. So the sentence sits in `ROADMAP.md`'s FRE-14 line, where the lane an operator follows actually passes. The trap closes from the store's side too: `an_aspect_declared_at_another_timestamp_scale_refuses_the_reading` takes an aspect the operator declared by hand at `Seconds` that holds no rows yet, and refuses the reading with both schemas printed rather than filing its rows a thousand-fold away from the instants somebody sized their queries around. Sealing does not use `rows.chunks`: `chunks_keeping_instants_together` extends a chunk until a group of rows sharing one instant ends, so a segment can come out a few rows longer than the caller asked for. That is the store's hazard rather than a legacy one. 0.1.x allowed several rows at one instant — `ImportedAspect::duplicate_timestamps` counts them — and the store treats that group differently depending where it lands: rows sharing an instant inside one segment survive maintenance, while rows sharing an instant across two segments whose windows intersect are folded by `reconcile_segment` under `merge_newer_wins`, which keeps only the more recently sealed one (`weftdb/src/types/segment_store.rs:3666-3672`). A fixed-size chunk that cut a group in two would hand it to a compaction pass this reading does not control, days after an import that had already looked clean. A second reading of the same tree refuses (`AlreadyImported`) rather than skipping, and the `ImportRecord` is what makes that answer available: one `store_meta` row under `rows_mode_import::` in the store's own index, holding the tree's path, the counts per aspect, the encoding each aspect was declared under, `duplicate_timestamps`, and what stayed behind. It is read back after the store is closed and reopened, so the idempotency is the store's rather than a process-local memory. Refuse-or-duplicate is the only honest pair of options once a reading has happened: 0.1.x's row ids are UUID transaction ids that this leg does not carry, and the store keeps no record of which rows came from a migration, so a migrated row and a row the store sealed itself are indistinguishable. The tree is read, not written, but it is not opened read-only either, and the reason is worth keeping: a WAL-mode database — the shipped fixture has a `measurements.db-wal` beside every series — can need WAL recovery at open, which a read-only connection cannot perform, so read-only is a failure mode rather than a guarantee. What holds the line is tests: `reading_the_tree_leaves_every_file_it_found_as_it_found_it` compares the tree's files before and after, and the fixture test compares the copy against the release's own bytes file for file. The boundaries the slice does not close are in the module doc: one aspect is held in memory whole because the encoding decision (`recommend_encoding`, tolerance zero) and `seal_declared` both take whole columns; and a reading that stops partway leaves the aspects that already crossed in the store with no record, so the next attempt refuses on `AspectNotEmpty` instead of finishing the job — pinned by a test, and 14c's to handle. 14c is the route an operator runs: the CLI/server path, the numbered migrations that register `metadata.db`'s schema, and a reading of a tree too big for RAM. --- CHANGELOG.md | 39 +- ROADMAP.md | 2 +- weftdb/src/types/mod.rs | 2 + weftdb/src/types/rows_mode.rs | 1143 +++++++++++++++++++++++++++++++++ weftdb/tests/compat_v010.rs | 129 +++- 5 files changed, 1308 insertions(+), 7 deletions(-) create mode 100644 weftdb/src/types/rows_mode.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8466b72a..c7bbe2fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -314,7 +314,44 @@ only the control plane, not the segment frames. order. It rides X-ERR's existing mapping — `503 incompatible_store`, CLI exit 65 — so there is no new status code. The refusal writes nothing but `LOCK` and `LOCK.holder`: the test asserts the file set is otherwise unchanged, that no marker is added, and that a second open refuses the same - way. Turning those rows into frames is the second half of FRE-14 and is still open. + way. Turning those rows into frames is the second half of FRE-14, below. +- **A WeftDB 0.1.x data directory can be read into a store** (1.0 plan FRE-14, second half). + `weftdb::rows_mode` reads the rows-mode tree the refusal above names. `discover` reports the + series under `//measurements.db` — row count, span, how many rows share an + instant — and lists, with a reason, everything it will not carry: the `metadata.db` catalog, + the databases 0.1.x kept beside the rows, and the `dictionaries/` directories. An aspect's name + comes from its **directory**, not from the database: `measurements.db` holds only + `(id, dataset_id, timestamp, value)` — 0.1.x's own DDL (`weftdb/src/types/aspect.rs` in the + `v0.1.0` tag) — so the directory is the only record of what a series is called. The value column + has to be the `value` that DDL created: a database that names it after the aspect is refused, not + read, because an aspect called `dataset_id` would otherwise be read out of the column holding its + identifiers. `import_into` reads + one subject into the store scoped to that `(database, subject)` pair, declares each aspect under + the narrowest encoding that holds its values exactly (tolerance zero, `recommend_encoding` — the + reading trades no precision across the version boundary), seals the rows in batches + (`rows_per_segment`, default `DEFAULT_ROWS_PER_SEGMENT`) without ever splitting a group of rows that + share one instant across two segments — the store folds two segments whose windows intersect and + keeps the newer row at an instant they share, so a group cut in two would lose rows in a compaction + the reading does not control; a segment can therefore come out a few rows longer than asked. The + reading then writes an `ImportRecord` into the + store's own index under `rows_mode_import::`; `read_import_record` reads it + back after a reopen. That record is what stops the same tree being read twice + (`RowsModeError::AlreadyImported`), because after the fact a migrated row and a row the store + sealed itself are indistinguishable — so an aspect that already holds segments refuses first + (`AspectNotEmpty`), as does a tree whose database directory is not the store's database + (`ScopeMismatch`: 0.1.x puts its database only in the path, so the path is the only thing that + can say where a tree belongs). Timestamps are `Millis` and cross unchanged: 0.1.x stamped + `timestamp_millis()` into an `INTEGER`, and the `resolution` its catalog records is the sampling + grid, not a unit, so there is nothing to infer and nothing is inferred. A value cell that is not + the text of a decimal stops the read and quotes the row (`ValueNotDecimal`) rather than putting + a number nobody can vouch for into the store. The tree keeps its bytes: each `measurements.db` + is opened bare, not through the control plane's `connect`, which switches journal mode and so + writes; the test compares every file it found, byte for byte, after the reading. The aspect list is + written one aspect at a time and its record last, so a reading that stops partway leaves the aspects + that already crossed in the store with nothing recorded — the next attempt refuses on `AspectNotEmpty` + rather than finishing the job, and saying what an operator does then is the remaining part of FRE-14: + the route an operator runs from the command line or the server, the numbered migrations that register + `metadata.db`'s schema, and a reading of a tree too big to hold one aspect in memory. ### Changed diff --git a/ROADMAP.md b/ROADMAP.md index bc079094..39d3d016 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1160,7 +1160,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [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.* - [x] **PAC-3** Library crate hygiene: no global allocator or wgpu features imposed on library users — *#93: two decisions were being made for every program that links these crates. `turso` 0.8.1 puts a `#[global_allocator]` in its own crate root behind its `mimalloc` feature, so the workspace's `features = ["mimalloc"]` on a **published library** (`weftdb/Cargo.toml`) chose the allocator of every embedder's process; and `splimes`' default features pull `wgpu`, so `wgpu` sat in the normal graph of `weftdb`, `weft-line-protocol`, `weft-reduce` and `weft-orchestration` — 52 packages of Vulkan, Metal and DX12 code for two crates that never call a GPU API. The allocator moved to the place that owns the process: `weft-server/src/main.rs` and `weft-tui/src/main.rs` each declare `static ALLOCATOR: mimalloc::MiMalloc` through a direct `mimalloc` dependency, so the same allocator on the same version still runs everything we benchmark and ship, and `weftdb` now carries no allocator feature at all — the plan's own lane row names this slice at `main.rs` (`docs/release/1.0-plan.md:126`). GPU is opt-in the same way: the workspace asks splimes for `default-features = false, features = ["serde"]`, `weft-server` and `weft-bench` add `gpu` themselves, and `weftdb`'s interpolation benches take it as a dev-dependency. Distinct packages in the normal graph: `weft-line-protocol` and `weft-reduce` 77 → 25, `weft-orchestration` 267 → 231, `weftdb` 266 → 227; `cargo tree -p weftdb -i wgpu -e normal` and `-i mimalloc` now return nothing. Two keeps are deliberate: splimes' `serde` stays on because `weftdb::BatchedMeasurement` holds a `splimes::Resolution` and derives `Serialize`/`Deserialize`, and turso's other default, `fts`, stays off exactly as it already was — an attempt to re-add it pulled `tantivy`, `zstd`, `datasketches` and friends, +435 lines of `Cargo.lock`, for full-text search nothing in this workspace uses. `tracing-subscriber` became a dev-dependency of `weftdb` (only test code installs a subscriber). One consequence for our own numbers: `cargo bench -p weftdb` used to inherit mimalloc and now runs on the system allocator, so those nine criterion benches are not comparable across this commit; the nightly regression track (EVI-5) uses `weft-bench`, which never linked `weftdb` and never had mimalloc, so no baseline moves there.* - [x] L-FMT: **X-FRAME** Frame registry, v7 envelope, `read_frame*`/`describe_frame`, `FrameFault` classes (absorbs FRE-4, TAG-2) — *#94: the three frame versions are named in one place, `weft_physical_type::frame_registry` — 5 single-block, 6 paged, 7 envelope, `layout_of`, `FrameExtension`, `FrameInfo`, `FrameFault { Integrity, Undecodable { newer_format } }` and `fault_class()`, with `KNOWN_EXTENSIONS` deliberately **empty** because an extension id is a permanent namespace. The envelope (`wrap_frame_envelope` in `weft-physical-type/src/weftseg.rs`) stores the older frame verbatim, magic, version and CRC included, for 16 bytes of overhead, so every byte-for-byte pin in this repo still holds and a legacy reader still answers `UnsupportedVersion { found: 7 }`; `frame_parts` verifies that CRC before it trusts a single header field, and `EnvelopeBodyMismatch` catches a header that disagrees with a body which is already under the checksum. Reading is now routed by **what the file says**: `weftdb/src/types/frame_read.rs` probes the header (`frame_layout_of`) instead of trusting `descriptor.format_version`, and its `FrameFaultError` reproduces the historical `decoding paged segment {path}: {sentence}` byte-for-byte, at all six call sites. Nothing opts into v7 yet — writers keep emitting 5 and 6, because `weftdb/src/types/segment_store.rs:7022` promises an unconfigured deployment bytes that are historical. `error::Code::FrameUndecodable` finally has a producer: a frame from a newer version is 503 `incompatible_store` and everything undecodable-but-current is 500, asserted in `weft-server/tests/error_contract.rs` by corrupting a real `.weftseg` (one byte, and a re-stamped-CRC version bump), which also shows the store surviving a damaged frame and the other aspect still reading. 306 `weft-physical-type` and 400 `weftdb` lib tests pass.* - - [ ] Legacy lane: **FRE-14** Legacy data-directory marker, numbered migrations and measurement upgrade test + - [ ] Legacy lane: **FRE-14** Legacy data-directory marker, numbered migrations and measurement upgrade test — *two of its three parts are in: 14a makes the build **refuse** a 0.1.x rows-mode directory instead of opening it as an empty store (`store_format`'s probe and the refusal at open), and 14b gives that tree a reader — `weftdb::rows_mode` discovers the series under `//measurements.db`, reads one subject into a store scoped to its `(database, subject)`, and records the reading in that store's own index under `rows_mode_import::`, which is what makes a second reading a refusal rather than a doubling of rows. `the_v010_rows_mode_tree_reads_into_a_store_of_its_own_database` takes the tree the release itself wrote and carries all 371 of its rows into a store of that tree's own database — 237 `price`, 54 `temp`, 80 `flow`, the non-null subset of `rows.csv`, each arriving at the instant its writer stamped, a thousand times the number the grid spells — and afterwards every file of the tree is still byte for byte what the release left. What remains is 14c: the route an operator runs, the numbered migrations that register `metadata.db`'s schema, and a reading of a tree too big to hold one aspect in RAM. One trap is pinned where the two trees meet: the same release stamped its rows-mode instants in **millis** (`v0.1.0:weftdb/src/types/database/inputs.rs:142`) and its Storage-v2 instants in **seconds** from the same value grid, so neither leg may read the unit off the numbers it is holding.* - [ ] **PAC-2** Portable Linux binaries on a glibc 2.28 baseline - [ ] **EVI-5** Nightly benchmark-regression workflow and `weft-bench compare` - [ ] **EVI-2** Claims registry and markdown claim/link lint diff --git a/weftdb/src/types/mod.rs b/weftdb/src/types/mod.rs index 05d46115..cc92ebe2 100644 --- a/weftdb/src/types/mod.rs +++ b/weftdb/src/types/mod.rs @@ -26,6 +26,7 @@ pub use partial_sidecar::{PartialSidecar, PartialSidecarPolicy, DEFAULT_PARTIAL_ pub use pattern::{Pattern, PatternID}; pub use pipeline::{DetectorMetadata, DetectorType, PipelineConfig, PipelineState}; pub use relative::Relative; +pub use rows_mode::{discover, import_into, read_import_record, record_key, ImportRecord, ImportedAspect, LegacyAspect, LegacyTree, NotCarried, RowsModeError, DEFAULT_ROWS_PER_SEGMENT, IMPORT_RECORD_KEY, MEASUREMENTS_FILE, TIMESTAMP_COLUMN, TIMESTAMP_TIME_UNIT, TIMESTAMP_UNIT}; pub use segment_index::SegmentIndexStore; #[cfg(feature = "fault-injection")] #[doc(hidden)] @@ -70,6 +71,7 @@ pub mod pattern; pub mod pipeline; pub(crate) mod reaper; pub mod relative; +pub mod rows_mode; pub mod segment_index; pub mod segment_store; pub mod signal; diff --git a/weftdb/src/types/rows_mode.rs b/weftdb/src/types/rows_mode.rs new file mode 100644 index 00000000..5ada3631 --- /dev/null +++ b/weftdb/src/types/rows_mode.rs @@ -0,0 +1,1143 @@ +//! Reading a released 0.1.x **rows-mode** database into the segment store (roadmap +//! **FRE-14b**). +//! +//! 0.1.x kept a measurement history per aspect in its own libSQL database, and this +//! build refused such a tree at the open (FRE-14a, whose probe is `rows_mode_aspects` in +//! `weftdb/src/types/store_format.rs`): the engine never opens Turso, so nothing in it could +//! read those files. This module is the leg that does read them — the input FRE-14 always had +//! to take over. +//! +//! ## What a rows-mode tree is +//! +//! `compat/v010/README.md` records the layout as the generator leaves it: +//! +//! ```text +//! / +//! ├── metadata.db # subjects, aspects, the database row — turso, not SQLite +//! └── // +//! ├── measurements.db # the series: id, dataset_id, timestamp, value +//! ├── dictionaries/ +//! └── pipeline.db correlations.db patterns.db processed_*.db unprocessed_*.db +//! ``` +//! +//! The series lives in `measurements`, four columns — `id TEXT NOT NULL` (the 0.1.x +//! transaction id of the write that produced the row, `tx_id.as_uuid()`: not a WeftDB id, +//! and not carried), `dataset_id TEXT NOT NULL` (the dataset's UUID, the same string on +//! every row of one dataset — `v0.1.0:weftdb/src/types/database/inputs.rs:136-142` for both +//! columns), `timestamp INTEGER NOT NULL` and `value TEXT NOT NULL` (the decimal written as +//! text). Neither identifier is a measurement, which is why the value column is picked by +//! name rather than by elimination: `dataset_id` is also a legal aspect name. Rows-mode has +//! no null value: the fixture generator **skips** a grid row with no value rather than +//! writing a sentinel, and says so on stderr, so the tree is the non-null subset of +//! whatever was captured. +//! +//! **The timestamp unit is milliseconds, always.** 0.1.x wrote the column through +//! `timestamp_millis()` (`v0.1.0:weftdb/src/types/database/inputs.rs`, in +//! `capture_measurement` and its siblings), whatever the aspect's recorded `resolution` +//! said: that column describes the sampling grid the aspect was declared with, not the +//! unit of storage. So the integers carry over unchanged and the schema declares +//! [`TimeUnit::Millis`] — no derivation from the values, which would guess wrong for the +//! same reason the release did. +//! +//! ## What this module does with it +//! +//! [`discover`] walks the tree and *reports* it: the series it can read, each one's row +//! count, time span and duplicate timestamps, and everything in the tree it will not +//! carry ([`NotCarried`], with the reason named per path). It opens each +//! `measurements.db` and checks the shape it needs, so a tree that is not what 0.1.x +//! wrote fails there, before any store is opened. +//! +//! [`import_into`] reads one subject's series into a store that is already open: it +//! declares each aspect under the narrowest **lossless** encoding its values fit +//! ([`recommend_encoding`] at tolerance `0` — the migration never trades precision), +//! seals the rows in time order in chunks of `rows_per_segment` (a chunk never splits +//! between two rows with the same timestamp), and writes an [`ImportRecord`] into +//! `store_meta` naming the tree, the counts, the encodings, and what stayed behind. +//! +//! ## What it refuses +//! +//! A tree with no `metadata.db`; a `measurements.db` with no `measurements` table or +//! without a `timestamp`/`value` it can type; a value that is not a decimal; a tree whose +//! own directory is not the database the store is scoped to, which is the only scope 0.1.x +//! recorded anywhere; an aspect this store already declares under a different schema or +//! already holds segments of; and a tree this store has already imported. Every refusal is typed ([`RowsModeError`]) and +//! names the file. The refusal of an already-imported tree is the idempotency the roadmap +//! asks for: the readable leg cannot tell the tree's rows apart from rows already in the +//! store, so a second import of the same tree is a duplicate, not a no-op — and a +//! duplicate is not something it will do quietly. +//! +//! ## Boundaries +//! +//! - **Memory**: one aspect's `(timestamp, value)` columns are held at once, because the +//! encoding declaration and [`SegmentStore::seal_declared`] both take whole columns. A +//! bounded-memory pass (survey in SQL, seal from a cursor) is a follow-up, not this slice. +//! - **The tree is not written to, but it is not opened read-only either**: turso may fold +//! a `-wal` into the database file when it opens one. Nothing here runs a write +//! statement. An operator who must leave the tree byte-for-byte identical points WeftDB +//! at a copy — which is what `compat/fixtures/v0.1.0/legacy/` is for. +//! - **The record's write is one `store_meta` row** ([`IMPORT_RECORD_KEY`]) through the +//! index's transaction path. `MetaSet` replaces rather than compares (its guard check is +//! `IndexOp::is_guarded`, which covers the row-writing variants and not this one), so two +//! imports racing +//! on one store leave the record of whichever wrote last; the segments each sealed are +//! conflict-guarded inserts, so no measurement is lost or replaced either way. +//! - **A reading that stops partway leaves what crossed.** `ScopeMismatch`, `NoSubject`, +//! `AlreadyImported` and the catalog refusals all arrive before a byte is written. After +//! that the aspects are declared and sealed one at a time — there is no transaction across +//! aspects, since sealing is per aspect — and the record is written last. So a cell that +//! will not type on the third aspect leaves the first two in the store with no record, and +//! the next attempt refuses on [`RowsModeError::AspectNotEmpty`] for an aspect it already +//! holds instead of finishing the job. The shape check in [`discover`] is what makes the +//! middle failure rare; the stranding itself is 14c's to handle, and it is pinned by +//! `a_row_that_is_not_a_decimal_stops_the_read_and_names_its_row`. +//! - **Not carried**: the legacy catalog (`metadata.db` — the store declares its own), +//! the per-aspect `dictionaries/`, and the ingestion/pipeline/correlation/pattern +//! databases beside the rows. They are listed with their reasons rather than dropped +//! silently; reading them is the rest of FRE-14's subsystem list, and 1.0 keeps them +//! behind. +//! +//! [`discover`] deliberately duplicates the path walk of the open's refusal probe: that +//! probe is best-effort by design (an unreadable root is reported as *not* a data +//! directory, so the open fails with its own context), while discovery here has to name +//! what it could not read. + +use std::{ + num::NonZeroUsize, path::{Path, PathBuf}, str::FromStr +}; + +use anyhow::{bail, Context, Result}; +use bigdecimal::BigDecimal; +use chrono::Utc; +use serde::{Deserialize, Serialize}; +use thiserror::Error as ThisError; +use turso::{Builder, Value}; +use weft_physical_type::{recommend_encoding, AspectSchema, PhysicalType, TimeUnit}; + +use crate::{ + aspect_name, types::index_txn::{IndexOp, IndexTxn}, SegmentStore +}; + +/// The file one aspect's history lives in, under `//`. +pub const MEASUREMENTS_FILE: &str = "measurements.db"; + +/// The column every rows-mode row keeps its instant in. +pub const TIMESTAMP_COLUMN: &str = "timestamp"; + +/// The `store_meta` key an [`ImportRecord`] is written under. +/// +/// One row per store, because one store holds one `(database, subject)` scope and a +/// rows-mode tree is read into the store of each subject it carries. +pub const IMPORT_RECORD_KEY: &str = "rows_mode_import"; + +/// Rows per sealed segment when the caller does not choose. +/// +/// Small enough that a migrated aspect starts inside the range the store's own +/// maintenance is shaped for, large enough that a fixture-sized series is one segment. +pub const DEFAULT_ROWS_PER_SEGMENT: usize = 10_000; + +/// The name of the unit the migrated timestamps are declared in ([`TimeUnit::Millis`]). +pub const TIMESTAMP_UNIT: &str = "millis"; + +/// The unit every timestamp an import writes is declared in. +/// +/// 0.1.x put milliseconds into `measurements.timestamp` — `capture_measurement` and +/// `capture_new_measurement` both bind `measurement.timestamp().timestamp_millis()` into the insert +/// (`v0.1.0:weftdb/src/types/database/inputs.rs:142`, `:187`), over a column that is only ever +/// `timestamp INTEGER NOT NULL` (`v0.1.0:weftdb/src/types/aspect.rs:475`) — while its HTTP layer +/// served the aspect's declared unit, which the fixture records as `seconds` +/// (`compat/fixtures/v0.1.0/aspects.json`), and so divides the stored number by a thousand. The +/// number we carry is the stored one, and declaring it `Millis` keeps that number meaning what it +/// says: the unit is declared, never inferred from how big the numbers happen to look. +/// +/// The trap to keep out of this module and out of 14c: the *other* tree in the same fixture — the +/// Storage-v2 root the same release wrote from the same value grid — is second-scale, because 0.1.x's +/// batch path kept seconds while its rows-mode path kept millis. Copy that unit here and every +/// migrated row lands a factor of a thousand early; apply this one there and every migrated row lands +/// a thousand times late. Both sides produce plausible-looking numbers, which is why both name their +/// unit in code with the line that says so. +pub const TIMESTAMP_TIME_UNIT: TimeUnit = TimeUnit::Millis; + +/// Chunk rows for sealing so that rows sharing one instant never straddle a segment. +/// +/// 0.1.x let a series hold several rows at the same instant, and this module carries them: they are +/// what [`ImportedAspect::duplicate_timestamps`] counts. The store does not treat them the same way +/// twice over. A range read returns them all, and each segment keeps its own internal duplicates +/// through maintenance — but when two segments' *windows* intersect, `reconcile_segment` folds them +/// into one under `merge_newer_wins`, and at an instant both members hold only the more recently +/// sealed row survives (`weftdb/src/types/segment_store.rs:3666-3672`). So a chunk boundary that cut +/// a group of same-instant rows in two would hand that group to a maintenance pass that drops all but +/// one of them, silently, some time after the import looked clean. Keeping the group in one segment +/// keeps the tree's answer at that instant intact. It costs a segment that is a few rows longer than +/// the caller asked for, which is the cheaper side of the trade. +fn chunks_keeping_instants_together<'a>(rows: &'a [(i64, BigDecimal)], rows_per_segment: usize) -> Vec<&'a [(i64, BigDecimal)]> { + let mut chunks = Vec::new(); + let mut start = 0usize; + while start < rows.len() { + let mut end = (start + rows_per_segment).min(rows.len()); + if end < rows.len() { + let boundary = rows[end - 1].0; + while end < rows.len() && rows[end].0 == boundary { + end += 1; + } + } + chunks.push(&rows[start..end]); + start = end; + } + chunks +} + +/// Why a rows-mode tree was refused, or why an import stopped. +/// +/// It arrives inside an [`anyhow::Error`]; `downcast_ref::()` recovers it +/// whatever context the read added. +#[derive(Debug, Clone, PartialEq, Eq, ThisError)] +#[non_exhaustive] +pub enum RowsModeError { + /// The tree root holds no `metadata.db`, which is where 0.1.x kept its subjects, its + /// aspects and its database row. Without it there is nothing to say the directory is a + /// database at all rather than a directory that happens to hold `*.db` files. + #[error("{} is not a 0.1.x database: it holds no `metadata.db`, where 0.1.x kept its subjects, aspects and database row", .root.display())] + NoCatalog { root: PathBuf }, + /// A `measurements.db` that is not the series database 0.1.x wrote: no `measurements` + /// table, or none carrying its `timestamp INTEGER NOT NULL` / `value TEXT NOT NULL` pair. + /// The readable leg asks the database which columns it has instead of assuming, and the + /// refusal names the columns it did find. + #[error("{} is not a 0.1.x series database: {}", .path.display(), .detail)] + NoSeries { path: PathBuf, detail: String }, + /// A row whose `timestamp` is not an integer. 0.1.x wrote `timestamp INTEGER NOT NULL`; + /// anything else is a database this module does not know how to read, and guessing would + /// put a wrong instant in the store under a name the operator would trust. A `value` cell + /// that is not the text of a decimal is [`ValueNotDecimal`](Self::ValueNotDecimal). + /// + /// `rowid` is the row's position in the read, counting from one — the 0.1.x row id is not + /// part of what the readable leg carries, and a row that fails is reported by where it + /// was found. It is 0 when the check that failed was an aggregate over the series rather + /// than one row. + #[error("{} row {rowid}: {}", .path.display(), .detail)] + RowShape { path: PathBuf, rowid: i64, detail: String }, + /// A `value` that does not parse as a decimal. 0.1.x stored `BigDecimal` as its text, + /// so a value that does not parse is not a value at all. + #[error("{} row {rowid}: `value` holds {found:?}, which is not a decimal number; 0.1.x wrote the decimal as text", .path.display())] + ValueNotDecimal { path: PathBuf, rowid: i64, found: String }, + /// The store already holds an [`ImportRecord`] naming this tree. Re-reading it would + /// add the same rows a second time: the readable leg has no way to tell a migrated row + /// apart from one it sealed before, so a second run refuses rather than doubling. + #[error("this store already imported {tree} at {imported_at} ({aspects} aspect(s), {rows} row(s)); the readable leg cannot tell those rows apart from the tree's, so it will not read the same tree twice")] + AlreadyImported { tree: String, imported_at: String, aspects: usize, rows: u64 }, + /// The aspect already has segments in this store, sealed by something other than this + /// import. Its rows and the tree's cannot be told apart afterwards, so the import stops + /// rather than mixing them. + #[error("aspect {aspect:?} already holds {segments} segment(s) this import did not seal; the readable leg cannot tell the tree's rows apart from those")] + AspectNotEmpty { aspect: String, segments: usize }, + /// The aspect is already declared in this store under a schema other than the tree's + /// rows need. Re-declaring it would silently re-encode rows the store already holds. + #[error("aspect {aspect:?} is declared {declared} in this store, but its rows need {wanted}; the readable leg will not re-declare an aspect it did not create")] + SchemaConflict { aspect: String, declared: String, wanted: String }, + /// The tree's database directory has one name and the store is scoped to another. + /// 0.1.x named a database by that directory, and two databases can hold the same + /// subject name: reading one into the other's store would put its rows where the + /// operator would look for the other's. + #[error("{tree:?} is 0.1.x database {tree_database:?}, but this store is scoped to database {store_database:?} (subject {subject:?}); open the store of the database you mean")] + ScopeMismatch { tree_database: String, store_database: String, subject: String, tree: String }, + /// The tree holds no `measurements.db` two levels down under `subject`, so it has no + /// series of that subject's to read. + #[error("{} holds no series of subject {subject:?}: no `{MEASUREMENTS_FILE}` at {subject}//", .tree.display())] + NoSubject { tree: PathBuf, subject: String }, + /// The tree root has no final directory name to take as the database name — the store's + /// scope needs one, and 0.1.x named its database by that directory. + #[error("{} has no directory name to read as the database name", .root.display())] + NoDatabaseName { root: PathBuf }, +} + +/// One aspect's series inside a rows-mode tree, as [`discover`] found it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LegacyAspect { + /// The tree's first level: the 0.1.x subject directory. + pub subject: String, + /// The tree's second level, which is the aspect's name — 0.1.x kept no aspect name + /// inside `measurements.db` (its rows carry the transaction id and the dataset id), so + /// the directory is the only record of what the series is called. + pub name: String, + /// The `measurements.db` itself. + pub database: PathBuf, + /// Rows in `measurements`. + pub rows: u64, + /// The earliest timestamp, in milliseconds, or [`None`] for an empty series. + pub first_timestamp: Option, + /// The latest timestamp, in milliseconds, or [`None`] for an empty series. + pub last_timestamp: Option, + /// Rows whose timestamp another row of the same series already has — `rows` less the + /// distinct timestamps. 0.1.x wrote with no unique constraint on `timestamp` and + /// deduplicated at the application level, so a tree can hold more than one row per + /// instant; the readable leg carries them as they are, and says how many. + pub duplicate_timestamps: u64, +} + +/// A path in the tree this module will not carry, with the reason named. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NotCarried { + /// Tree-relative, with `/` separators. + pub path: String, + /// Why it stays behind, in the words of the subsystem that owned it. + #[serde(rename = "not_carried_because")] + pub reason: String, +} + +/// What [`discover`] read of a rows-mode tree. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LegacyTree { + /// The root, as given. + pub root: PathBuf, + /// The database name: 0.1.x named a database by its top directory, and that is the + /// name the store's scope takes. Same-named subjects of two databases stay apart + /// because they land in stores of two databases. + pub database: String, + /// The series the readable leg can read, ordered by `/`. + pub aspects: Vec, + /// Everything in the tree it will not carry, ordered by path. + pub not_carried: Vec, +} + +impl LegacyTree { + /// The tree's subjects, in the order their series appear. + #[must_use] + pub fn subjects(&self) -> Vec { + let mut seen: Vec = Vec::new(); + for aspect in &self.aspects { + if !seen.contains(&aspect.subject) { + seen.push(aspect.subject.clone()); + } + } + seen + } + + /// The series of `subject`, in order. + #[must_use] + pub fn for_subject(&self, subject: &str) -> Vec<&LegacyAspect> { + self.aspects.iter().filter(|aspect| aspect.subject == subject).collect() + } +} + +/// What an import did for one aspect: the counts, the encoding it chose, and the span it +/// wrote. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImportedAspect { + /// The aspect's name, which is the tree's directory name. + pub aspect: String, + /// Rows read from the tree and sealed into the store. + pub rows: u64, + /// Segments sealed for it, each `rows_per_segment` rows or fewer. + pub segments: u64, + /// The encoding its values were declared in — the narrowest that held them exactly. + pub encoding: String, + /// Its first and last timestamp, in milliseconds; [`None`] only for an empty series. + pub first_timestamp: Option, + pub last_timestamp: Option, + /// Rows sharing a timestamp with another row, carried as they are. + pub duplicate_timestamps: u64, +} + +/// The record an import leaves in the store's `store_meta`, naming the tree it read, what +/// it wrote, and what it left behind. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImportRecord { + /// The tree, as it was addressed. + pub source: String, + /// The store's scope, so the record says which side of a two-database tree it belongs to. + pub database: String, + pub subject: String, + /// The unit every migrated timestamp is declared in ([`TIMESTAMP_UNIT`]). + pub timestamp_unit: String, + /// Rows per sealed segment. + pub rows_per_segment: usize, + /// What was read, per aspect. + pub aspects: Vec, + /// What stayed in the tree, with reasons. + pub not_carried: Vec, + /// When the import finished, RFC 3339 UTC. + pub imported_at: String, +} + +impl ImportRecord { + /// Rows the import sealed, across every aspect. + #[must_use] + pub fn rows(&self) -> u64 { + self.aspects.iter().map(|aspect| aspect.rows).sum() + } + + /// Segments the import sealed, across every aspect. + #[must_use] + pub fn segments(&self) -> u64 { + self.aspects.iter().map(|aspect| aspect.segments).sum() + } +} + +/// Reads the tree at `tree` and reports it, without touching a store. +/// +/// The root must be a 0.1.x database directory: `metadata.db` at the top, and one +/// database per aspect at `//measurements.db`. Each of those is opened +/// and asked for the series the readable leg needs, so a file that is not what 0.1.x +/// wrote is refused here, before anything is sealed. +/// +/// # Errors +/// +/// [`NoCatalog`](RowsModeError::NoCatalog) for a root with no `metadata.db`, +/// [`NoDatabaseName`](RowsModeError::NoDatabaseName) for a root with no directory name to +/// read as the database, [`NoSeries`](RowsModeError::NoSeries) or +/// [`RowShape`](RowsModeError::RowShape) for a database that is not the series layout, and +/// an I/O failure for a directory that cannot be read. +pub async fn discover(tree: &Path) -> Result { + let root = tree.to_path_buf(); + if !tokio::fs::try_exists(&root).await.with_context(|| format!("looking at the tree root {}", root.display()))? { + bail!("there is no rows-mode tree at {}", root.display()); + } + if !tokio::fs::metadata(&root).await.with_context(|| format!("looking at the tree root {}", root.display()))?.is_dir() { + bail!("{} is a file, not a rows-mode tree", root.display()); + } + if !tokio::fs::try_exists(root.join("metadata.db")).await.with_context(|| format!("looking for the 0.1.x catalog in {}", root.display()))? { + return Err(anyhow::Error::new(RowsModeError::NoCatalog { root })); + } + let database = root.file_name().and_then(|name| name.to_str()).map(str::to_string).ok_or_else(|| anyhow::Error::new(RowsModeError::NoDatabaseName { root: root.clone() }))?; + + let mut aspects = Vec::new(); + let mut not_carried = vec![NotCarried { path: "metadata.db".to_string(), reason: "the 0.1.x catalog: its subjects, its aspects and its database row. The store declares its own catalog, and the readable leg takes the aspect's name from its directory.".to_string() }]; + // Symlinks are passed over, not followed, as in the open's refusal probe: following one + // would read somebody else's tree into a store the operator scoped to this one. + for subject in directories(&root).await? { + let subject_name = subject.file_name().to_string_lossy().into_owned(); + for sibling in files(&subject.path()).await? { + let name = sibling.file_name().to_string_lossy().into_owned(); + if is_sidecar(&name) || !name.ends_with(".db") { + continue; + } + not_carried.push(NotCarried { path: format!("{subject_name}/{name}"), reason: SUBJECT_LEVEL.to_string() }); + } + for aspect in directories(&subject.path()).await? { + let aspect_name = aspect.file_name().to_string_lossy().into_owned(); + let database_path = aspect.path().join(MEASUREMENTS_FILE); + if !tokio::fs::try_exists(&database_path).await.with_context(|| format!("looking for {MEASUREMENTS_FILE} in {}", aspect.path().display()))? { + not_carried.push(NotCarried { path: format!("{subject_name}/{aspect_name}"), reason: if aspect_name == "dictionaries" { DICTIONARIES.to_string() } else { not_a_series(&aspect.path()).await } }); + continue; + } + for sibling in files(&aspect.path()).await? { + let name = sibling.file_name().to_string_lossy().into_owned(); + if is_sidecar(&name) || !name.ends_with(".db") || name == MEASUREMENTS_FILE { + continue; + } + not_carried.push(NotCarried { path: format!("{subject_name}/{aspect_name}/{name}"), reason: ASPECT_SIBLING.to_string() }); + } + for nested in directories(&aspect.path()).await? { + let nested_name = nested.file_name().to_string_lossy().into_owned(); + not_carried.push(NotCarried { path: format!("{subject_name}/{aspect_name}/{nested_name}"), reason: if nested_name == "dictionaries" { DICTIONARIES.to_string() } else { ASPECT_SIBLING.to_string() } }); + } + aspects.push(survey(&subject_name, &aspect_name, &database_path).await?); + } + } + aspects.sort_by(|one, other| (one.subject.as_str(), one.name.as_str()).cmp(&(other.subject.as_str(), other.name.as_str()))); + not_carried.sort_by(|one, other| one.path.cmp(&other.path)); + Ok(LegacyTree { root, database, aspects, not_carried }) +} + +/// The reason a database beside the rows stays behind. +const ASPECT_SIBLING: &str = "0.1.x kept its ingestion, pipeline, correlation and pattern state beside the rows (`compat/v010/README.md` records these files per aspect); the readable leg reads only `measurements.db`, and 1.0 leaves those subsystems behind."; + +/// The reason a database at the subject level stays behind. +const SUBJECT_LEVEL: &str = "it sits where the layout puts no aspect's series: 0.1.x kept one database per aspect at `//measurements.db`."; + +/// The reason an aspect's `dictionaries/` stays behind. +const DICTIONARIES: &str = "0.1.x's compression dictionaries for the aspect beside them; the segment store's encodings are declared per aspect, not learned and stored beside the data."; + +/// Why a directory with no `measurements.db` in it is not a series. +async fn not_a_series(dir: &Path) -> String { + let holds_databases = files(dir).await.unwrap_or_default().iter().any(|entry| entry.file_name().to_string_lossy().ends_with(".db")); + if holds_databases { + format!("it holds `*.db` files but no {MEASUREMENTS_FILE}, so the readable leg finds no series of ours in it") + } else { + format!("it holds no {MEASUREMENTS_FILE}, so it is not a series directory") + } +} + +/// Whether `name` is one of a libSQL database's sidecars, which travel with the database +/// they belong to and are never listed on their own. +#[must_use] +fn is_sidecar(name: &str) -> bool { + ["-wal", "-shm", "-log", ".wal", ".shm"].iter().any(|suffix| name.ends_with(suffix)) +} + +/// The child entries of `dir`, sorted by file name. +async fn entries(dir: &Path) -> Result> { + let mut reader = tokio::fs::read_dir(dir).await.with_context(|| format!("reading the 0.1.x directory {}", dir.display()))?; + let mut found = Vec::new(); + while let Some(entry) = reader.next_entry().await.with_context(|| format!("reading the 0.1.x directory {}", dir.display()))? { + found.push(entry); + } + found.sort_by_key(|entry| entry.file_name()); + Ok(found) +} + +/// The child **directories** of `dir`. A symlink is passed over, not followed: following one +/// would read a tree the operator did not scope this store to. +async fn directories(dir: &Path) -> Result> { + let mut kept = Vec::new(); + for entry in entries(dir).await? { + if !entry.file_type().await.is_ok_and(|kind| kind.is_dir()) { + continue; + } + kept.push(entry); + } + Ok(kept) +} + +/// The child **files** of `dir`, sidecars included: the callers ask whether a name is one. +async fn files(dir: &Path) -> Result> { + let mut kept = Vec::new(); + for entry in entries(dir).await? { + if !entry.file_type().await.is_ok_and(|kind| !kind.is_dir()) { + continue; + } + kept.push(entry); + } + Ok(kept) +} + +/// Reports one aspect's series, by reading the aggregates the tree itself holds. +/// +/// This is the cheap half: it asks the database for its row count and span, and does not +/// carry its values. [`import_into`] reads those when it writes. +async fn survey(subject: &str, name: &str, database: &Path) -> Result { + let conn = open(database).await?; + let columns = columns_of(&conn, database).await?; + pick_value_column(database, &columns)?; + #[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)] + let total = u64::try_from(scalar(&conn, database, "SELECT count(*) FROM measurements").await?).unwrap_or(u64::MAX); + let mut rows = conn.query("SELECT min(timestamp), max(timestamp) FROM measurements", ()).await.with_context(|| format!("asking {} for the span of its series", database.display()))?; + let row = rows.next().await?.with_context(|| format!("{} answers the span query with no row", database.display()))?; + let first = match row.get_value(0)? { + Value::Integer(when) => Some(when), + Value::Null => None, + other => return Err(anyhow::Error::new(RowsModeError::RowShape { path: database.to_path_buf(), rowid: 0, detail: format!("`min(timestamp)` is {other:?}, and 0.1.x wrote `timestamp INTEGER NOT NULL`") })), + }; + let last = match row.get_value(1)? { + Value::Integer(when) => Some(when), + Value::Null => None, + other => return Err(anyhow::Error::new(RowsModeError::RowShape { path: database.to_path_buf(), rowid: 0, detail: format!("`max(timestamp)` is {other:?}, and 0.1.x wrote `timestamp INTEGER NOT NULL`") })), + }; + #[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)] + let distinct = u64::try_from(scalar(&conn, database, "SELECT count(DISTINCT timestamp) FROM measurements").await?).unwrap_or(0); + Ok(LegacyAspect { subject: subject.to_string(), name: name.to_string(), database: database.to_path_buf(), rows: total, first_timestamp: first, last_timestamp: last, duplicate_timestamps: total.saturating_sub(distinct) }) +} + +/// Opens one `measurements.db` of the tree, to read it. +/// +/// Deliberately bare: the crate's control-plane databases open through +/// [`connect`](crate::types::durable::control_plane::connect) and `enable_mvcc_full`, and +/// both **write** to the database they open. The readable leg reads a tree it does not own, +/// and 14a's whole promise is that nothing in it changes; its writes go to the store. +async fn open(database: &Path) -> Result { + // `Builder::new_local` takes a `&str`; a lossy conversion is fine here because the path + // came off the disk, and a name that is not valid UTF-8 still reads the same file. + let db = Builder::new_local(&database.to_string_lossy()).build().await.with_context(|| format!("opening the 0.1.x series database {}", database.display()))?; + Ok(db.connect().with_context(|| format!("connecting to the 0.1.x series database {}", database.display()))?) +} + +/// The table every rows-mode database keeps its series in. +const LEGACY_TABLE: &str = "measurements"; + +/// The columns of `measurements`, each with the type its table declares it as. +/// +/// `table_info` answers this whether or not the table holds a row, and it is also the +/// question "is this table here at all": no rows means the table is not there, which is +/// the refusal [`NoSeries`](RowsModeError::NoSeries) makes. +async fn columns_of(conn: &turso::Connection, database: &Path) -> Result> { + let mut rows = conn.query(format!("PRAGMA table_info({LEGACY_TABLE})"), ()).await.with_context(|| format!("asking {} which tables it has", database.display()))?; + let mut columns = Vec::new(); + while let Some(row) = rows.next().await? { + let name = match row.get_value(1)? { + Value::Text(name) => name.to_string(), + other => bail!("{} names a column of {} as {other:?}", database.display(), LEGACY_TABLE), + }; + let declared = match row.get_value(2)? { + Value::Text(declared) => declared.to_string(), + Value::Null => String::new(), + other => bail!("{} declares column {name:?} of {} as {other:?}", database.display(), LEGACY_TABLE), + }; + columns.push((name, declared)); + } + if columns.is_empty() { + return Err(anyhow::Error::new(RowsModeError::NoSeries { path: database.to_path_buf(), detail: format!("it has no `{LEGACY_TABLE}` table") })); + } + Ok(columns) +} + +/// The column the aspect's value sits in: `value`, and only `value`. +/// +/// 0.1.x's own server created `measurements (id TEXT NOT NULL, dataset_id TEXT NOT NULL, +/// timestamp INTEGER NOT NULL, value TEXT NOT NULL)` (`v0.1.0:weftdb/src/types/aspect.rs:475`, +/// repeated at `:1195`) and every writer it has names that column in its `INSERT` +/// (`v0.1.0:weftdb/src/types/database/inputs.rs:283`, `:373`); the column list in the fixture's +/// page images matches, and so does its `value TEXT`. An earlier draft of this reader also +/// accepted a column named after the **aspect**, on a theory about a migration rename that does +/// not appear anywhere in 0.1.x's source — and it was worse than unattested. An aspect named +/// `id`, `dataset_id` or `timestamp` would have been read out of the column holding its +/// identifiers, and every row imported from it would have been an identifier posing as a +/// measurement, with nothing afterwards to say so. A column 0.1.x did not write is refused, +/// naming the columns that are there. +fn pick_value_column(database: &Path, columns: &[(String, String)]) -> Result { + if columns.is_empty() { + return Err(anyhow::Error::new(RowsModeError::NoSeries { path: database.to_path_buf(), detail: "it has no `measurements` table to describe".to_string() })); + } + if !columns.iter().any(|(column, _)| column == TIMESTAMP_COLUMN) { + return Err(anyhow::Error::new(RowsModeError::NoSeries { path: database.to_path_buf(), detail: format!("it has no `timestamp` column, which is the column 0.1.x stamped its instants into: its columns are {}", listed(columns)) })); + } + if !columns.iter().any(|(column, _)| column == "value") { + return Err(anyhow::Error::new(RowsModeError::NoSeries { path: database.to_path_buf(), detail: format!("it has no `value` column, which is the column 0.1.x wrote its measurements into: its columns are {}", listed(columns)) })); + } + Ok("value".to_string()) +} + +/// Names a list of columns in one sentence. +fn listed(columns: &[(String, String)]) -> String { + columns.iter().map(|(column, _)| format!("{column:?}")).collect::>().join(", ") +} + +/// One integer the database answers with, where zero rows or `NULL` means zero. +async fn scalar(conn: &turso::Connection, database: &Path, sql: &str) -> Result { + let mut rows = conn.query(sql, ()).await.with_context(|| format!("asking {} for `{sql}`", database.display()))?; + let Some(row) = rows.next().await? else { return Ok(0) }; + match row.get_value(0)? { + Value::Integer(number) => Ok(number), + Value::Null => Ok(0), + other => bail!("{} answers `{sql}` with {other:?}, not a number", database.display()), + } +} + +/// Reads one aspect's rows, oldest first, as numbers the store can hold. +/// +/// 0.1.x wrote no ordering guarantee and no unique constraint, so the read asks for one: +/// `ORDER BY timestamp`. A tie keeps the database's own row order, which is arrival order, +/// and the store sorts a seal anyway — the readable leg's read is stable rather than lucky. +async fn read_rows(conn: &turso::Connection, database: &Path, value_column: &str) -> Result> { + let sql = format!("SELECT {TIMESTAMP_COLUMN}, {} FROM {LEGACY_TABLE} ORDER BY {TIMESTAMP_COLUMN}", quote(value_column)); + let mut rows = conn.query(sql, ()).await.with_context(|| format!("reading the rows of {}", database.display()))?; + let mut series = Vec::new(); + let mut position = 0i64; + while let Some(row) = rows.next().await? { + position += 1; + let when = match row.get_value(0)? { + Value::Integer(when) => when, + other => { + return Err(anyhow::Error::new(RowsModeError::RowShape { path: database.to_path_buf(), rowid: position, detail: format!("`{TIMESTAMP_COLUMN}` holds {other:?}, and 0.1.x wrote `timestamp INTEGER NOT NULL`") })); + } + }; + let value = match row.get_value(1)? { + Value::Text(text) => { + let shown = format!("{text}"); + BigDecimal::from_str(&shown).map_err(|error| RowsModeError::ValueNotDecimal { path: database.to_path_buf(), rowid: position, found: format!("{shown:?} ({error})") })? + } + // 0.1.x wrote its `BigDecimal` as the text of it, so a numeric or NULL value cell + // is not a row 0.1.x wrote. Carrying the number anyway would put a value in the + // store whose exactness nobody can vouch for. + other => { + return Err(anyhow::Error::new(RowsModeError::ValueNotDecimal { path: database.to_path_buf(), rowid: position, found: format!("{other:?}") })); + } + }; + series.push((when, value)); + } + Ok(series) +} + +/// Wraps a column name in double quotes, doubling any inside it. The column names this module +/// interpolates come from its own constants, but a name that came from a database would need +/// this before it went into a statement. +fn quote(name: &str) -> String { + format!("\"{}\"", name.replace('"', "\"\"")) +} + +/// How a schema reads in a refusal sentence: the encoding, its tolerance, and the unit. +fn schema_text(schema: &AspectSchema) -> String { + format!("{} (tolerance {}, timestamps in {})", encoding_name(schema.value), schema.value_tolerance, schema.timestamp_unit.name()) +} + +/// Names an encoding the way a record and a refusal should: `scaled_i64` for the plain +/// ones, `scaled_i64:4` where the encoding carries a scale. +fn encoding_name(physical_type: PhysicalType) -> String { + match physical_type { + PhysicalType::ScaledI64 { scale } | PhysicalType::ScaledI128 { scale } => format!("{}:{scale}", physical_type.name()), + other => other.name().to_string(), + } +} + +/// The `store_meta` key the import record of one scope sits under. +/// +/// It names the scope because one store root can hold scoped stores of several subjects and +/// databases, and a record has to say which import wrote which of them. +#[must_use] +pub fn record_key(database: &str, subject: &str) -> String { + format!("{IMPORT_RECORD_KEY}:{database}:{subject}") +} + +/// The import record of `store`'s own scope, if an import wrote one. +/// +/// # Errors +/// +/// Propagates a read failure of the store's index, and reports a record that is not the +/// JSON this module writes as an error naming the key. +pub async fn read_import_record(store: &SegmentStore) -> Result> { + let key = record_key(store.database(), store.subject()); + let Some(text) = store.index().meta(&key).await.with_context(|| format!("reading {key} from the store's own index"))? else { + return Ok(None); + }; + Ok(Some(serde_json::from_str(&text).with_context(|| format!("{} names {key}, and it is not the import record this module wrote", store.root().display()))?)) +} + +/// Writes `record` into the store's own index, under the key of the scope it describes. +async fn write_import_record(store: &SegmentStore, record: &ImportRecord) -> Result<()> { + let key = record_key(&record.database, &record.subject); + let value = serde_json::to_string(record).with_context(|| format!("rendering the import record of {key}"))?; + store.index().apply(&IndexTxn::new(vec![IndexOp::MetaSet { key: key.clone(), value }])).await.with_context(|| format!("recording {key} in the store's own index"))?; + Ok(()) +} + +/// Reads the rows-mode tree at `tree` into `store`, and records what it did. +/// +/// The tree is read and never changed; the store takes the rows. Each aspect is declared +/// under the narrowest encoding that holds its values exactly (tolerance zero), then sealed +/// in batches of `rows_per_segment`, oldest first. When the last batch is in, the import's +/// own record goes into the store's index, naming the tree, the per-aspect counts, the +/// encodings, and what stayed in the tree. +/// +/// The guard against a second reading of the same tree is that record: a migrated row and a +/// row the store sealed before look the same to it afterwards, so an import that found a +/// record for its own scope refuses rather than doubling the rows. +/// +/// # Errors +/// +/// Whatever [`discover`] refuses, plus [`ScopeMismatch`](RowsModeError::ScopeMismatch) when +/// the tree's database directory and the store's scope disagree, +/// [`NoSubject`](RowsModeError::NoSubject) when the tree holds no series of `subject`, +/// [`AlreadyImported`](RowsModeError::AlreadyImported) when this scope already has a record, +/// [`AspectNotEmpty`](RowsModeError::AspectNotEmpty) when the aspect already holds segments +/// this reading did not seal, [`SchemaConflict`](RowsModeError::SchemaConflict) when it is +/// declared under a schema the tree's values do not fit, +/// [`RowShape`](RowsModeError::RowShape) and +/// [`ValueNotDecimal`](RowsModeError::ValueNotDecimal) for a row that is not what 0.1.x +/// wrote, and any seal or index failure of the store itself. +/// +/// A failure partway leaves the segments it already sealed, and no record: the next attempt +/// refuses on [`AspectNotEmpty`](RowsModeError::AspectNotEmpty), naming the aspect it +/// stopped on, rather than writing the tree's rows a second time. Freeing them is FRE-01's +/// rollback boundary, which is why no half-write is cleaned up here. +pub async fn import_into(store: &SegmentStore, tree: &Path, subject: &str, rows_per_segment: NonZeroUsize) -> Result { + let rows_per_segment = rows_per_segment.get(); + if store.subject() != subject { + bail!("this store is scoped to subject {:?}, and the readable leg reads one subject at a time; open a store of the subject you mean", store.subject()); + } + let legacy = discover(tree).await?; + if legacy.database != store.database() { + return Err(anyhow::Error::new(RowsModeError::ScopeMismatch { tree_database: legacy.database.clone(), store_database: store.database().to_string(), subject: subject.to_string(), tree: tree.display().to_string() })); + } + aspect_name::validate(subject).with_context(|| format!("the tree's subject directory {:?} cannot name a subject in a store", legacy.database))?; + let series = legacy.for_subject(subject); + if series.is_empty() { + return Err(anyhow::Error::new(RowsModeError::NoSubject { tree: tree.to_path_buf(), subject: subject.to_string() })); + } + if let Some(existing) = read_import_record(store).await? { + return Err(anyhow::Error::new(RowsModeError::AlreadyImported { tree: existing.source.clone(), imported_at: existing.imported_at.clone(), aspects: existing.aspects.len(), rows: existing.rows() })); + } + + let mut imported = Vec::new(); + for found in series { + let aspect = found.name.as_str(); + aspect_name::validate(aspect).with_context(|| format!("the tree's aspect directory {:?} cannot name an aspect in a store", found.name))?; + let conn = open(&found.database).await?; + let columns = columns_of(&conn, &found.database).await?; + let value_column = pick_value_column(&found.database, &columns)?; + let rows = read_rows(&conn, &found.database, &value_column).await?; + + // The narrowest encoding that holds the tree's values of this aspect exactly, at + // tolerance zero: the readable leg trades no precision across the version boundary, + // so the choice is the store's own recommendation rather than a schema carried over + // from 0.1.x, which kept its values as text. + let chosen = recommend_encoding(&rows.iter().map(|(_, value)| value.clone()).collect::>(), &BigDecimal::from(0)); + let wanted = AspectSchema { value: chosen.physical_type, value_tolerance: BigDecimal::from(0), timestamp_unit: TIMESTAMP_TIME_UNIT }; + // The aspect's own state in the store decides, before anything is written: rows + // already sealed under it stop the reading (a migrated row and a row the store + // sealed before are indistinguishable afterwards), and a schema already declared + // has to be the one the tree's values need. + let segments = store.aspect_stats(aspect).await.with_context(|| format!("asking {} what it holds of {aspect:?}", store.root().display()))?.segment_count; + if segments > 0 { + return Err(anyhow::Error::new(RowsModeError::AspectNotEmpty { aspect: aspect.to_string(), segments })); + } + if let Some(declared) = store.schema_for(aspect).await? { + if declared != wanted { + return Err(anyhow::Error::new(RowsModeError::SchemaConflict { aspect: aspect.to_string(), declared: schema_text(&declared), wanted: schema_text(&wanted) })); + } + } else { + store.declare(aspect, &wanted).await.with_context(|| format!("declaring {aspect:?} in {}", store.root().display()))?; + } + + let mut sealed = 0u64; + for chunk in chunks_keeping_instants_together(&rows, rows_per_segment) { + let timestamps: Vec = chunk.iter().map(|(when, _)| *when).collect(); + let chunk_values: Vec = chunk.iter().map(|(_, value)| value.clone()).collect(); + store.seal_declared(aspect, ×tamps, &chunk_values).await.with_context(|| format!("sealing {} row(s) of {aspect:?} into {}", timestamps.len(), store.root().display()))?; + sealed += 1; + } + imported.push(ImportedAspect { aspect: aspect.to_string(), rows: u64::try_from(rows.len()).unwrap_or(u64::MAX), segments: sealed, encoding: encoding_name(chosen.physical_type), first_timestamp: rows.first().map(|(when, _)| *when), last_timestamp: rows.last().map(|(when, _)| *when), duplicate_timestamps: found.duplicate_timestamps }); + } + + // The record speaks for this scope: it lists what stayed in the tree under this + // subject's directory, and the tree-level files, and not another subject's series. + let not_carried = legacy.not_carried.iter().filter(|entry| entry.path.split_once('/').map_or(true, |(head, _)| head == subject)).cloned().collect(); + let record = ImportRecord { source: tree.display().to_string(), database: legacy.database.clone(), subject: subject.to_string(), timestamp_unit: TIMESTAMP_UNIT.to_string(), rows_per_segment, aspects: imported, not_carried, imported_at: Utc::now().to_rfc3339() }; + write_import_record(store, &record).await?; + Ok(record) +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + /// Writes one `measurements.db` the way 0.1.x wrote it: the table its own server + /// created, the instant as an integer, and the value as the text of a `BigDecimal`. + async fn write_series(database: &Path, value_column: &str, rows: &[(i64, &str)]) { + if let Some(parent) = database.parent() { + tokio::fs::create_dir_all(parent).await.expect("the tree's directories are there"); + } + let db = Builder::new_local(&database.to_string_lossy()).build().await.expect("a local database of the 0.1.x shape"); + let conn = db.connect().expect("a connection to it"); + // The table 0.1.x's server ran, column for column + // (`v0.1.0:weftdb/src/types/aspect.rs:475`), identifiers included. A caller that wants a + // shape 0.1.x did *not* write passes another value column, which is what the refusal test + // asks for. + conn.execute("CREATE TABLE measurements (id TEXT NOT NULL, dataset_id TEXT NOT NULL, timestamp INTEGER NOT NULL, value TEXT NOT NULL)", ()).await.expect("0.1.x created its table with this DDL"); + if value_column != "value" { + conn.execute(format!("ALTER TABLE measurements RENAME COLUMN value TO {}", quote(value_column)), ()).await.expect("a column name 0.1.x did not use"); + } + // The two identifiers are written because the DDL will not accept a row without them: + // 0.1.x put a transaction id and its database id in every row. The readable leg reads + // neither — it takes the aspect's name from the directory and the instant and value from + // the columns — so the values here only have to satisfy the table the release created. + for (position, (when, value)) in rows.iter().enumerate() { + conn.execute(format!("INSERT INTO measurements (id, dataset_id, timestamp, {}) VALUES (?1, ?2, ?3, ?4)", quote(value_column)), turso::params![format!("legacy-tx-{position}"), "legacy-dataset", when, value]).await.expect("a row goes in"); + } + // The whole story on disk before the readable leg looks: a checkpointed database is + // what an operator would hand over, WAL sidecars and all. `wal_checkpoint` answers with + // a row, so it goes through `query` and the row is drained — `execute` refuses a + // statement that returns results. + let mut checkpoint = conn.query("PRAGMA wal_checkpoint(TRUNCATE)", ()).await.expect("the rows are in the database file"); + while checkpoint.next().await.expect("the checkpoint answers once").is_some() {} + drop(conn); + } + + /// The catalog that makes a directory a 0.1.x database at all. The readable leg never opens + /// it — it takes the aspect's name from the directory beside it — so the tests leave a + /// stand-in, and discovery only asks whether it is there. + async fn write_catalog(root: &Path) { + tokio::fs::create_dir_all(root).await.expect("the tree root is there"); + tokio::fs::write(root.join("metadata.db"), b"the 0.1.x catalog, as a stand-in").await.expect("a catalog"); + } + + /// The tree most of these tests read: database `plant`, two subjects, one aspect whose + /// value column named `value`, one subject-level database of 0.1.x's own state, and + /// a dictionary directory beside the series. + async fn build_tree(root: &Path) { + write_catalog(root).await; + write_series(&root.join("pump-01/temperature/measurements.db"), "value", &[(1767225600000, "100.2500"), (1767225601000, "101.0000"), (1767225602000, "99.5000")]).await; + write_series(&root.join("pump-01/vibration/measurements.db"), "value", &[(1767225600000, "0.1250"), (1767225601000, "0.2500")]).await; + write_series(&root.join("pump-02/temperature/measurements.db"), "value", &[(1767225600000, "-3.5000")]).await; + tokio::fs::write(root.join("pump-01/ingestion.db"), b"0.1.x kept its ingestion state beside the rows").await.expect("a subject-level database"); + tokio::fs::create_dir_all(root.join("pump-02/dictionaries")).await.expect("a dictionary directory"); + } + + /// A store scoped to `database`/`subject`, beside the tree rather than in it. + async fn store_at(root: &Path, database: &str, subject: &str) -> SegmentStore { + SegmentStore::open_scoped(root, database, subject).await.expect("a scoped store opens") + } + + /// Every file under `root`, by tree-relative path, with its bytes. + async fn tree_state(root: &Path) -> BTreeMap> { + let mut files = BTreeMap::new(); + let mut pending = vec![root.to_path_buf()]; + while let Some(dir) = pending.pop() { + let mut entries = tokio::fs::read_dir(&dir).await.expect("the tree can be read"); + while let Some(entry) = entries.next_entry().await.expect("a directory entry") { + let path = entry.path(); + if path.is_dir() { + pending.push(path); + continue; + } + let name = path.strip_prefix(root).expect("a tree-relative path").display().to_string(); + files.insert(name, tokio::fs::read(&path).await.expect("a file's bytes")); + } + } + files + } + + /// The release's DDL calls the value column `value` whatever the aspect is called + /// (`v0.1.0:weftdb/src/types/aspect.rs:475-480`), so a database that names the column after + /// the aspect is a shape this build has no evidence about. Reading "whichever column shares + /// the aspect's name" was an earlier draft of this reader, and guessed the wrong way it reads + /// 0.1.x's own identifiers back as measurements — which is the hazard the positive test below + /// pins down. + #[tokio::test] + async fn a_series_database_that_is_not_the_shape_0_1_x_wrote_is_refused() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + write_catalog(&root).await; + write_series(&root.join("pump-01/flow/measurements.db"), "flow", &[(1767225600000, "1.5000")]).await; + let error = discover(&root).await.expect_err("a series database whose value column is not the one 0.1.x created is a shape this build does not guess at"); + assert!(matches!(error.downcast_ref::(), Some(RowsModeError::NoSeries { .. })), "{error}"); + let detail = error.to_string(); + assert!(detail.contains("it has no `value` column"), "the refusal says which column it was looking for: {detail}"); + assert!(detail.contains("\"flow\""), "and it names the columns the database does have, so the operator can see what they handed over: {detail}"); + assert!(!detail.contains("value cell"), "it refused the shape before reading anything out of it: {detail}"); + } + + /// The hazard above stated positively, because refusing is only half of it: `dataset_id` is a + /// legal 0.1.x aspect name *and* the name of the column in the release's DDL that holds 0.1.x's + /// own dataset identifier. An aspect with that name is a legal series — its name comes from the + /// directory, its values from `value` — and the identifier column is never a measurement. + #[tokio::test] + async fn an_aspect_named_after_an_identifier_column_is_still_read_from_value() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + write_catalog(&root).await; + write_series(&root.join("pump-01/dataset_id/measurements.db"), "value", &[(1767225600000, "1.5000"), (1767225601000, "2.5000")]).await; + let tree = discover(&root).await.expect("an aspect called dataset_id is a name 0.1.x allowed, and the directory is what names the series"); + assert_eq!(tree.aspects.iter().map(|aspect| (aspect.subject.as_str(), aspect.name.as_str(), aspect.rows)).collect::>(), [("pump-01", "dataset_id", 2)], "the aspect keeps its directory's name even where it collides with a column"); + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let record = import_into(&store, &root, "pump-01", NonZeroUsize::new(8).unwrap()).await.expect("the series reads in"); + assert_eq!((record.rows(), record.aspects.len()), (2, 1), "nothing of the identifier column crossed"); + let (timestamps, values) = store.read_time_range("dataset_id", i64::MIN, i64::MAX).await.expect("the aspect reads back under its directory name"); + assert_eq!(timestamps, [1767225600000, 1767225601000], "and it reads back under the name the operator will type"); + let want: Vec = ["1.5000", "2.5000"].map(|text| BigDecimal::from_str(text).expect("a decimal the tree would hold")).to_vec(); + assert_eq!(values.iter().flatten().cloned().collect::>(), want, "the values come from `value`, not from the column that shares the aspect's name"); + } + + /// What the readable leg sees before it reads anything: the series, by name, and the + /// things it will not carry. + #[tokio::test] + async fn a_rows_mode_tree_reads_as_a_tree_of_series() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let tree = discover(&root).await.expect("a tree of one database and two subjects is a rows-mode tree"); + assert_eq!(tree.database, "plant", "0.1.x named a database by its directory, and the readable leg takes the store's scope from that name"); + assert_eq!(tree.subjects(), ["pump-01", "pump-02"]); + let series: Vec<(&str, &str, u64)> = tree.aspects.iter().map(|aspect| (aspect.subject.as_str(), aspect.name.as_str(), aspect.rows)).collect(); + assert_eq!(series, [("pump-01", "temperature", 3), ("pump-01", "vibration", 2), ("pump-02", "temperature", 1)], "one database per aspect, read from the directory that names it"); + assert_eq!(tree.for_subject("pump-01").iter().map(|aspect| aspect.name.as_str()).collect::>(), ["temperature", "vibration"], "the series of one subject, which is the size of a scoped store's import"); + let temperature = tree.aspects.iter().find(|aspect| aspect.subject == "pump-01").expect("the tree holds pump-01's temperature"); + assert_eq!((temperature.first_timestamp, temperature.last_timestamp), (Some(1767225600000), Some(1767225602000)), "the span is the instants as stored, in milliseconds"); + let reported: Vec<&str> = tree.not_carried.iter().map(|entry| entry.path.as_str()).collect(); + assert_eq!(reported, ["metadata.db", "pump-01/ingestion.db", "pump-02/dictionaries"], "the catalog, 0.1.x's own state beside the rows, and the dictionaries are each named with a reason"); + } + + /// The point of the slice: the rows come across, exactly, and the store can answer for + /// them afterwards. + #[tokio::test] + async fn reading_a_rows_mode_tree_puts_its_rows_in_the_store() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let record = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect("the readable leg reads the tree's pump-01 series"); + assert_eq!(record.database, "plant"); + assert_eq!(record.subject, "pump-01"); + assert_eq!(record.timestamp_unit, "millis", "0.1.x stamped milliseconds into the column, and the declaration says so instead of guessing at the numbers"); + assert_eq!(record.rows_per_segment, 2); + let read: Vec<(&str, u64, u64)> = record.aspects.iter().map(|aspect| (aspect.aspect.as_str(), aspect.rows, aspect.segments)).collect(); + assert_eq!(read, [("temperature", 3, 2), ("vibration", 2, 1)], "three rows in batches of two is two segments, and two rows is one"); + assert_eq!((record.rows(), record.segments()), (5, 3), "the record totals what the store took"); + + let (timestamps, values) = store.read_time_range("temperature", i64::MIN, i64::MAX).await.expect("the aspect reads back"); + assert_eq!(timestamps, [1767225600000, 1767225601000, 1767225602000], "the instants cross the boundary unchanged: no division, and no unit read off how big they look"); + let want: Vec = ["100.2500", "101.0000", "99.5000"].map(|text| BigDecimal::from_str(text).expect("a decimal the tree would hold")).to_vec(); + assert_eq!(values.iter().flatten().cloned().collect::>(), want, "the values are the ones the tree held, to the last digit"); + + // What the other subject holds is not in this store, and never was. + assert_eq!(store.read_time_range("temperature", 1767225600000, 1767225600000).await.expect("a one-instant window").1.iter().flatten().count(), 1, "the window holds pump-01's row and not pump-02's"); + for aspect in ["temperature", "vibration"] { + let declared = store.schema_for(aspect).await.expect("the store answers for its schema").unwrap_or_else(|| panic!("the leg declared {aspect} before sealing it")); + let named = record.aspects.iter().find(|entry| entry.aspect == aspect).expect("the record names each aspect it read"); + assert_eq!(named.encoding, encoding_name(declared.value), "the record names the encoding the store actually holds {aspect} in"); + assert_eq!(declared.timestamp_unit, TIMESTAMP_TIME_UNIT, "the unit is declared, per aspect"); + assert_eq!(declared.value_tolerance, BigDecimal::from(0), "exactness across a version boundary, not tolerance"); + } + } + + /// A second reading of the same tree would double the rows, and a migrated row and a row + /// the store sealed before look the same to it afterwards. The record is what stops it. + #[tokio::test] + async fn a_tree_already_read_refuses_to_be_read_again() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let first = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect("the first reading goes through"); + let second = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the tree was read once already, and the store knows it"); + assert!(matches!(second.downcast_ref::(), Some(RowsModeError::AlreadyImported { aspects: 2, rows: 5, .. })), "{second}"); + assert!(second.to_string().contains(&first.source), "the refusal names the tree it was asked to read again: {second}"); + assert_eq!(store.read_time_range("temperature", i64::MIN, i64::MAX).await.expect("the aspect still reads").0.len(), 3, "the refusal left the rows it would have doubled where they were"); + } + + /// A directory is only a rows-mode tree if 0.1.x's catalog is in it. Without that the + /// readable leg has no database to name, and an empty or half-copied directory must not + /// be read into a store on a guess. + #[tokio::test] + async fn a_directory_that_is_not_a_rows_mode_tree_is_refused() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + tokio::fs::create_dir_all(root.join("pump-01/temperature")).await.expect("the tree's directories are there"); + let missing = discover(&root).await.expect_err("there is no catalog, so this is not a tree yet"); + assert!(matches!(missing.downcast_ref::(), Some(RowsModeError::NoCatalog { .. })), "{missing}"); + + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let nested = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the readable leg does not read an incomplete tree into a store"); + assert!(nested.to_string().contains("metadata.db"), "the refusal says what the tree is missing: {nested}"); + } + + /// The tree's database directory names the store's scope. A two-database tree — which is + /// what 0.1.x left behind when an operator kept more than one — must land on the right + /// side of the boundary, and not on the other one by mistake. + #[tokio::test] + async fn a_tree_of_another_database_refuses_to_be_read_into_this_store() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let store = store_at(&dir.path().join("store"), "greenhouse", "pump-01").await; + let wrong = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the tree is `plant` and this store is `greenhouse`"); + assert!(matches!(wrong.downcast_ref::(), Some(RowsModeError::ScopeMismatch { tree_database: _, store_database: _, .. })), "{wrong}"); + assert!(wrong.to_string().contains("greenhouse") && wrong.to_string().contains("plant"), "the refusal names both sides of the mismatch: {wrong}"); + assert_eq!(store.list_declared_aspects().await.expect("the store answers for its declared aspects").len(), 0, "the refusal declared nothing"); + } + + /// A tree read into a store that already holds the aspect would double its rows, and the + /// store cannot tell the two apart afterwards. The aspect's own state stops it. + #[tokio::test] + async fn a_store_that_already_holds_the_aspect_keeps_the_rows_it_has() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let schema = AspectSchema { value: PhysicalType::ScaledI64 { scale: 4 }, value_tolerance: BigDecimal::from(0), timestamp_unit: TIMESTAMP_TIME_UNIT }; + store.declare("temperature", &schema).await.expect("the aspect is declared before the store takes a row of its own"); + store.seal_declared("temperature", &[1767225699000], &[BigDecimal::from_str("7.0000").expect("a decimal a 0.1.x tree would hold")]).await.expect("a row the store wrote itself"); + let clashing = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the store holds temperature already, and reading the tree again would add to it"); + assert!(matches!(clashing.downcast_ref::(), Some(RowsModeError::AspectNotEmpty { aspect, segments: 1 }) if aspect == "temperature"), "{clashing}"); + let (timestamps, _) = store.read_time_range("temperature", i64::MIN, i64::MAX).await.expect("the aspect reads back"); + assert_eq!(timestamps, [1767225699000], "the row the store wrote is the only one it holds: the tree's rows were not added to it"); + assert!(read_import_record(&store).await.expect("the index answers for its own keys").is_none(), "a reading that stopped wrote no record for a later run to trust"); + } + + /// The same guard from the store's other side: an aspect that is declared but holds nothing. + /// This is what an operator who set the aspect up for live data walks into, and the field + /// that matters is the timestamp unit — 0.1.x's rows-mode path stamped millis where its batch + /// path kept seconds, so a hand-declared aspect is plausible at either scale. Reading past + /// the declaration would file every row of the tree a thousand-fold away from the instants the + /// operator sized their queries around, and the aspect's schema is the only thing in the store + /// that says which scale they meant. + #[tokio::test] + async fn an_aspect_declared_at_another_timestamp_scale_refuses_the_reading() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + write_catalog(&root).await; + write_series(&root.join("pump-01/temperature/measurements.db"), "value", &[(1767225600000, "100.2500"), (1767225601000, "101.0000")]).await; + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + // The schema the tree's own rows call for, in every field but the unit: the refusal is + // about the unit and nothing else. + let values: Vec = ["100.2500", "101.0000"].map(|text| BigDecimal::from_str(text).expect("a decimal the tree would hold")).to_vec(); + let chosen = recommend_encoding(&values, &BigDecimal::from(0)); + let seconds = AspectSchema { value: chosen.physical_type, value_tolerance: BigDecimal::from(0), timestamp_unit: TimeUnit::Seconds }; + store.declare("temperature", &seconds).await.expect("an aspect the operator declared for live data, at seconds"); + let clash = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the tree stamps millis and the aspect is declared at seconds"); + assert!(matches!(clash.downcast_ref::(), Some(RowsModeError::SchemaConflict { aspect, .. }) if aspect == "temperature"), "{clash}"); + let detail = clash.to_string(); + assert!(detail.contains("seconds") && detail.contains("millis"), "the refusal prints both schemas, so the operator can see which side to change: {detail}"); + let (timestamps, _) = store.read_time_range("temperature", i64::MIN, i64::MAX).await.expect("an empty aspect answers"); + assert!(timestamps.is_empty(), "and nothing was written at either scale: {timestamps:?}"); + assert!(read_import_record(&store).await.expect("the index answers for its own keys").is_none(), "nor was anything recorded as having crossed"); + } + + /// A value cell that is not the text of a decimal is not a row 0.1.x wrote. Carrying the + /// bytes anyway would put a value in the store whose exactness nobody can vouch for, so + /// the whole read stops and says which row it stopped on — and what it left behind is part + /// of the answer, because the leg writes per aspect and writes its record last. + #[tokio::test] + async fn a_row_that_is_not_a_decimal_stops_the_read_and_names_its_row() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + tokio::fs::create_dir_all(&root).await.expect("the tree root is there"); + tokio::fs::write(root.join("metadata.db"), b"the 0.1.x catalog, as a stand-in").await.expect("a catalog"); + write_series(&root.join("pump-01/temperature/measurements.db"), "value", &[(1767225600000, "100.2500")]).await; + write_series(&root.join("pump-01/vibration/measurements.db"), "value", &[(1767225600000, "bearing-worn")]).await; + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + let broken = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("one of the tree's value cells is not a number"); + assert!(matches!(broken.downcast_ref::(), Some(RowsModeError::ValueNotDecimal { .. })), "the refusal is typed, and it is the value refusals rather than a shape one: {broken}"); + assert!(broken.to_string().contains("bearing-worn"), "the refusal quotes the cell it could not read: {broken}"); + assert!(read_import_record(&store).await.expect("the index answers for its own keys").is_none(), "a reading that stopped wrote no record for a later run to trust"); + + // What crossed stays: `temperature` read cleanly, so it is declared and sealed, and the + // leg's record says nothing about it. A retry of the same tree then refuses on the + // aspect that is already there rather than finishing the job — the stranding the module + // doc lists as a boundary, pinned here so the doc stays the one the code keeps. + let crossed = store.read_time_range("temperature", i64::MIN, i64::MAX).await.expect("the aspect that read cleanly reads back"); + assert_eq!(crossed.0.len(), 1, "the leg writes one aspect at a time, so `temperature` crossed before `vibration` stopped it"); + let retry = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect_err("the half-read tree cannot be read again quietly: the aspect it already sealed would be doubled"); + assert!(matches!(retry.downcast_ref::(), Some(RowsModeError::AspectNotEmpty { aspect, .. }) if aspect.as_str() == "temperature"), "the retry refuses on the aspect already in the store, which is the trap 14c has to design around: {retry}"); + } + + /// 14a promised the operator's tree keeps its bytes. The readable leg reads it and writes + /// nothing: every file it found is still the file it left, at the same path. + #[tokio::test] + async fn reading_the_tree_leaves_every_file_it_found_as_it_found_it() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let before = tree_state(&root).await; + assert_eq!(before.keys().filter(|name| name.ends_with("measurements.db")).count(), 3, "the test's tree holds three series databases"); + let store = store_at(&dir.path().join("store"), "plant", "pump-01").await; + import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect("the readable leg reads the tree's pump-01 series"); + let after = tree_state(&root).await; + for (path, bytes) in &before { + assert_eq!(after.get(path), Some(bytes), "reading the tree changed {path}"); + } + assert_eq!(after.keys().collect::>(), before.keys().collect::>(), "the leg added no file to the tree, and deleted none from it"); + } + + /// The record is the store's own, and it survives the store being closed and reopened: + /// it is what a later run, or an operator asking what came across, reads. + #[tokio::test] + async fn a_finished_reading_is_recorded_in_the_store_itself() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("plant"); + build_tree(&root).await; + let at = dir.path().join("store"); + let store = store_at(&at, "plant", "pump-01").await; + assert!(read_import_record(&store).await.expect("an untouched store has no record").is_none(), "a store nobody read a tree into answers with nothing"); + let written = import_into(&store, &root, "pump-01", NonZeroUsize::new(2).unwrap()).await.expect("the readable leg reads the tree's pump-01 series"); + drop(store); + let reopened = store_at(&at, "plant", "pump-01").await; + let read_back = read_import_record(&reopened).await.expect("the record is in the store's index, not in memory").expect("a store an import wrote has a record"); + let round_trip = serde_json::to_string(&read_back).expect("the record is JSON by construction"); + assert_eq!(round_trip, serde_json::to_string(&written).expect("the record the leg returned"), "the record the leg returned is the record the store kept"); + assert_eq!(read_back.not_carried.iter().map(|entry| entry.path.as_str()).collect::>(), ["metadata.db", "pump-01/ingestion.db"], "the record keeps the tree's catalog and this subject's own state beside the rows; pump-02's dictionary directory is not this reading's to report"); + assert_eq!(read_back.aspects.iter().map(|aspect| aspect.aspect.as_str()).collect::>(), ["temperature", "vibration"], "the record says which series crossed, and no other subject's"); + } + + #[test] + fn a_chunk_never_splits_rows_that_share_an_instant() { + let rows: Vec<(i64, BigDecimal)> = [0i64, 1, 1, 1, 2, 3, 3, 4].iter().map(|when| (*when, BigDecimal::from(*when))).collect(); + let chunks = chunks_keeping_instants_together(&rows, 2); + let instants: Vec> = chunks.iter().map(|chunk| chunk.iter().map(|(when, _)| *when).collect()).collect(); + assert_eq!(instants, vec![vec![0, 1, 1, 1], vec![2, 3, 3], vec![4]], "the four rows at instant 1 go into one segment though the caller asked for two rows each: a segment a few rows longer than asked is cheaper than the reconciliation that folds two segments and keeps only the newer row at an instant they share"); + assert_eq!(chunks.iter().map(|chunk| chunk.len()).sum::(), rows.len(), "and no row is lost or sealed twice on the way"); + assert!(chunks.iter().all(|chunk| !chunk.is_empty()), "a chunk always has rows, so no seal call is asked to write an empty segment"); + assert!(chunks_keeping_instants_together(&[], 3).is_empty(), "an empty series seals nothing at all"); + let even: Vec<(i64, BigDecimal)> = [1i64, 2, 3, 4].iter().map(|when| (*when, BigDecimal::from(*when))).collect(); + assert_eq!(chunks_keeping_instants_together(&even, 2).iter().map(|chunk| chunk.len()).collect::>(), vec![2, 2], "a series with one row per instant still gets the chunk size it asked for"); + } +} diff --git a/weftdb/tests/compat_v010.rs b/weftdb/tests/compat_v010.rs index d8e26956..8d5c27ab 100644 --- a/weftdb/tests/compat_v010.rs +++ b/weftdb/tests/compat_v010.rs @@ -19,14 +19,15 @@ //! - what a v0.1.0 backup actually contains, which is **not** the segment data. That is a //! finding, not an aspiration: see [`v010_backup_covers_the_control_plane_only`]. //! -//! The rows-mode tree is refused, not read: an open of it names the aspect databases and +//! The rows-mode tree is refused on `open`: an open of it names the aspect databases and //! writes nothing (FRE-14a, [`a_v010_rows_mode_directory_is_refused_and_writes_nothing`]). -//! Turning those rows into frames is the other half of FRE-14 and is still open; the tree is -//! here so that slice is driven by what the release wrote instead of by a fixture someone -//! invented. +//! It is also now read, deliberately, through [`weftdb::rows_mode`] — the second half of +//! FRE-14 ([`the_v010_rows_mode_tree_reads_into_a_store_of_its_own_database`]). The tooling an +//! operator would run (FRE-14c) is still open; the tree is here so both halves are driven by +//! what the release wrote instead of by a fixture someone invented. use std::{ - collections::BTreeMap, path::{Path, PathBuf} + collections::{BTreeMap, BTreeSet}, num::NonZeroUsize, path::{Path, PathBuf} }; use weft_physical_type::{PhysicalType, TimeUnit}; @@ -413,3 +414,121 @@ async fn the_migrated_measurements_are_the_ones_v010_counted() { let frames = files_under(&to).into_iter().filter(|name| name.ends_with(".weftseg")).collect::>(); assert_eq!(frames.iter().filter(|name| name.starts_with("segments/")).count(), 6, "the upgrade moved six frames forward and did not rewrite or add any: {frames:?}"); } + +/// One row of `rows.csv`: subject, aspect, the instant in **seconds** as the grid spells it, +/// and the value — `None` for the empty field that means a null in this file. +fn grid() -> Vec<(String, String, i64, Option)> { + let text = std::fs::read_to_string(fixture().join("rows.csv")).expect("the shared grid is part of the fixture"); + let mut rows = Vec::new(); + for (number, line) in text.lines().enumerate() { + if line.is_empty() { + continue; + } + let fields: Vec<&str> = line.split(',').collect(); + let [subject, aspect, when, value] = fields.as_slice() else { + panic!("grid line {}: expected subject,aspect,timestamp,value in {line:?}", number + 1); + }; + let Ok(when) = when.parse::() else { + panic!("grid line {}: timestamp {when:?} is not an integer", number + 1); + }; + rows.push((subject.to_string(), aspect.to_string(), when, (!value.is_empty()).then(|| value.to_string()))); + } + rows +} + +/// The second half of FRE-14: the rows-mode tree is not only refused on `open`, it can be +/// **read** — [`weftdb::rows_mode`] walks the tree the release wrote and `import_into` carries +/// one subject's series into a store scoped to that database. FRE-14c, the tooling an operator +/// runs, is still open; this is the ground it gets built on, driven by the release's bytes +/// rather than by a tree this build invented. +/// +/// The scale assertions are why this runs against the fixture and not only against synthetic +/// trees. rows-mode stamped `timestamp_millis()` into `timestamp` +/// (`v0.1.0:weftdb/src/types/database/inputs.rs`, `capture_measurement`), while the Storage-v2 +/// batches fed from **the same grid** kept seconds +/// (`v0.1.0:weftdb/src/types/batches/batch.rs`: `BigDecimal::from_i64(….timestamp())`) — which +/// is why the migrated store above declares [`TimeUnit::Seconds`] while an import of this tree +/// declares `Millis`. One grid, two scales, a factor of a thousand apart: declare the wrong one +/// and every migrated row lands six decades early, with nothing to show it. +#[tokio::test] +async fn the_v010_rows_mode_tree_reads_into_a_store_of_its_own_database() { + let source = fixture().join("legacy/v010_legacy"); + let dir = tempfile::tempdir().unwrap(); + let tree = dir.path().join("v010_legacy"); + copy_tree(&source, &tree); + + let found = weftdb::rows_mode::discover(&tree).await.expect("the tree the release wrote is a tree this build can read"); + assert_eq!(found.database, "v010_legacy", "0.1.x named its database by that top directory, and that name is what the store's scope takes"); + assert_eq!(found.subjects(), ["plant"], "the fixture's tree holds one subject, named by its directory under the root"); + + // Row counts and spans are derived from the grid, not hard-coded, so a regenerated grid + // shows up here rather than in a diff of a Turso file nobody can read. + let grid = grid(); + for aspect in ["flow", "price", "temp"] { + let carried: Vec<&(String, String, i64, Option)> = grid.iter().filter(|(_, name, _, value)| name == aspect && value.is_some()).collect(); + let series = found.aspects.iter().find(|series| series.name == aspect).unwrap_or_else(|| panic!("the tree holds a series of {aspect}")); + assert_eq!(series.subject, "plant", "the aspect's subject is the directory it sits in — 0.1.x wrote no subject into `measurements.db`"); + assert_eq!(usize::try_from(series.rows).expect("a row count in range"), carried.len(), "{aspect}: rows-mode carries the grid's non-null rows and skips the null ones rather than writing a sentinel"); + let (first, last) = carried.iter().fold((i64::MAX, i64::MIN), |(lo, hi), (_, _, when, _)| (lo.min(*when), hi.max(*when))); + assert_eq!((series.first_timestamp, series.last_timestamp), (Some(first * 1000), Some(last * 1000)), "{aspect}: the tree's `timestamp` is milliseconds where the grid says seconds — see the test's doc"); + } + + // What stays behind is named, per path, rather than dropped quietly. + let refused: BTreeSet<&str> = found.not_carried.iter().map(|entry| entry.path.as_str()).collect(); + assert!(refused.contains("metadata.db"), "the 0.1.x catalog stays behind, and says why: {refused:?}"); + assert!(refused.contains("plant/price/dictionaries"), "an aspect's compression dictionaries stay behind: {refused:?}"); + for file in files_under(&tree) { + if !file.ends_with(".db") || file.ends_with("/measurements.db") { + continue; + } + assert!(refused.contains(file.as_str()) || refused.iter().any(|entry| file.starts_with(&format!("{entry}/"))), "{file} is a database in the tree that the readable leg neither carries nor names"); + } + assert!(found.not_carried.iter().all(|entry| !entry.reason.is_empty()), "every path left behind states why, in the words of the subsystem that owned it"); + + let store = SegmentStore::open_scoped(dir.path().join("plant-store"), "v010_legacy", "plant").await.expect("a store scoped to the tree's own database and subject opens"); + let record = weftdb::rows_mode::import_into(&store, &tree, "plant", NonZeroUsize::new(100).unwrap()).await.expect("the subject's series reads into the store of its own database"); + assert_eq!(record.timestamp_unit, weftdb::TIMESTAMP_UNIT, "the import declares the unit the timestamps carry; it does not infer it from how big the numbers look"); + assert_eq!(record.database, "v010_legacy"); + assert_eq!(record.subject, "plant"); + assert_eq!(record.aspects.len(), 3, "one entry per aspect the tree carried"); + assert_eq!(record.rows(), 371, "237 price + 54 temp + 80 flow: the grid's non-null rows, none lost and none added"); + for aspect in found.for_subject("plant") { + let entry = record.aspects.iter().find(|entry| entry.aspect == aspect.name).unwrap_or_else(|| panic!("the record covers {}", aspect.name)); + assert_eq!(entry.rows, aspect.rows, "{}: the record counts what the survey counted", aspect.name); + assert!(entry.segments >= 1, "{}: the rows were sealed", aspect.name); + assert!(!entry.encoding.is_empty(), "{}: the record names the encoding its values were declared in", aspect.name); + } + + // Values and instants, exactly, for all three aspects: the grid row for row, at a + // thousand times its spelled instant. Sorted on both sides, because two rows of `price` + // share the epoch and the store does not promise an order between equals. + for aspect in ["flow", "price", "temp"] { + let (timestamps, values) = store.read_time_range(aspect, i64::MIN, i64::MAX).await.expect("the migrated aspect reads back"); + let mut expected: Vec = grid.iter().filter(|(_, name, _, value)| name == aspect && value.is_some()).map(|(_, _, when, value)| format!("{}|{}", when * 1000, canonical(value.as_deref().expect("a carried row has a value")))).collect(); + expected.sort(); + let mut actual: Vec = timestamps.iter().zip(&values).map(|(when, value)| format!("{when}|{}", value.as_ref().map(|value| canonical(&value.to_string())).unwrap_or_default())).collect(); + actual.sort(); + assert_eq!(actual, expected, "{aspect}: every value of the tree arrives at the same instant, and no decimal was traded for a nearby one"); + } + + // The tree cannot be read twice into the same store: after the first import there is no + // way to tell a migrated row from one the store sealed itself. + let again = weftdb::rows_mode::import_into(&store, &tree, "plant", NonZeroUsize::new(100).unwrap()).await.expect_err("reading the same tree twice would double the rows, and nothing would say so"); + assert!(matches!(again.downcast_ref::(), Some(weftdb::RowsModeError::AlreadyImported { aspects: 3, .. })), "the refusal is typed and counts what it refused to write again: {again}"); + let stored = weftdb::rows_mode::read_import_record(&store).await.expect("the record reads back").expect("the import left its record in the store's own index"); + assert_eq!(stored.source, tree.to_string_lossy(), "the record names the tree it read"); + assert_eq!(stored.rows(), 371, "the record's own account adds up to what the store holds"); + assert_eq!(store.read_time_range("price", i64::MIN, i64::MAX).await.expect("the aspect still reads").0.len(), 237, "the refusal added nothing"); + + // Reading must not write. `discover` opens every `measurements.db`, and each of the + // release's databases sits beside a `-wal` and a `-log`: a reader that checkpoints them + // would change the very bytes the fixture gate compares. The test works on a copy, and + // this says the copy is still what the release left, file for file and byte for byte. + let files = files_under(&tree); + assert!(!files.is_empty(), "the copied tree has files to compare"); + for name in files { + let (fixture_bytes, copy_bytes) = (std::fs::read(source.join(&name)).expect("the fixture holds the file"), std::fs::read(tree.join(&name)).expect("the copy holds the file")); + assert_eq!(fixture_bytes.len(), copy_bytes.len(), "reading {name} changed its length: the readable leg writes to the tree it reads"); + assert_eq!(fixture_bytes, copy_bytes, "reading {name} changed its bytes: the readable leg writes to the tree it reads"); + } +} From dde137a4fe1b908adb27d761987aee5c1ab633e2 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Tue, 20 Oct 2026 21:22:16 -0500 Subject: [PATCH 11/12] docs(roadmap): tick PAC-5a, the config registry slice The registry, its two anti-drift tests and the startup report are all in this PR, so the W1 line is checked here rather than in a follow-up: 51 VarSpec entries, tiers 7 Stable / 17 Tuning / 2 Experimental / 25 Test, and a coverage walk over the five crates that ship binaries. Signed-off-by: Justin Icenhour --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 4b39e353..5693dbb2 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1146,7 +1146,7 @@ any write, never purge an unproven frame), with its unfinished work on `durabili - [ ] **FRE-1** Release truth, per-crate versions, single edition, publish split, package hygiene - [ ] **FRE-3** Golden frames, golden sidecars and persisted-encoding goldens (gated codecs included) - [x] **PAC-1b** Stripped dist profile and a third-party license bundle (absorbs EVI-20) — *the dist profile shipped in #71; the bundle in #87: `THIRD-PARTY-LICENSES.md`, generated from `Cargo.lock` alone by `cargo-about` over the union of the five release targets' graphs (397 crates, 220 license texts, 432 crate entries), build-dependencies in because a `-sys` build script links foreign code into the archive, dev-dependencies out because no archive carries tests, `stage.sh` packing it next to the two crates' own notices, and a `third-party licenses` CI job that regenerates it and fails on drift. `deny.toml`'s allow-list is `about.toml`'s, in both files' comments, because a dependency one accepts and the other does not is a crate that ships unlabelled. `cfg_block` (through `turso_core`) publishes the Apache-2.0 **notice** rather than the text, so it is only in the bundle at all through a `clarify` entry checksummed against the published file* - - [ ] **PAC-5a** Configuration registry skeleton: one `VarSpec` registry (absorbs FRE-11, ROB-10's knobs) + - [x] **PAC-5a** Configuration registry skeleton: one `VarSpec` registry (absorbs FRE-11, ROB-10's knobs) — *#88: `weft-server/src/settings.rs` holds one `VarSpec` per environment name the workspace reads — 51 entries, each with a tier (`Stable` 7 / `Tuning` 17 / `Experimental` 2 / `Test` 25), a one-line summary, what it accepts, its default, and the file that reads it — sorted by name, so `spec()` is a binary search and `suggest()` is a two-edit Levenshtein over the same table. Three rules keep it from rotting: a test that walks every `.rs` in the five shipped crates (`weftdb`, `weft-server`, `weft-tui`, `weft-bench`, `weft-orchestration`) and fails on any `WEFT_`/`TEST_`/`SKIP_`/`OTEL_` literal that is not registered; a test that asserts the walk sees every crate it claims, so the net cannot quietly narrow; and a tier partition that names the Stable, Experimental and Test-tier variables explicitly. Fixture files (`settings.rs` itself, `tests/config_warnings.rs`) sit outside the net because their literals are the disease, not the symptom. `report()` runs as the first startup phase and prints, for a name WeftDB does not read, the closest registered one or the honest "there is nothing close to mistype"; for a test-harness channel set on a real server, the file that reads it. Values are still read where they were read: this is the registry, not the read path. PAC-5b's strict parsing, PAC-5c's single read path and PAC-5d's generated documentation build on it.* - [ ] **TAG-3** Open `ValueKind` in weftdb, provenance scope; rewords the **B-tags** gate and backlog item - [ ] **EVI-1** Claims hygiene: links, contradicted wording, precision, memory, cadence, corpus protection - [ ] **EVI-3** Run manifest (schema v23: the 2026-10-09 routine took v20–v22, so EVI-3 takes the next free number per the plan's rule): runtime provenance, secret-safe capture, reference_grade verdict From ad21916e231eab913405e51538216166c15ee708 Mon Sep 17 00:00:00 2001 From: Justin Icenhour Date: Thu, 22 Oct 2026 08:56:40 -0500 Subject: [PATCH 12/12] docs(roadmap): land the W2 stack, and say so where the waves are listed #91 closed after its head branch became #92-#96's merge target, so five slices of W2 sat on a branch off main with their boxes ticked. This merge puts the code, the ticks and the wave status back in agreement: X-NAME, ROB-2, PAC-3, X-FRAME, and FRE-14's refusal and rows-mode import, plus main's roadmap edits, the cfg(unix) on the symlink probe, and PAC-5a's tick, whose registry arrived in #88 without its line. Signed-off-by: Justin Icenhour --- ROADMAP.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index 5693dbb2..0a981394 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1113,6 +1113,15 @@ phase of the [1.0 plan](docs/release/1.0-plan.md#6-waves) (lanes, dependencies, `→` is merge order after the previous slice, *(owner)* marks a step only the owner can take, and a box is ticked only when its slice is on main. +*Status 2026-10-22:* The W2 stack is on main. #91 (X-NAME → main) was closed after its head branch +had become the merge target for #92–#96, so X-NAME, ROB-2, PAC-3, X-FRAME and FRE-14's two +first slices — the 0.1.x data-directory refusal and the rows-mode import — sat on +`xname/declare-validation` with their boxes ticked and their code off main. This wave lands them: +the stack, main's two roadmap edits, the `#[cfg(unix)]` that keeps the symlink probe honest on a +Windows runner, and PAC-5a's roadmap tick (`b5d0667`), whose registry merged in #88 without its +line being checked. The owner's W0 items are done — private vulnerability reporting is on, and the +stale `v0.1.0-preview.1` draft prerelease is gone. + *Status 2026-10-20:* W1 is running. The v0.1.0 baseline is cut and pinned (`compat/releases.toml`, re-checked by CI's `release pins` job, #83), and X-FX's fixture set is on main (#84): three trees written by binaries compiled from the tag, the reads that release recorded for them, and