fix(storage): a missing SSTable component is a defect, not a later problem

ferrosa-storage/README.md

@@ -202,6 +202,15 @@ data through this crate, almost always via the `Arc<dyn DataStore>` indirection
   found at flush/replay are written to a durable `quarantine/*.jsonl` sidecar
   instead of crashing; the self-heal controller detects corrupt SSTables and
   quarantines them under a safety rail.
+- **Startup SSTable health** (`sstable_health.rs`) — decides whether a
+  generation on disk can serve reads before it is loaded. A critical component
+  (`Data.db`, `Partitions.db`) that is **missing**, **zero-byte**, or
+  **unreadable** withholds the generation; `Rows.db` is excluded because the
+  writer legitimately emits it empty for simple partitions. The judgement is a
+  pure function so it is testable without a disk or a cluster, and the log line
+  names the component and the reason — a manifest entry pointing at a file that
+  is gone used to surface only as `No such file or directory (os error 2)` with
+  no table, generation or path, failing every read of that table.
 - **Accord** (`accord/`) — per-shard conflict index + protocol log for
   strict-serializable transactions. Also defines `TransactionCommitter` (ADR-021):
   the front-end-facing seam CQL/Postgres `BEGIN`/`COMMIT` call to commit a

ferrosa-storage/specs/fmea.md

@@ -29,6 +29,7 @@ the top of the scale: a defect here is silent data loss or corruption.
 | ST-15 | **Concurrent schema replays replace a live table store** — two registrations observe an absent table, then the later builder overwrites the first store after it has accepted writes | Read-after-write and secondary-index queries return empty results until a later write or flush rebuilds state | 8 | 3 | 6 | 144 | **Mitigated**: table installation is compare-and-install under the table-map write lock. A losing replay merges its index declarations into the winning store and never replaces active memtable/index postings. Guarded by `engine::tests::late_table_registration_cannot_replace_live_memtable`. |
 | ST-16 | **`ALTER TABLE ADD` shifts storage and index cell ordinals** — regular columns are ordered by the column-name comparator, so adding a column that sorts before an existing column re-numbers cell ordinals while existing rows and index declarations were written under the old layout | Post-ALTER reads could misattribute pre-ALTER row cells to the wrong column, and index declarations/backfills could extract postings from the wrong cell, serving false empty or wrong-column results (`agent_memory.entity_store` phonetic lookups after ddl/020) | 8 | 4 | 7 | 224 → 32 | **Fixed**: `TableStore::update_schema` remaps every positional index declaration (scalar `indexed_columns`, `fulltext_indexes`, `vector_index_configs`, filtered-index predicate clauses) through the old schema's column name before swapping the new schema; an unmappable ordinal fails loud (error log) instead of silently indexing the wrong cell. `StorageEngine::update_table_schema` now flushes dirty pre-ALTER memtable/replay rows under the old schema while holding the table-map write lock, then swaps schemas and discards the covered commit-log position. Reads already use each SSTable `SerializationHeader` to remap physical ordinals by column name, and index backfills now map current ordinals and filtered predicates through each SSTable header before building sidecars. Tests: `store::update_schema_remaps_index_ordinal_when_added_column_sorts_first`, `store::index_backfill_maps_current_ordinals_to_legacy_sstable_source_ordinals`, `engine::update_table_schema_preserves_unflushed_pre_alter_row_ordinals`, and `ferrosa-cql` `router::tests::phonetic_keyed_equality_matches_when_fulltext_shares_the_column`. |
 | ST-17 | **Storage flush replaces the registry snapshot with an incompatible JSON array** — two components wrote different formats to `schema.json` | Next boot can register storage tables while the CQL registry starts empty, making intact SSTables unreachable | 10 | 3 | 8 | 240 → 10 | **Fixed**: `SchemaSnapshotStore` is the sole crash-safe publisher of discriminated `schema.json`; storage-only registrations use `storage-schema.json`. Reads and writes are capped at 64 MiB and stream without an intermediate JSON buffer. Corrupt, unknown, oversized, and legacy-array registry inputs are quarantined and returned as startup errors. File locking, verified staging, fsync, atomic rename, directory fsync, and three retained generations cover concurrent writers and crash boundaries. |
