From fcc8d82545884cb2ff2725def20a846987e18ec4 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 31 Jul 2026 14:59:24 -0400 Subject: [PATCH] Repair checkpoint recovery reconciliation --- crates/levcs-store/src/checkpoint.rs | 161 ++++++- crates/levcs-store/src/engine.rs | 604 ++++++++++++++++++++---- crates/levcs-store/src/recovery.rs | 16 + crates/levcs-store/src/sys.rs | 24 +- doc/instance-throughput-rewrite-plan.md | 136 +++++- 5 files changed, 853 insertions(+), 88 deletions(-) diff --git a/crates/levcs-store/src/checkpoint.rs b/crates/levcs-store/src/checkpoint.rs index 2f2707e..6410829 100644 --- a/crates/levcs-store/src/checkpoint.rs +++ b/crates/levcs-store/src/checkpoint.rs @@ -25,7 +25,6 @@ //! and digest domain. This is recorded as an interface note rather than worked //! around silently. -use std::fs::File; use std::io::IoSlice; use std::path::{Path, PathBuf}; @@ -949,6 +948,159 @@ pub fn install( dir: &Path, checkpoint: &Checkpoint, counters: &DurabilityCounters, +) -> Result { + install_at(dir, checkpoint, counters, false) +} + +/// A checkpoint reduced to what it durably *claims*, with the values a crash and +/// the recovery after it are entitled to move erased. +/// +/// This is a comparison aid, never a description of what may be published. +/// Three things come out, and `reconciles_with` re-imposes a rule on two of +/// them rather than letting them go: +/// +/// * `created_at_micros` — when the image was built, not what it says. This one +/// really is free. +/// * The resume point, which the found image records against the journal that +/// was active before the crash. +/// * Per-receipt visibility, which recovery re-derives and extends. +/// +/// Everything left is a claim about the state below the horizon: catalog, refs, +/// and every durable field of every receipt. `receipt_for_recovery` rebuilds all +/// of those from the frame's own facts, so a faithful pre-crash image and a +/// post-recovery rebuild agree on them exactly, and anything that does not agree +/// is describing something else. +pub(crate) fn durable_claims(checkpoint: &Checkpoint) -> Checkpoint { + let mut claims = checkpoint.clone(); + claims.created_at_micros = 0; + claims.active_journal_id = [0u8; 16]; + claims.active_journal_offset = 0; + for receipt in claims.receipts.iter_mut() { + receipt.first_receipt_visibility_micros = None; + receipt.receipt_visible_until_micros = 0; + } + claims +} + +/// Whether `candidate` may replace the finalized `existing` at their shared +/// name — the whole safety argument for overwriting a checkpoint. +/// +/// `Err` carries the clause for the refusal message. Three tests, and the last +/// two exist because erasing a field from [`durable_claims`] is not the same as +/// deciding it does not matter. +/// +/// 1. **Same durable claims.** Otherwise the file describes state this shard +/// cannot produce, and overwriting it would destroy the only evidence of +/// however that happened. +/// 2. **The resume point may differ only by naming a different journal.** A +/// checkpoint whose `active_journal_id` matches the surviving journal is +/// trusted for its *offset* by recovery, without a derivation of its own, so +/// a matching identity at a disagreeing offset is a claim about live state +/// that this shard did not make — it reopens as `recovered through sequence +/// N but its active journal resumes at N+1`. The ordinary strand passes +/// freely: it names the journal the crash left behind, recovery sealed that +/// journal away, and the candidate names the fresh one. +/// 3. **The replacement may not shorten retention.** Recovery may extend a +/// receipt's first-visibility and deadline and never reduce them (plan +/// invariant 7), so the candidate — built after that recovery — must be at or +/// beyond the found image on both. A `None` first-visibility is the weakest +/// value there is and is therefore always allowed on the found side; on the +/// candidate's side it means the promotion this rule exists to preserve never +/// happened. +/// +/// Receipts are compared pairwise by position, which is sound only because rule +/// 1 has already established that both sides carry the same receipts in the same +/// canonical order. +pub(crate) fn reconciles_with(existing: &Checkpoint, candidate: &Checkpoint) -> Result<(), String> { + if durable_claims(existing) != durable_claims(candidate) { + return Err(format!( + "describes different state than this shard would checkpoint at {}; publishing it \ + would suppress replay of frames it does not describe", + candidate.shard_committed_sequence + )); + } + + if existing.active_journal_id == candidate.active_journal_id + && existing.active_journal_offset != candidate.active_journal_offset + { + return Err(format!( + "resumes the journal this shard is still writing at offset {}, which is not where \ + this shard would resume it ({}); recovery trusts that offset without deriving it", + existing.active_journal_offset, candidate.active_journal_offset + )); + } + + for (found, ours) in existing.receipts.iter().zip(candidate.receipts.iter()) { + let regressed = match ( + found.first_receipt_visibility_micros, + ours.first_receipt_visibility_micros, + ) { + (Some(found_first), Some(our_first)) => our_first < found_first, + (Some(_), None) => true, + (None, _) => false, + }; + if regressed || ours.receipt_visible_until_micros < found.receipt_visible_until_micros { + return Err(format!( + "holds receipt retention for operation {} that this shard's image would \ + shorten; recovery may extend a receipt's visibility and never reduce it", + found.operation_id.to_hex() + )); + } + } + + Ok(()) +} + +/// [`install`] over a name a crash already finalized, and only over one this +/// call has just proved out. +/// +/// A directory fence failure lands *after* the rename, so an interrupted +/// checkpoint leaves a whole, validating file at its published name that no +/// manifest references. Its name comes from the committed sequence, so the +/// retry cannot choose another one, and `rename_noreplace` refuses forever. +/// +/// The retry cannot simply publish what it finds, either: it happens after the +/// recovery the crash forced, and recovery legitimately moves values the found +/// image still holds at their pre-crash settings — the resume point, and receipt +/// visibility, which recovery may only ever extend. Publishing the older image +/// would walk those backwards. So the shard writes its own and replaces. +/// +/// **This is the one name in the store that may be overwritten**, and the proof +/// that licenses it is [`reconciles_with`], which is why the two are one +/// operation rather than two. Exposing the replacement separately would put an +/// unproved checkpoint overwrite on this module's surface, and the proof is the +/// entire safety argument: the occupant must read back as a valid checkpoint for +/// this shard, agree on every durable claim, and be no *stronger* than the image +/// replacing it. Because the two then differ only where the replacement is the +/// better of the pair, and because the rename is atomic, a manifest that names +/// this file holds a valid referent at every instant — before, during, and +/// after. +/// +/// `existing` is the occupant as *this store* read it back, not as the caller +/// wishes it were; the caller is responsible for having loaded it through +/// [`load_newest_valid`] at this exact path. Frozen-seam amendment recorded as +/// contract review 2026-07-30-F; the `rename_noreplace` rule stands everywhere +/// else, including for a checkpoint name no such proof covers. +pub(crate) fn replace_reconciled( + dir: &Path, + existing: &Checkpoint, + checkpoint: &Checkpoint, + counters: &DurabilityCounters, +) -> Result { + reconciles_with(existing, checkpoint).map_err(|reason| { + StoreError::Corruption(format!( + "{} already exists and {reason}; it is left untouched", + dir.join(checkpoint.file_name()).display() + )) + })?; + install_at(dir, checkpoint, counters, true) +} + +fn install_at( + dir: &Path, + checkpoint: &Checkpoint, + counters: &DurabilityCounters, + replace: bool, ) -> Result { std::fs::create_dir_all(dir)?; let bytes = checkpoint.encode()?; @@ -990,7 +1142,12 @@ pub fn install( } crate::sys::fdatasync(&file, counters)?; } - crate::sys::rename_noreplace(&tmp_path, &final_path).map_err(|e| { + let renamed = if replace { + crate::sys::rename_replace(&tmp_path, &final_path) + } else { + crate::sys::rename_noreplace(&tmp_path, &final_path) + }; + renamed.map_err(|e| { // Leave the temporary behind for forensics rather than unlinking it // on a path that already surprised us. StoreError::from(e) diff --git a/crates/levcs-store/src/engine.rs b/crates/levcs-store/src/engine.rs index 30cb166..b6d9ab5 100644 --- a/crates/levcs-store/src/engine.rs +++ b/crates/levcs-store/src/engine.rs @@ -1437,24 +1437,42 @@ fn spawn_shard_writer( ))); } - // What recovery already made unreplayable. A checkpoint moved the replay - // start, so everything the manifest's runs hold and every namespace the - // checkpoint's catalog carries is below the horizon and is not rebuilt by - // replaying anything. Without a checkpoint there is no horizon and the - // whole journal is replayable, which is the pre-checkpoint accounting. - let checkpointed = if recovered.report.checkpoint_sequence.is_some() { - ( - recovered - .retained_generation - .index_runs - .iter() - .map(|retained| retained.run().entry_count()) - .sum::(), - recovered.catalog.len() as u64, - ) - } else { - (0, 0) - }; + // Two corrections to replay accounting that are deliberately *not* the same + // idea, however similar they look here. + // + // `run_entry_baseline` cancels **duplicate representation**; it is not a + // claim that these particular runs lie below the checkpoint horizon. + // Recovery both retains every run it finds and rebuilds every replayed frame + // into the delta, so a frame a retained run already covers is counted twice. + // Subtracting the run total *as it stood at open* removes one copy, and it + // has to stay that open-time total: a later seal moves entries out of the + // backlog and into runs above this fixed baseline, leaving the sum + // unchanged. Restricting the subtraction to runs below the horizon would + // double-count every post-checkpoint entry instead. + // + // `checkpoint_namespaces` really is a horizon claim, because namespaces have + // no backlog term to cancel against. It must therefore come from the + // checkpoint's own catalog rather than from `recovered.catalog`, which also + // carries every namespace replay just rebuilt — and will rebuild again. + // + // Without a checkpoint both terms are zero, so a run's entries are counted + // once in the run and once in the replayed backlog. That over-states the + // pressure and can only refuse early, never admit work a reopen cannot + // rebuild, so it stays as it is rather than widening this change. + let (run_entry_baseline, checkpoint_namespaces) = + if recovered.report.checkpoint_sequence.is_some() { + ( + recovered + .retained_generation + .index_runs + .iter() + .map(|retained| retained.run().entry_count()) + .sum::(), + recovered.checkpoint_namespaces, + ) + } else { + (0, 0) + }; let mut repositories = BTreeMap::new(); for (namespace, record) in recovered.catalog.iter() { @@ -1472,8 +1490,8 @@ fn spawn_shard_writer( in_flight_reservation: None, repositories, tail_generation: tail.logical_generation, - checkpointed_run_entries: checkpointed.0, - checkpointed_namespaces: checkpointed.1, + run_entry_baseline, + checkpoint_namespaces, journal, shard_index, poison: None, @@ -1553,15 +1571,25 @@ struct ShardWriter { /// Logical index generation of the active journal, so an index location /// built here resolves to the same pinned file after a reopen. tail_generation: u64, - /// What the newest checkpoint already made unreplayable, so the replay - /// ceiling measures what a reopen would actually rebuild. + /// Sealed-run entries that some other term of the replay ceiling already + /// accounts for — a **duplicate-representation baseline**, not a horizon. /// - /// Set at open from the state recovery loaded, and again by every - /// checkpoint this writer takes. Both are the same statement: everything - /// sealed and every namespace catalogued as of the checkpoint is below its - /// horizon, and a reopen starts above it. - checkpointed_run_entries: u64, - checkpointed_namespaces: u64, + /// Recovery retains every run it finds *and* rebuilds every replayed frame + /// into the delta, so frames a retained run covers arrive in the ceiling + /// twice. This cancels one copy. It is the run total as of the moment it was + /// set, and it stays fixed while the shard runs: a seal moves entries from + /// the backlog into runs *above* it, which is exactly why the total does not + /// move. It says nothing about which side of the checkpoint those runs sit + /// on, and must not be narrowed to the ones below it. + run_entry_baseline: u64, + /// Namespaces the newest checkpoint's own catalog carries, and therefore the + /// ones a reopen does not rebuild by replaying. + /// + /// This one *is* a horizon claim: namespaces have no backlog term to cancel + /// against, so it must count the checkpoint's catalog rather than the live + /// or fully replayed one, which also holds namespaces created above the + /// horizon that every replay rebuilds. + checkpoint_namespaces: u64, poison: Option, } @@ -2742,15 +2770,15 @@ impl ShardWriter { backlog: &UnsealedBacklog, incoming: Option<&ValidatedTransaction>, ) -> (u64, u64) { - // Sealed-run entries **above the checkpoint horizon**. A reopen rebuilds - // what it replays, and it replays nothing at or below the newest - // checkpoint — so counting every run entry made checkpointing unable to - // relieve the pressure it exists to relieve: a shard at the ceiling - // checkpointed successfully and was then refused its next single-object - // transaction. Subtracting rather than dropping the accounting: runs - // sealed *after* the checkpoint cover frames a reopen does replay, and - // ignoring them would reopen the hole that made a store accept work it - // could not read back. + // Sealed-run entries, less the entries some other term here already + // stands for. Counting every run entry outright made checkpointing + // unable to relieve the pressure it exists to relieve: a shard at the + // ceiling checkpointed successfully and was then refused its next + // single-object transaction. Dropping run accounting instead would + // reopen the hole that let a store accept work it could not read back, + // because runs sealed after the checkpoint do cover frames a reopen + // replays. Subtracting the baseline is what leaves each replayable + // entry counted exactly once — see `run_entry_baseline`. let sealed: u64 = root .retained_generations() .values() @@ -2758,7 +2786,7 @@ impl ShardWriter { .flat_map(|generation| generation.index_runs.iter()) .map(|retained| retained.run().entry_count()) .sum::() - .saturating_sub(self.checkpointed_run_entries); + .saturating_sub(self.run_entry_baseline); let open_group: u64 = self .pending .iter() @@ -2780,10 +2808,12 @@ impl ShardWriter { if let Some(transaction) = incoming { namespaces.insert(transaction.namespace); } - // Namespaces the checkpoint's catalog already carries are not rebuilt - // by replay either, and the byte ceiling is measured over what replay - // rebuilds. - let namespaces = (namespaces.len() as u64).saturating_sub(self.checkpointed_namespaces); + // Namespaces the checkpoint's own catalog carries are not rebuilt by + // replay either, and the byte ceiling is measured over what replay + // rebuilds. Only the checkpoint's count may be subtracted here: a + // namespace created *above* the horizon is rebuilt by every replay, so + // subtracting it would let the shard admit a section it cannot reopen. + let namespaces = (namespaces.len() as u64).saturating_sub(self.checkpoint_namespaces); ( entries, crate::index::encoded_bytes_for(entries, namespaces), @@ -3030,7 +3060,8 @@ impl ShardWriter { // describe other state, and the manifest, the journal and the root are // all untouched. Wedging the shard for a condition it can still refuse // cleanly would turn a diagnosable state into an outage. - let installed = self.install_or_adopt_checkpoint(&paths, &checkpoint, &counters)?; + let installed = + self.install_or_replace_checkpoint(&paths, &manifest, &checkpoint, &counters)?; // The manifest may not commit through a sequence no retained segment // names, so the prefix in `active/` is sealed in the same publication. @@ -3179,16 +3210,19 @@ impl ShardWriter { return Err(self.poison_now(format!("publishing a checkpoint: {error}"))); } - // The horizon moved. Everything now sealed, and every namespace this - // checkpoint catalogued, is below it and is not rebuilt by replay — - // which is what makes the next admission measure the work a reopen - // would actually do rather than the whole history of the shard. - self.checkpointed_run_entries = successor + // The horizon moved, so both terms are re-taken against the state this + // checkpoint just published. The journal is fresh and the backlog is + // empty, so re-baselining the runs to their current total leaves the + // ceiling measuring only work taken after this point — the same + // one-copy-per-entry rule the open-time baseline enforces. The + // namespace term comes from the checkpoint's own catalog, which here is + // also the live one: nothing has been written above the horizon yet. + self.run_entry_baseline = successor .index_runs .iter() .map(|retained| retained.run().entry_count()) .sum(); - self.checkpointed_namespaces = checkpoint.catalog.len() as u64; + self.checkpoint_namespaces = checkpoint.catalog.len() as u64; // Only after both the checkpoint and its authoritative manifest are // fenced, so every retained manifest keeps a retained referent. @@ -3300,33 +3334,59 @@ impl ShardWriter { }) } - /// Install the checkpoint, or adopt a finalized one already at its name. + /// Install the checkpoint, or replace a finalized one already at its name. /// /// A crash between `install`'s rename and the manifest that publishes it /// leaves a complete, fenced checkpoint no manifest references. Its name is /// derived from the sequence, so the next attempt at the same sequence /// renames onto it — and `rename_noreplace` refuses, permanently, for a - /// file that is not wrong but merely unreferenced. Validating and adopting - /// it is what turns a wedged shard back into a checkpointed one. + /// file that is not wrong but merely unreferenced. /// - /// # What "the same checkpoint" has to mean + /// # Why the retry replaces rather than adopts /// - /// Matching the name and the committed sequence is not enough, and the gap - /// is not theoretical: publishing a *different* checkpoint at the right - /// sequence suppresses replay of exactly the frames it fails to describe, - /// so a checkpoint with an empty catalog makes a namespace disappear at the - /// next open with nothing anywhere reporting a fault. Adoption therefore - /// requires logical equivalence to the checkpoint this call would have - /// written — catalog, refs, receipts, resume point and all — and - /// `created_at_micros` is the only field allowed to differ, being when the - /// image was built rather than what it says. + /// Adopting it — publishing the bytes already on the device — was the + /// obvious repair and is wrong twice, because the retry necessarily happens + /// *after* the recovery the crash forced, and the found image still holds + /// values from before it: /// - /// Anything else at the name is refused. The file may be perfectly valid - /// and simply describe another state; overwriting it would destroy the - /// evidence, and adopting it would publish a claim this shard cannot make. - fn install_or_adopt_checkpoint( + /// * **Receipt visibility.** Recovery re-derives first-visibility and the + /// retention deadline and may only ever *extend* them (plan invariant 7). + /// The found image carries the pre-crash pair; publishing it puts an + /// earlier `Some(first_visibility)` on the device, and the *next* open + /// anchors on that value instead of re-promoting — so retention regresses, + /// which the invariant forbids. + /// * **The resume point.** Recovery does not independently re-derive this. + /// When a checkpoint's `active_journal_id` matches the surviving journal it + /// trusts the recorded offset outright, so a found image naming that + /// journal at the wrong offset is adopted straight into a store that + /// cannot reopen. + /// + /// So the shard writes its own image and replaces. Both hazards disappear + /// with the same stroke: what reaches the device is what this call built, + /// carrying the promoted deadlines and a resume point describing the journal + /// this checkpoint is about to seal. + /// + /// # What the occupant still has to prove + /// + /// Replacement is not licence to overwrite. Publishing a *different* + /// checkpoint at the right sequence suppresses replay of exactly the frames + /// it fails to describe — an empty catalog at the live sequence makes a + /// namespace disappear at the next open with nothing reporting a fault — and + /// a file describing other state is evidence of something this shard does not + /// understand. So the occupant must read back as a valid checkpoint for this + /// shard, and then satisfy [`crate::checkpoint::reconciles_with`] — which + /// [`crate::checkpoint::replace_reconciled`] applies itself, so the proof + /// and the overwrite cannot come apart — before a single byte moves. + /// Otherwise it is left exactly where it is and the checkpoint refuses, + /// without poisoning: nothing durable has moved. + /// + /// A published name is never a candidate. If the current manifest already + /// references this file the shard is not recovering from a strand at all, and + /// overwriting a referenced checkpoint is not a repair. + fn install_or_replace_checkpoint( &self, paths: &crate::segment::ShardPaths, + manifest: &Manifest, checkpoint: &crate::checkpoint::Checkpoint, counters: &Arc, ) -> Result { @@ -3337,18 +3397,32 @@ impl ShardWriter { return crate::checkpoint::install(&directory, checkpoint, counters) } // A symlink or a device at the published name is refused for the - // same reason the temporary is opened through the funnel: adopting - // it would publish a manifest row pointing at something the store - // never wrote. + // same reason the temporary is opened through the funnel: writing + // through it would put the store's bytes somewhere it never chose, + // and a manifest row would then point at something it never wrote. crate::sys::NamedRegularFile::NotRegular => { return Err(StoreError::UnrecognizedLayout(format!( - "{} is not a regular file; refusing to adopt a checkpoint through it", + "{} is not a regular file; refusing to install a checkpoint through it", destination.display() ))) } crate::sys::NamedRegularFile::Opened(_) => {} } + let name = checkpoint.file_name(); + if manifest + .checkpoints + .iter() + .any(|(_, published)| *published == name) + { + return Err(StoreError::Corruption(format!( + "{} is referenced by the current manifest of shard {}, so it is published \ + rather than stranded; a checkpoint at this sequence has already been taken", + destination.display(), + self.shard_index + ))); + } + // Read back through the reader recovery itself uses, so a file that // does not validate is not a checkpoint here either. let existing = match crate::checkpoint::load_newest_valid( @@ -3371,18 +3445,10 @@ impl ShardWriter { } }; - let equivalent = crate::checkpoint::Checkpoint { - created_at_micros: checkpoint.created_at_micros, - ..existing - }; - if &equivalent != checkpoint { - return Err(StoreError::Corruption(format!( - "{} already exists and describes different state than this shard would checkpoint at {}; adopting it would publish a manifest that suppresses replay of frames it does not describe", - destination.display(), - checkpoint.shard_committed_sequence - ))); - } - Ok(destination) + // Proof and replacement are one call: there is no way to overwrite a + // checkpoint in this crate that does not first establish the occupant + // reconciles with what is about to replace it. + crate::checkpoint::replace_reconciled(&directory, &existing, checkpoint, counters) } /// The successor pin: what the predecessor held, plus the sealed segment @@ -6818,6 +6884,47 @@ mod index_maintenance_tests { RootLayout::new(root).shard(shard) } + /// The checkpoint rows the manifest `CURRENT` names — the only list that + /// makes a checkpoint on the device authoritative rather than residue. + /// The checkpoint on the device at one sequence, read back through the + /// reader recovery uses. + #[cfg(feature = "failpoints")] + fn load_checkpoint_at(root: &Path, shard: u16, sequence: u64) -> crate::checkpoint::Checkpoint { + let directory = shard_paths(root, shard).checkpoints(); + let root_uuid = segment::read_format(&RootLayout::new(root)) + .expect("FORMAT") + .root_uuid; + match crate::checkpoint::load_newest_valid(&directory, &root_uuid, shard, 8) + .expect("load the newest valid checkpoint") + { + crate::checkpoint::CheckpointLoad::Loaded { + checkpoint, path, .. + } => { + assert_eq!( + path.file_name().and_then(|name| name.to_str()), + Some(format!("{sequence}.checkpoint").as_str()), + "the newest valid checkpoint is not the one under test" + ); + *checkpoint + } + other => panic!("expected a loadable checkpoint at {sequence}, got {other:?}"), + } + } + + #[cfg(feature = "failpoints")] + fn manifest_checkpoints(root: &Path, shard: u16) -> Vec<(u64, String)> { + let paths = shard_paths(root, shard); + let root_uuid = segment::read_format(&RootLayout::new(root)) + .expect("FORMAT") + .root_uuid; + let Some(pointer) = read_current(&paths, &root_uuid).expect("read CURRENT") else { + return Vec::new(); + }; + read_manifest(&paths, pointer.generation, &root_uuid) + .expect("read the current manifest") + .checkpoints + } + /// Every index run the manifest `CURRENT` names, which is the only list /// that makes a run authoritative. fn manifest_runs(root: &Path, shard: u16, root_uuid: [u8; 16]) -> Vec { @@ -7736,6 +7843,67 @@ mod index_maintenance_tests { ); } + /// A namespace created *above* the checkpoint horizon is replayed, so the + /// ceiling has to keep counting it. + /// + /// The writer took its "already durable" namespace count from the fully + /// recovered catalog, which is the checkpoint's catalog with every replayed + /// frame applied on top. A namespace born after the checkpoint therefore + /// subtracted itself out of the accounting: admission measured a section + /// that was not there, accepted a transaction on that basis, and the next + /// reopen — rebuilding the section for real — refused the store it had just + /// written. Unlike the run-entry baseline there is no backlog term to + /// cancel against here, so the count must come from the checkpoint itself. + #[test] + fn replay_accounting_keeps_a_namespace_the_checkpoint_never_saw() { + let serial = writer_serial(); + let temporary = tempfile::tempdir().expect("tempdir"); + let below = NamespaceId([0x57; 32]); + let above = NamespaceId([0x58; 32]); + { + let engine = StoreEngine::open(sealing_options(&serial, temporary.path(), 4_000_000)) + .expect("open a fresh root"); + block_on(engine.submit(create_transaction(below, 2))).expect("create the first"); + drop( + engine + .checkpoint() + .expect("checkpoint, catalogue only the first"), + ); + block_on(engine.submit(create_transaction(above, 3))) + .expect("create the second above the horizon"); + } + + // Room for exactly the one entry replay rebuilds and the one section it + // needs — so anything the accounting drops here shows up as a store that + // accepts what it cannot reopen. + let configure = || { + let mut options = sealing_options(&serial, temporary.path(), 4_000_000); + options.max_active_index_bytes = crate::index::encoded_bytes_for(1, 1); + options + }; + { + let engine = + StoreEngine::open(configure()).expect("reopen over the replayed namespace"); + let refused = block_on(engine.submit(push_transaction(above, 0x71, 0x71, None))) + .expect_err( + "a second entry in the replayed namespace needs two entries and a section \ + after the next reopen, which does not fit; accepting it writes a store \ + that cannot be opened", + ); + assert!( + matches!( + refused, + StoreError::LimitExceeded { + limit: "max_active_index_bytes", + .. + } + ), + "expected the byte ceiling, got {refused:?}" + ); + } + StoreEngine::open(configure()).expect("a store must reopen whatever it accepted"); + } + /// The pins a generation holds must be the rows its manifest publishes. /// /// The manifest trims to `checkpoint_retain`, so pruning unlinks the older @@ -7774,6 +7942,278 @@ mod index_maintenance_tests { assert_eq!(newest.checkpoints.len(), 2, "and retention is honoured"); } + /// The crash the replacement path exists for, produced by the fault that + /// actually produces it. + /// + /// `checkpoint::install` fences the directory *after* the rename, so a + /// failure there leaves the checkpoint at its published name — whole, + /// validating, and referenced by no manifest. The name is derived from the + /// committed sequence, so every later attempt at that sequence renames onto + /// it, and at the replay ceiling no write can move the shard to a different + /// sequence. Without a repair this is a permanent wedge. + /// + /// What the repair may not be is *adoption*. The retry runs after the + /// recovery the crash forced, and the file on the device still carries + /// pre-recovery receipt visibility and a resume point for the journal that + /// recovery replaced. Publishing those walks retention backwards. So the + /// assertions below are not only "the checkpoint succeeded" but "what is on + /// the device afterwards is the image this shard built". + /// + /// Sealing the run first is load-bearing. With no unsealed backlog the + /// checkpoint skips `seal_index`, so the first directory fsync it issues is + /// `install`'s — which is what makes the one-shot fault land after the + /// rename rather than somewhere harmless before it. The assertion below + /// fails the test rather than silently proving nothing if that stops being + /// true. + #[cfg(feature = "failpoints")] + #[test] + fn a_checkpoint_stranded_by_a_crash_is_replaced_after_recovery() { + let serial = writer_serial(); + let temporary = tempfile::tempdir().expect("tempdir"); + let namespace = NamespaceId([0x59; 32]); + let configure = || sealing_options(&serial, temporary.path(), 2); + + let committed; + { + let engine = StoreEngine::open(configure()).expect("open a fresh root"); + block_on(engine.submit(create_transaction(namespace, 2))).expect("create"); + block_on(engine.submit(push_transaction(namespace, 0x62, 0x62, None))) + .expect("a second object, which fills the entry ceiling"); + // Admission seals a due backlog *before* it checks the ceiling, so + // this refusal leaves the run written and the layers discarded + // without committing anything further — the one state that is + // checkpointable, not yet checkpointed, and carries no backlog. + block_on(engine.submit(push_transaction(namespace, 0x63, 0x63, None))) + .expect_err("a third entry is over the replay ceiling"); + assert_eq!( + engine.index_maintenance().unsealed_delta_layers, + 0, + "entry pressure must have sealed the backlog already; with layers left the \ + checkpoint seals first and the fault lands there instead of after the rename" + ); + committed = engine + .committed_root() + .shard_committed_sequence(0) + .expect("a shard that has committed work has a sequence"); + + crate::sys::arm(&serial, crate::sys::Fault::DirSyncEio); + let interrupted = engine.checkpoint(); + crate::sys::disarm(&serial); + interrupted + .err() + .expect("the injected directory fsync failure did not stop the checkpoint"); + } + + let name = format!("{committed}.checkpoint"); + assert!( + checkpoint_files(temporary.path(), 0).contains(&name), + "the crash must leave a finalized checkpoint at its published name, or this \ + test exercises installation rather than the repair: {:?}", + checkpoint_files(temporary.path(), 0) + ); + assert!( + !manifest_checkpoints(temporary.path(), 0) + .iter() + .any(|(sequence, _)| *sequence == committed), + "and no manifest may reference it, or it is published rather than stranded" + ); + let stranded = load_checkpoint_at(temporary.path(), 0, committed); + assert!( + !stranded.receipts.is_empty(), + "the stranded image must carry receipts, or the retention assertions below \ + range over nothing" + ); + + { + let engine = + StoreEngine::open(configure()).expect("reopen over the stranded checkpoint"); + drop(engine.checkpoint().expect( + "the stranded file describes exactly what this shard would checkpoint at this \ + sequence; refusing it wedges the shard at a name it can never write again", + )); + assert!( + manifest_checkpoints(temporary.path(), 0) + .iter() + .any(|(sequence, _)| *sequence == committed), + "the repair must publish the checkpoint it wrote" + ); + } + + // The heart of it. Recovery promoted these receipts on the way through, + // and plan invariant 7 lets recovery extend a receipt's visibility and + // never reduce it. Publishing the bytes the crash left behind would put + // the pre-promotion deadline back on the device, and the *next* open + // would anchor on it rather than re-promote. + let published = load_checkpoint_at(temporary.path(), 0, committed); + assert_ne!( + published.active_journal_id, stranded.active_journal_id, + "the published resume point must name the journal this checkpoint sealed, not \ + the one recovery already replaced" + ); + let mut advanced = false; + for (before, after) in stranded.receipts.iter().zip(published.receipts.iter()) { + assert_eq!( + before.operation_id, after.operation_id, + "canonical ordering must hold, or these comparisons pair unrelated receipts" + ); + assert!( + after.receipt_visible_until_micros >= before.receipt_visible_until_micros, + "operation {} lost retention: {} is earlier than the {} already on the device", + before.operation_id.to_hex(), + after.receipt_visible_until_micros, + before.receipt_visible_until_micros + ); + match ( + before.first_receipt_visibility_micros, + after.first_receipt_visibility_micros, + ) { + (Some(before_first), Some(after_first)) => { + assert!( + after_first >= before_first, + "operation {} regressed its first-visibility anchor", + before.operation_id.to_hex() + ); + advanced |= after_first > before_first; + } + (Some(_), None) => panic!( + "operation {} lost a durable first-visibility anchor", + before.operation_id.to_hex() + ), + (None, _) => {} + } + advanced |= after.receipt_visible_until_micros > before.receipt_visible_until_micros; + } + assert!( + advanced, + "no receipt moved across the recovery, so publishing the stranded bytes would \ + have passed these assertions too and they prove nothing" + ); + + // And what was published has to be the truth: the manifest now + // suppresses replay of everything through this sequence, so anything the + // published image failed to describe is gone. + let engine = StoreEngine::open(configure()).expect("reopen after the repair"); + let root = engine.committed_root(); + for object in [genesis_object().id, ObjectId([0x62; 32])] { + let key = IndexKey::new(namespace, object); + assert!( + root.index().get(&key).is_some(), + "{} was acknowledged before the crash and is unreachable after the repair", + object.to_hex() + ); + } + assert!( + root.repositories().contains_key(&namespace), + "the published checkpoint must still carry the namespace it catalogued" + ); + } + + /// A receipt for a checkpoint under reconciliation, at a stated visibility. + fn receipt_at( + operation: u8, + first: Option, + until: i64, + ) -> crate::checkpoint::ReceiptRecord { + crate::checkpoint::ReceiptRecord { + namespace: NamespaceId([0x5a; 32]), + operation_id: OperationId([operation; 16]), + operation_digest: ObjectId([operation; 32]), + repo_sequence: 1, + shard_sequence: 1, + current_authority: genesis_object().id, + refs: Vec::new(), + objects_new: 1, + retry_until_micros: 10_000, + first_receipt_visibility_micros: first, + receipt_visible_until_micros: until, + } + } + + fn checkpoint_carrying( + journal: [u8; 16], + offset: u64, + receipts: Vec, + ) -> crate::checkpoint::Checkpoint { + crate::checkpoint::Checkpoint { + shard_committed_sequence: 7, + active_journal_id: journal, + active_journal_offset: offset, + created_at_micros: 1_000, + receipts, + ..crate::checkpoint::Checkpoint::empty([7u8; 16], 0) + } + } + + /// The ordinary strand, which is the whole point of the path: the found + /// image names the journal recovery replaced, and its receipts predate the + /// promotion recovery performed. + #[test] + fn a_stranded_checkpoint_reconciles_across_the_recovery_that_follows_it() { + let found = checkpoint_carrying([1u8; 16], 4_096, vec![receipt_at(0x71, None, 10_000)]); + let ours = + checkpoint_carrying([2u8; 16], 512, vec![receipt_at(0x71, Some(50_000), 90_000)]); + crate::checkpoint::reconciles_with(&found, &ours) + .expect("this is exactly what a crash leaves behind"); + } + + /// P1: the resume pair cannot be waved through unconditionally. + /// + /// Recovery does not re-derive the offset. When a checkpoint names the + /// journal that is still live it trusts the recorded offset outright, so an + /// image claiming the *surviving* journal at a different offset is making a + /// claim about live state that this shard did not make. Publishing it + /// reopens as `recovered through sequence N but its active journal resumes + /// at N+1` — and at that point no write can move the shard to another name. + #[test] + fn a_found_checkpoint_may_not_redirect_the_journal_this_shard_is_writing() { + let live = [3u8; 16]; + let ours = checkpoint_carrying(live, 512, vec![receipt_at(0x71, Some(50_000), 90_000)]); + + let forged = checkpoint_carrying(live, 8_192, vec![receipt_at(0x71, Some(50_000), 90_000)]); + let refusal = crate::checkpoint::reconciles_with(&forged, &ours) + .expect_err("a disagreeing offset on the live journal must not reconcile"); + assert!( + refusal.contains("recovery trusts that offset without deriving it"), + "the refusal must name why the offset matters: {refusal}" + ); + + let agreeing = checkpoint_carrying(live, 512, vec![receipt_at(0x71, Some(50_000), 90_000)]); + crate::checkpoint::reconciles_with(&agreeing, &ours) + .expect("naming the live journal is fine when the offset agrees"); + } + + /// P1: replacement may not walk receipt retention backwards. + /// + /// Plan invariant 7 lets recovery extend a receipt's first-visibility and + /// deadline and never reduce them. A found image that is *ahead* of the + /// candidate on either would be shortened by the replacement, so the + /// checkpoint refuses rather than publishing the weaker pair. + #[test] + fn a_replacement_that_would_shorten_receipt_retention_is_refused() { + let ours = + checkpoint_carrying([2u8; 16], 512, vec![receipt_at(0x71, Some(50_000), 90_000)]); + + for ahead in [ + receipt_at(0x71, Some(60_000), 90_000), + receipt_at(0x71, Some(50_000), 120_000), + ] { + let found = checkpoint_carrying([1u8; 16], 4_096, vec![ahead]); + let refusal = crate::checkpoint::reconciles_with(&found, &ours) + .expect_err("the found image is ahead, so replacing it shortens retention"); + assert!( + refusal.contains("recovery may extend a receipt's visibility and never reduce it"), + "the refusal must name the invariant it protects: {refusal}" + ); + } + + // Losing a durable anchor altogether is the same regression in its + // strongest form. + let found = checkpoint_carrying([1u8; 16], 4_096, vec![receipt_at(0x71, Some(1), 0)]); + let unpromoted = checkpoint_carrying([2u8; 16], 512, vec![receipt_at(0x71, None, 90_000)]); + crate::checkpoint::reconciles_with(&found, &unpromoted) + .expect_err("an unpromoted candidate must not replace a durably anchored receipt"); + } + /// The installer became production-reachable, and with it the redirection /// family `segment.rs` closed. /// diff --git a/crates/levcs-store/src/recovery.rs b/crates/levcs-store/src/recovery.rs index 9954365..3034d2e 100644 --- a/crates/levcs-store/src/recovery.rs +++ b/crates/levcs-store/src/recovery.rs @@ -1646,6 +1646,14 @@ impl Eq for RecoveredTail {} pub struct RecoveredShard { pub shard_index: u16, pub catalog: NamespaceCatalog, + /// How many namespaces the authoritative checkpoint's own catalog carried, + /// before replay applied anything on top of it. + /// + /// [`RecoveredShard::catalog`] is the fully replayed state and cannot answer + /// this: replay may add namespaces above the checkpoint horizon, and those + /// are rebuilt by the *next* replay too. Zero when no checkpoint was + /// adopted, which is also the count an empty checkpoint carries. + pub checkpoint_namespaces: u64, pub refs: Vec, pub receipts: Vec, pub index: LayeredObjectIndex, @@ -2175,6 +2183,13 @@ fn recover_shard_under_lock( let replayed_facts: Vec = replayed.iter().map(|frame| frame.facts.clone()).collect(); let mut catalog = checkpoint.catalog.clone(); + // Captured before replay applies anything. The two counts diverge exactly + // when replay introduces a namespace the checkpoint never saw, and a + // consumer that needs "what a reopen does not have to rebuild" must read + // this one: the finished `catalog` also carries every namespace replay + // just rebuilt, and treating those as already-durable understates the + // work the *next* reopen faces. + let checkpoint_namespaces = catalog.len() as u64; verify_and_apply_catalog( &replayed_facts, checkpointed.then(|| committed.saturating_add(1)), @@ -2360,6 +2375,7 @@ fn recover_shard_under_lock( let recovered = RecoveredShard { shard_index: shard, catalog, + checkpoint_namespaces, refs, receipts, index, diff --git a/crates/levcs-store/src/sys.rs b/crates/levcs-store/src/sys.rs index ef83db2..676c318 100644 --- a/crates/levcs-store/src/sys.rs +++ b/crates/levcs-store/src/sys.rs @@ -318,6 +318,17 @@ pub(crate) fn fsync_dir_fd(directory: &File, counters: &DurabilityCounters) -> i /// `renameat2(RENAME_NOREPLACE)`. For every name that must never overwrite: /// new manifests, segments, checkpoints, index runs, and migration/restore /// siblings. Plan §9 forbids a copy fallback. +/// +/// One exception, and it is narrow. A checkpoint whose install was interrupted +/// after this rename and before the manifest that publishes it leaves a +/// finalized file at a name derived from the committed sequence, so the retry +/// can never choose another one and this call would refuse forever. The retry +/// may use [`rename_replace`] over **that** name, and only through +/// `checkpoint::replace_reconciled`, whose own `reconciles_with` proof +/// establishes before the rename that the occupant is a valid +/// checkpoint for the shard agreeing on every durable claim and no stronger +/// than its replacement. Contract review 2026-07-30-F; the rule stands for +/// every other name and for any checkpoint name not proved out that way. pub(crate) fn rename_noreplace(from: &Path, to: &Path) -> io::Result<()> { rustix::fs::renameat_with( rustix::fs::CWD, @@ -329,9 +340,16 @@ pub(crate) fn rename_noreplace(from: &Path, to: &Path) -> io::Result<()> { .map_err(|e| io::Error::from_raw_os_error(e.raw_os_error())) } -/// Plain atomic replacing rename. Exactly one legitimate call site: the -/// `CURRENT.tmp` -> `CURRENT` pointer install of scope 3.5, which by -/// definition replaces. Every other site uses `rename_noreplace`. +/// Plain atomic replacing rename. Two legitimate call sites, and no others: +/// +/// 1. The `CURRENT.tmp` -> `CURRENT` pointer install of scope 3.5, which by +/// definition replaces. +/// 2. `checkpoint::replace_reconciled`, repairing a checkpoint an interrupted +/// install left finalized at a name the retry cannot choose again — and only +/// over an occupant that call has proved reconciles with its replacement. +/// Contract review 2026-07-30-F; see [`rename_noreplace`]. +/// +/// Every other site uses `rename_noreplace`. pub(crate) fn rename_replace(from: &Path, to: &Path) -> io::Result<()> { std::fs::rename(from, to) } diff --git a/doc/instance-throughput-rewrite-plan.md b/doc/instance-throughput-rewrite-plan.md index 381f689..bcf42c5 100644 --- a/doc/instance-throughput-rewrite-plan.md +++ b/doc/instance-throughput-rewrite-plan.md @@ -1676,6 +1676,131 @@ B4 surface this commit touches — the emitter's remaining claims are unchanged. **Next, and in this dispatch:** recovery discarding an index run whose covered identity was not preserved, which turns 2026-07-30-B's refusal into reclamation. +##### Contract review 2026-07-30-F + +Two P1s and a P2 against the checkpoint-equivalence half of 2026-07-30-E. The namespace and +run-baseline halves of E passed review and are untouched. Run-discard remains held. + +**The mistake underneath both P1s.** E loosened the adoption predicate so the crash path could +succeed, and stopped there. It should have asked what *gets published* once the predicate passes. +Adoption publishes the bytes the crash left behind — an image written **before** the recovery that +the crash forced — so every field recovery is entitled to move was published at its pre-recovery +value. Erasing a field from a comparison is not the same as deciding it does not matter, and E +treated the two as one. + +**P1 — adoption shortened receipt retention.** Plan invariant 7: recovery re-derives a receipt's +first-visibility and deadline and "therefore only extends retention". The stranded image carries the +pre-promotion pair, so publishing it puts an earlier `Some(first_visibility)` on the device — and +the next open anchors on that value instead of re-promoting, because a durable anchor is exactly +what suppresses promotion. Retention regresses across a repair. Confirmed by the reviewer against an +isolated `DirSyncEio` run: both timestamps advanced during recovery and then went backwards after +adoption. + +**P1 — the resume pair could not be erased.** E claimed recovery independently verifies the offset. +It does not. When a checkpoint's `active_journal_id` matches the surviving journal, recovery trusts +the recorded offset outright. The reviewer changed only a valid orphan's resume pair to the +surviving journal's ID plus its preallocation end; adoption accepted it, a frame committed, and the +next open failed `Corruption("shard 0 recovered through sequence 2 but its active journal resumes at +3")`. + +**The repair is replacement, not adoption.** The shard writes its own image and installs it over the +found name. Both hazards close with one stroke: what reaches the device is what this call built, +carrying the promoted deadlines and a resume point naming the journal this checkpoint is about to +seal. `durable_claims` survives as what it always was — a comparison aid — and `reconciles_with` now +states the whole safety argument as three tests: + +1. **Same durable claims**, or the file describes state this shard cannot produce and overwriting it + would destroy the only evidence of however that happened. This is 2026-07-30-D's rule, unchanged. +2. **The resume point may differ only by naming a different journal.** The ordinary strand passes + freely — it names the journal the crash left behind, which recovery sealed away. A matching + identity at a disagreeing offset is refused, which is the reviewer's attack. +3. **The replacement may not shorten retention.** The candidate must be at or beyond the found image + on both visibility fields, which is the direction recovery guarantees. If it is not, something + other than the expected crash produced that file and the checkpoint refuses. + +**Frozen-seam amendment to `checkpoint.rs` (A2), and a narrow exception to a funnel rule.** +`replace_reconciled` is added beside `install`, sharing one body. `sys::rename_noreplace`'s contract +lists checkpoints among the names that must never be overwritten; **a checkpoint name this shard has +proved out under `reconciles_with` is now the one exception**, and the rule stands everywhere else, +including for any name the shard has not proved out. The safety argument is that the two images +agree on every durable claim and differ only where the replacement is the better of the pair, and +that `rename` is atomic — so a manifest naming that file holds a valid referent at every instant, +before, during and after. An unlink-then-install repair would not have this property, which is why +it was not used. A published name is refused outright as a further guard: if the current manifest +already references the file, the shard is not recovering from a strand. + +**Evidence.** The `DirSyncEio` regression now asserts what is on the device after the repair, not +only that the checkpoint succeeded: the published resume point must not name the journal recovery +replaced, no receipt may lose retention, and at least one must have *advanced* — that last one +exists so the retention assertions cannot pass vacuously. Reverting replacement to adoption fails +both halves independently. The three reconciliation rules also have direct unit coverage, including +the ordinary strand, which must keep reconciling. + +**P2 — the contracts contradicted the code and each other.** The method documentation still required +the resume point to match; E claimed both exclusions were safe. Both are rewritten here against what +the code now does. + +**P2, second round — an unproved overwrite was on the module's public surface.** The first cut put +`install_replacing` beside `install` as `pub`, while `reconciles_with` — the proof that licenses +overwriting anything — was private to `engine.rs`. Any caller reaching `checkpoint` could therefore +replace a checkpoint without the proof, which is precisely the operation the exception above was +granted for and only for. The predicate and its replacement are now **one operation**: +`durable_claims` and `reconciles_with` move to `checkpoint.rs`, which owns the type they reason +about; `install_replacing` is gone; and `pub(crate) replace_reconciled` performs the proof and the +rename together, so no path in the crate — let alone outside it — can overwrite a checkpoint without +first establishing that the occupant reconciles with what replaces it. `install` remains the only +public installer and remains no-replace. `sys::rename_replace`'s "exactly one legitimate call site" +sentence is amended to name both. + +##### Contract review 2026-07-30-E + +Two P1s against `bff8e8a`, one terminology correction, and the gap 2026-07-30-D disclosed. All +closed. Everything is in `engine.rs` except one addition to `RecoveredShard`: a new +`checkpoint_namespaces` field and the line that captures it. That is a **frozen-seam amendment to +`recovery.rs` (A2)**, directed by the lead in this dispatch, read-only in effect — it introduces no +rule and changes no existing value — and recorded here rather than assumed. + +**Not a bug, and the distinction is the point.** The reviewer's first correction was aimed at me: +the two subtractions in `replayable_index` look like one idea and are not. The **entry** term is a +*duplicate-representation* baseline. Recovery both retains every run it finds and rebuilds every +replayed frame into the delta, so `current run entries − pre-open run total + backlog` counts each +replayable entry exactly once, and a later seal merely moves entries from the backlog into runs +*above* that fixed baseline. Narrowing it to runs below the horizon — which I had proposed — would +double-count every post-checkpoint entry and refuse early. No entry accessor and no per-run horizon +seam is being added. What was wrong there was the name and the comment claiming a proof the term +does not make: it is now `run_entry_baseline`, documented as what it is. The reviewer's own +regression (four-entry ceiling, three replayed plus one incoming accepted, reopen succeeds, fifth +refused `observed: 5, allowed: 4`) pins the boundary from both sides. + +**P1 — replayed namespaces were subtracted away.** The **namespace** term has no backlog term to +cancel against, so it is a genuine horizon claim, and it was reading `recovered.catalog` — the +checkpoint's catalog with every replayed frame already applied. A namespace created above the +horizon therefore subtracted itself out: admission measured a section that was not there, accepted +on that basis, and the next reopen refused the store it had just written. Recovery now carries +`RecoveredShard::checkpoint_namespaces`, captured before replay applies anything, and the writer +counts that. Mutation-checked both ways — with the old count the regression's submit is accepted +and the reopen fails `observed: 158, allowed: 111`, which is the reviewer's reproduction exactly. + +**P1 — the advertised adoption path could not succeed.** Equality was the wrong predicate. The only +state that produces an unreferenced checkpoint is a crash, and the retry necessarily happens after +the recovery that follows it — so the stranded image and the rebuilt one always differed, and at the +replay ceiling no write could move the shard to another filename. The refusal was the original wedge +with a better error message. **Superseded in part by 2026-07-30-F**: the first attempt at this +loosened the predicate and then *adopted* the found bytes, which was wrong for reasons recorded +there. The repair is replacement, not adoption. + +**The disclosed gap is closed.** `DirSyncEio` produces the state deterministically, and no new +seam was needed. `checkpoint::install` fences the directory *after* the rename, so a failure there +strands a finalized, validating, unreferenced checkpoint. Reaching it needs the checkpoint to skip +`seal_index`, which is why the test first drives an admission that seals the due backlog and is +*then* refused by the ceiling — the one state that is checkpointable, uncheckpointed, and carries no +layers. The test asserts that precondition rather than assuming it. + +**One asymmetry left deliberately.** With no checkpoint at all both terms are zero, so a run's +entries are counted once in the run and again in the replayed backlog. That over-states pressure and +can only refuse early, never admit work a reopen cannot rebuild, so it is documented rather than +changed here. + ##### Contract review 2026-07-30-D Five review findings against `5462952`, all closed. The first two are the ones that mattered. @@ -1685,7 +1810,10 @@ Five review findings against `5462952`, all closed. The first two are the ones t silent logical deletion: the manifest suppresses replay of the frames the checkpoint covers, so an empty catalog at the live sequence makes the namespace disappear at the next open with nothing reporting a fault. Adoption now requires logical equivalence to the checkpoint the call would itself -have written, with `created_at_micros` the only field allowed to differ. Anything else at the name is +have written, with `created_at_micros` the only field allowed to differ — **narrowed by +2026-07-30-E** and again by **2026-07-30-F**, which reaches the same place by replacing the stranded +artifact rather than adopting it. +Anything else at the name is refused — and refused **without poisoning**, since nothing durable has moved. The test is the reopen: catalog, ref and receipt all read back through the public surface after the refusal, and the shard is still writable. @@ -1720,6 +1848,12 @@ the body the engine builds, and the honest options were a new failpoint seam in window or a test-only accessor. Neither belongs in this commit. The refusal path, which is the one that could lose data, is covered. +**Closed by 2026-07-30-E and 2026-07-30-F**, and the gap turned out to be hiding a defect rather +than only a missing test: writing the success path proved the equivalence predicate could never hold +across the recovery that separates a crash from its retry, and then that loosening it was not enough +either. Neither of the options considered here was needed — `DirSyncEio` reaches the state on its +own. + ##### Contract review 2026-07-28-C B4 re-pointed `store-bench` at a real `StoreEngine::submit` and found that four verification