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..c0a51785 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,67 @@ 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] + 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. + 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:?}"); +}