+| ST-18 | **Startup repair ignores a MISSING SSTable component** — the zero-byte check quarantined an empty `Data.db`/`Partitions.db` but let an absent one fall through, on the reasoning that it would "fail in `open_sstable_from_dir`" | It does fail there, as `storage: I/O error: No such file or directory (os error 2)` naming no table, generation or path — and it fails EVERY read of that table rather than skipping the one generation. Observed on the ferrosa-memory native cluster: `agent_memory.mobile_control_cursor_state` and `knowledge_by_state` unreadable on all three nodes, the cursor allocator silently reverting to an older value and regressing ~10,600 cursors | 8 | 4 | 8 | 256 → 24 | **Fixed**: `sstable_health.rs` makes the decision a pure function over probed components — missing, zero-byte and unreadable all withhold the generation, `Rows.db` stays excluded because the writer legitimately emits it empty. Both startup log lines now name the component and the reason. Tests: `sstable_health::tests` (8 cases, including `a_missing_data_file_is_unusable` and `the_first_defect_in_order_is_the_one_reported`). Residual: the manifest entry itself is not reconciled, so the generation is re-probed and re-quarantined on every boot. |
 
 ## Top risks to act on
 

ferrosa-storage/src/engine.rs

@@ -4385,29 +4385,34 @@ impl StorageEngine {
             // a per-partition row index (ferrosa-sstable/src/writer.rs:212).
             // A 0-byte Rows.db is the expected output, not corruption; the
             // reader treats a missing/empty Rows.db as "no row index".
-            // Only Data.db and Partitions.db being zero-byte is unrecoverable.
-            let critical_components = ["Data.db", "Partitions.db"];
-            let mut quarantine = false;
-            for comp in &critical_components {
-                let path = Self::generation_component_path(table_dir, &gen_str, comp)
-                    .unwrap_or_else(|| table_dir.join(format!("{gen_str}-{comp}")));
-                match std::fs::metadata(&path) {
-                    Ok(meta) if meta.len() == 0 => {
-                        quarantine = true;
-                    }
-                    Err(_) => {
-                        // Missing component — will fail in open_sstable_from_dir.
-                    }
-                    _ => {}
-                }
-            }
-            if quarantine {
+            // Only Data.db and Partitions.db are critical.
+            //
+            // A MISSING component counts, not only a zero-byte one. This used
+            // to fall through on the reasoning that it "will fail in
+            // open_sstable_from_dir" -- and it does, as
+            // `storage: I/O error: No such file or directory (os error 2)`
+            // naming no table, no generation and no path, failing every read
+            // of that table rather than the one generation. Missing is
+            // strictly worse than empty and is caught in the same place.
+            let probes: Vec<(&str, crate::sstable_health::ComponentProbe)> =
+                crate::sstable_health::CRITICAL_COMPONENTS
+                    .iter()
+                    .map(|comp| {
+                        let path = Self::generation_component_path(table_dir, &gen_str, comp)
+                            .unwrap_or_else(|| table_dir.join(format!("{gen_str}-{comp}")));
+                        (*comp, crate::sstable_health::probe_component(&path))
+                    })
+                    .collect();
+            let defect = crate::sstable_health::first_unusable_component(&probes);
+            if let Some((component, reason)) = defect {
                 match repair_mode {
                     StartupSstableRepairMode::Quarantine => {
                         tracing::error!(
                             gen,
                             dir = %table_dir.display(),
-                            "storage-engine: startup repair quarantining SSTable with zero-byte critical component"
+                            component,
+                            reason = reason.describe(),
+                            "storage-engine: startup repair quarantining SSTable with an unusable critical component"
                         );
                         let quarantine_dir = table_dir.join("quarantine");
                         let _ = std::fs::create_dir_all(&quarantine_dir);
@@ -4422,7 +4427,9 @@ impl StorageEngine {
                             gen,
                             dir = %table_dir.display(),
                             mode = ?repair_mode,
-                            "storage-engine: startup repair excluded SSTable with zero-byte critical component from active readers; files remain in place for salvage"
+                            component,
+                            reason = reason.describe(),
+                            "storage-engine: startup repair excluded SSTable with an unusable critical component from active readers; files remain in place for salvage"
                         );
                         excluded_count += 1;
                     }

ferrosa-storage/src/lib.rs

@@ -40,6 +40,7 @@ pub mod schema_snapshot;
 pub mod self_heal;
 pub mod snapshot;
 pub mod spill_budget;
+pub(crate) mod sstable_health;
 pub mod store;
 pub mod subscription_observer;
 #[cfg(feature = "test-generators")]

ferrosa-storage/src/sstable_health.rs

@@ -0,0 +1,197 @@
+//! Module: Decide whether an SSTable generation on disk can serve reads.
+//!
+//! Correctness: Correct when every way a critical component can fail to serve
+//! — absent, empty, or unreadable — withholds the generation from readers, and
+//! when a component that is legitimately empty does not.
+//!
+//! Last revised: 2026-08-28
+//! Last changed: New.
+//!
+//! # Why this is a pure function and not an fs call
+//!
+//! The decision lives inside `load_existing_sstables_and_sidecars`, which walks
+//! a data directory and needs an engine, a manifest and a disk to run at all.
+//! The judgement it makes — is this generation servable — needs none of those,
+//! and is the part that was wrong. Separated, it is testable without a cluster.
+
+/// What probing one component on disk found.
+///
+/// `Missing` and `Unreadable` are distinct because they mean different things
+/// to an operator: one is a file that is gone, the other is a file that is
+/// there and cannot be stat'd. Both withhold the generation; only the message
+/// differs.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum ComponentProbe {
+    /// The component exists and holds this many bytes.
+    Present(u64),
+    /// The path does not exist.
+    Missing,
+    /// The path exists but could not be inspected.
+    Unreadable,
+}
+
+/// Why a critical component cannot serve reads.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum ComponentDefect {
+    /// Zero bytes. For a critical component this is unrecoverable.
+    Empty,
+    /// The file the manifest points at is not on disk.
+    Missing,
+    /// Present, but its metadata could not be read.
+    Unreadable,
+}
+
+impl ComponentDefect {
+    /// How this reads in a log line an operator has to act on.
+    pub(crate) fn describe(self) -> &'static str {
+        match self {
+            Self::Empty => "zero-byte",
+            Self::Missing => "missing from disk",
+            Self::Unreadable => "unreadable",
+        }
+    }
+}
+
+/// The components a generation cannot serve reads without.
+///
+/// Rows.db is deliberately absent. The SSTable writer emits a zero-byte
+/// Rows.db for simple partitions that need no per-partition row index, and the
+/// reader treats a missing or empty Rows.db as "no row index". Treating it as
+/// critical would quarantine healthy SSTables.
+pub(crate) const CRITICAL_COMPONENTS: [&str; 2] = ["Data.db", "Partitions.db"];
+
+/// The first critical component that cannot serve reads, if any.
+///
+/// Returns the first rather than all of them because the caller's decision is
+/// binary — withhold the generation or do not — and naming one concrete file
+/// is what makes the log line actionable. Order follows `probes`, so the
+/// message is deterministic when several components are defective.
+pub(crate) fn first_unusable_component<'a>(
+    probes: &[(&'a str, ComponentProbe)],
+) -> Option<(&'a str, ComponentDefect)> {
+    probes
+        .iter()
+        .find_map(|(name, probe)| component_defect(*probe).map(|defect| (*name, defect)))
+}
+
+/// Whether one probed component can serve reads.
+const fn component_defect(probe: ComponentProbe) -> Option<ComponentDefect> {
+    match probe {
+        ComponentProbe::Present(0) => Some(ComponentDefect::Empty),
+        ComponentProbe::Present(_) => None,
+        // A component the manifest references but disk does not have cannot be
+        // read, and deferring that to the read path costs an operator a whole
+        // table: the failure that reaches them is a bare ENOENT naming no
+        // file, no generation and no table.
+        ComponentProbe::Missing => Some(ComponentDefect::Missing),
+        ComponentProbe::Unreadable => Some(ComponentDefect::Unreadable),
+    }
+}
+
+/// Probe one component path, mapping io failures onto the states above.
+pub(crate) fn probe_component(path: &std::path::Path) -> ComponentProbe {
+    match std::fs::metadata(path) {
+        Ok(meta) => ComponentProbe::Present(meta.len()),
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => ComponentProbe::Missing,
+        Err(_) => ComponentProbe::Unreadable,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn probes(
+        data: ComponentProbe,
+        partitions: ComponentProbe,
+    ) -> Vec<(&'static str, ComponentProbe)> {
+        vec![("Data.db", data), ("Partitions.db", partitions)]
+    }
+
+    /// The ordinary healthy generation.
+    #[test]
+    fn a_generation_whose_components_all_hold_bytes_is_servable() {
+        let probed = probes(ComponentProbe::Present(4_096), ComponentProbe::Present(512));
+        assert_eq!(first_unusable_component(&probed), None);
+    }
+
+    /// Pins the behaviour that already existed: a zero-byte critical component
+    /// is unrecoverable and must be withheld.
+    #[test]
+    fn a_zero_byte_data_file_is_unusable() {
+        let probed = probes(ComponentProbe::Present(0), ComponentProbe::Present(512));
+        assert_eq!(
+            first_unusable_component(&probed),
+            Some(("Data.db", ComponentDefect::Empty))
+        );
+    }
+
+    /// The defect this module exists for.
+    ///
+    /// Startup repair used to treat a MISSING component as benign while
+    /// treating a zero-byte one as fatal, on the reasoning that it "will fail
+    /// in open_sstable_from_dir". It does — as `storage: I/O error: No such
+    /// file or directory (os error 2)` with no table, no generation and no
+    /// path, failing every read of that table. Missing is strictly worse than
+    /// empty and must be caught in the same place.
+    #[test]
+    fn a_missing_data_file_is_unusable() {
+        let probed = probes(ComponentProbe::Missing, ComponentProbe::Present(512));
+        assert_eq!(
+            first_unusable_component(&probed),
+            Some(("Data.db", ComponentDefect::Missing))
+        );
+    }
+
+    /// The other critical component, so the rule is not accidentally
+    /// Data.db-only.
+    #[test]
+    fn a_missing_partitions_file_is_unusable() {
+        let probed = probes(ComponentProbe::Present(4_096), ComponentProbe::Missing);
+        assert_eq!(
+            first_unusable_component(&probed),
+            Some(("Partitions.db", ComponentDefect::Missing))
+        );
+    }
+
+    /// Present but unstat-able is still unservable, and says so differently.
+    #[test]
+    fn an_unreadable_component_is_unusable() {
+        let probed = probes(ComponentProbe::Unreadable, ComponentProbe::Present(512));
+        assert_eq!(
+            first_unusable_component(&probed),
+            Some(("Data.db", ComponentDefect::Unreadable))
+        );
+    }
+
+    /// Deterministic reporting: with two defects the message names the first,
+    /// so the same broken generation does not log a different file each boot.
+    #[test]
+    fn the_first_defect_in_order_is_the_one_reported() {
+        let probed = probes(ComponentProbe::Missing, ComponentProbe::Present(0));
+        assert_eq!(
+            first_unusable_component(&probed),
+            Some(("Data.db", ComponentDefect::Missing))
+        );
+    }
+
+    /// Rows.db is legitimately zero-byte for simple partitions, so it must not
+    /// be in the critical set — including it would quarantine healthy tables.
+    #[test]
+    fn rows_db_is_not_a_critical_component() {
+        assert!(!CRITICAL_COMPONENTS.contains(&"Rows.db"));
+        assert_eq!(CRITICAL_COMPONENTS, ["Data.db", "Partitions.db"]);
+    }
+
+    /// The defects an operator reads are distinguishable.
+    #[test]
+    fn each_defect_describes_itself_distinctly() {
+        let all = [
+            ComponentDefect::Empty,
+            ComponentDefect::Missing,
+            ComponentDefect::Unreadable,
+        ];
+        let described: std::collections::BTreeSet<_> = all.iter().map(|d| d.describe()).collect();
+        assert_eq!(described.len(), all.len(), "two defects read the same");
+    }
+}