diff --git a/crates/levcs-store/src/engine.rs b/crates/levcs-store/src/engine.rs index c391ce7..97af4cc 100644 --- a/crates/levcs-store/src/engine.rs +++ b/crates/levcs-store/src/engine.rs @@ -2686,27 +2686,33 @@ impl ShardWriter { /// /// An `IndexLocation` names a **logical generation**, and a run is the first /// thing in this store that persists one across a session. That is only sound - /// while the generation it names is stable, and exactly one kind is: a - /// segment, which the manifest pins by name. The active tail's generation is - /// assigned by recovery from a counter over existing artifacts, so it moves - /// whenever any artifact appears — and when recovery seals a journal holding - /// frames, the resulting segment takes that counter's value rather than the - /// generation the tail had. Locations written against the tail therefore - /// dangle after the next open: `object_source` returns `None` for a run the - /// manifest still names, and a reader going through the run rather than the - /// replay delta above it reads nothing. + /// while the generation it names is stable across an open, and since contract + /// review 2026-07-30-A two kinds are: a **segment**, which the manifest pins + /// by name, and the **active tail**, whose identity is derived from the + /// manifest's committed prefix rather than from a counter over artifacts, and + /// which the segment recovery seals it into now inherits. Moving frames from + /// `active/` to `segments/` changes where they are, not what they are called, + /// so a run may cover the frames of the session that wrote them. /// - /// So coverage stops at the first layer holding an entry that is not - /// segment-backed. An oldest-first prefix rather than a filter, because the - /// discard is expressed as "everything through sequence N": covering a later - /// layer while skipping an earlier one would discard the earlier one too. + /// Coverage therefore stops at the first layer holding an entry the root + /// resolves to neither — an oldest-first prefix rather than a filter, because + /// the discard is expressed as "everything through sequence N": covering a + /// later layer while skipping an earlier one would discard the earlier one + /// too. /// - /// The practical consequence is recorded in scope §6.5 — until a rotation or - /// a checkpoint moves frames out of `active/`, the coverable set is whatever - /// a previous session left in segments, so sealing lags one session behind. - /// Lifting it means separating two numbers `recovery_generation` currently - /// serves as at once: the new manifest's generation and the sealed segment's - /// logical generation. + /// # What still bounds this + /// + /// A tail's identity can be *occupied* by an orphan `.seg` from an + /// interrupted seal, and recovery must then rename the frames. Rather than + /// leave a published run naming a generation nothing pins, recovery refuses + /// the open (contract review 2026-07-30-B), so the unsound state this + /// function used to avoid by restricting coverage is now avoided by + /// refusing to produce it. The closure — recovery discarding such a run + /// instead of refusing — is scope §6.5, with the checkpointing work. + /// + /// Separately and unrelated to identity: sealing moves no frame out of + /// `active/`, so the replay ceiling below still bounds admission until + /// `StoreEngine::checkpoint` can advance the committed prefix. fn coverable_through(&self, root: &CommittedRoot) -> Option { let mut covered = None; for layer in root.index().delta_layers().iter().rev() { @@ -6490,6 +6496,124 @@ mod index_maintenance_tests { assert!(orphan.is_file(), "and it is left alone, not reclaimed here"); } + /// Link the live journal into `segments/` under `generation`. + /// + /// Scope 3.4 step 4 interrupted before step 5: the name is taken, and what + /// holds it is not a readable segment, so recovery learns the generation is + /// occupied from the name alone. + fn orphan_segment_at(root: &Path, shard: u16, generation: u64) { + let paths = shard_paths(root, shard); + let journal = std::fs::read_dir(paths.active()) + .expect("active/") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .find(|path| path.extension().and_then(|value| value.to_str()) == Some("journal")) + .expect("an open root has an active journal"); + std::fs::hard_link( + &journal, + paths + .segments() + .join(crate::segment::segment_filename(generation, 0, 9)), + ) + .expect("link the orphan"); + } + + /// Contract review 2026-07-30-B: the two states an occupied identity leaves. + /// + /// The tail's identity is generation 1 and a published run holds locations + /// against it. An orphan `.seg` occupies that name, so recovery must seal + /// the frames under a different one — and the run then stays authoritative + /// through the manifest while resolving to nothing. Recovery refuses. + /// + /// Replay masks this while a lookup goes through the delta above the run, so + /// the assertion is the open itself. `StoreEngine::checkpoint` is a direct + /// run consumer and would not be masked. + #[test] + fn an_orphan_holding_a_published_runs_identity_refuses_the_open() { + let serial = writer_serial(); + let temporary = tempfile::tempdir().expect("tempdir"); + let namespace = NamespaceId([0x4A; 32]); + let configure = || { + let mut options = sealing_options(&serial, temporary.path(), 4_000_000); + options.max_index_runs = 3; + options.max_open_index_runs = 3; + options + }; + { + let engine = StoreEngine::open(configure()).expect("open a fresh root"); + block_on(engine.submit(create_transaction(namespace, 2))).expect("create"); + push_groups(&engine, namespace, 0x60, 2); + block_on(engine.submit(push_transaction(namespace, 0x68, 0x68, None))) + .expect("the group after the seal commits"); + assert_eq!(engine.index_maintenance().sealed_runs, 1, "the seal ran"); + orphan_segment_at(temporary.path(), 0, 1); + } + + let runs = manifest_runs(temporary.path(), 0, root_uuid_of(temporary.path())); + assert_eq!(runs.len(), 1, "one published run is the whole premise"); + + match StoreEngine::open(configure()) { + Err(StoreError::Corruption(message)) => { + assert!( + message.contains(&runs[0]), + "the refusal must name the run that cannot be resolved, not just \ + report a generation: {message}" + ); + } + Err(other) => panic!("expected Corruption naming the run, got {other:?}"), + // Not prose: the state this refuses is verified here, so removing + // the refusal reports what it costs rather than a bare expectation. + Ok(opened) => { + let root = opened.committed_root(); + let pinned = root.object_source(0, 1).expect("resolve generation 1"); + panic!( + "the open succeeded with a published run naming generation 1, and the \ + reopened root pins {pinned:?} there — every lookup reaching the run \ + rather than the replay delta above it reads nothing" + ); + } + } + } + + /// The other half, and the reason the refusal is conditioned on the run + /// rather than on the orphan. + /// + /// Same occupied identity, nothing published against it. An orphan segment + /// is a state in which the active journal remains the authority, and + /// refusing here would turn a recoverable root into an outage. + #[test] + fn an_orphan_holding_no_published_identity_still_opens() { + let serial = writer_serial(); + let temporary = tempfile::tempdir().expect("tempdir"); + let namespace = NamespaceId([0x4B; 32]); + let mut objects = vec![genesis_object().id]; + { + let engine = StoreEngine::open(sealing_options(&serial, temporary.path(), 4_000_000)) + .expect("open a fresh root"); + block_on(engine.submit(create_transaction(namespace, 2))).expect("create"); + objects.extend(push_groups(&engine, namespace, 0x70, 2)); + assert_eq!( + engine.index_maintenance().sealed_runs, + 0, + "nothing is published against the identity the orphan takes" + ); + orphan_segment_at(temporary.path(), 0, 1); + } + + let engine = StoreEngine::open(sealing_options(&serial, temporary.path(), 4_000_000)) + .expect("an orphan segment alone must not fail recovery"); + let root = engine.committed_root(); + for object in &objects { + assert!( + root.index() + .get(&IndexKey::new(namespace, *object)) + .is_some(), + "{} was acknowledged and must survive the reopen", + object.to_hex() + ); + } + } + /// The discard is scoped to the shard that sealed. #[test] fn only_the_sealing_shards_layers_are_discarded() { diff --git a/crates/levcs-store/src/index.rs b/crates/levcs-store/src/index.rs index 6e7b4ca..2f12996 100644 --- a/crates/levcs-store/src/index.rs +++ b/crates/levcs-store/src/index.rs @@ -1110,6 +1110,44 @@ impl IndexRun { None } + /// Does any entry in this run name `generation` as its logical segment + /// generation? + /// + /// Granted to recovery by contract review 2026-07-30-B, which has the one + /// caller: before recovery seals a journal under a generation other than + /// the identity its frames already carry, it must know whether a published + /// run holds locations against the identity it is about to displace. A run + /// like that stays authoritative through the manifest while resolving to + /// nothing, so recovery refuses instead of opening the store. + /// + /// Read-only and exact rather than a range test. A section's entries store + /// a 16-bit delta from its base, so `[base, base + u16::MAX]` bounds what + /// the section *could* name and skips the sections that could not name it + /// at all — but a section covering the range does not mean an entry in it + /// does, and answering `true` on the range alone would turn recoverable + /// roots into outages for a generation no entry mentions. + pub fn references_segment_generation(&self, generation: u64) -> bool { + for i in 0..self.section_count { + let section = self.section(i); + let Some(delta) = generation.checked_sub(section.base_segment_generation) else { + continue; + }; + if delta > u64::from(u16::MAX) { + continue; + } + for j in 0..section.entry_count { + if self + .entry_location(section.first_entry + j, §ion) + .segment_generation + == generation + { + return true; + } + } + } + false + } + fn find_section(&self, namespace: &NamespaceId) -> Option { let mut lo = 0u64; let mut hi = self.section_count; @@ -1684,6 +1722,49 @@ mod tests { } } + /// Recovery refuses an open on the answer this gives, so a `true` it does + /// not owe is an outage on a healthy root. + /// + /// Both directions, and the one that matters is the negative: generation 5 + /// sits *inside* the packed span of a section based at 4, so a range test + /// over the section header alone would claim it. No entry names it. + #[test] + fn a_run_reports_only_the_segment_generations_its_entries_actually_name() { + let mut delta = IndexDelta::new(1_000, 1 << 20); + for (i, generation) in [4u64, 6, 40].into_iter().enumerate() { + delta + .insert( + IndexKey::new(ns(1), oid(i as u8)), + IndexLocation { + segment_generation: generation, + frame_offset: 0, + frame_len: 1, + object_type: 0, + shard_sequence: i as u64, + }, + ) + .expect("insert"); + } + let bytes = IndexRunBuilder::new(ROOT, 1, 0xFEED) + .build(&delta) + .expect("build"); + let run = IndexRun::from_vec(bytes, &ROOT).expect("open"); + + for named in [4u64, 6, 40] { + assert!( + run.references_segment_generation(named), + "generation {named} is named by an entry in this run" + ); + } + for unnamed in [0u64, 3, 5, 7, 39, 41, u64::MAX] { + assert!( + !run.references_segment_generation(unnamed), + "no entry names generation {unnamed}; claiming it would refuse a \ + recovery that has nothing to lose" + ); + } + } + // -- ceilings ----------------------------------------------------------- #[test] diff --git a/crates/levcs-store/src/recovery.rs b/crates/levcs-store/src/recovery.rs index da1cd9f..e4d7f7c 100644 --- a/crates/levcs-store/src/recovery.rs +++ b/crates/levcs-store/src/recovery.rs @@ -1944,6 +1944,7 @@ fn recover_shard_under_lock( &root_uuid, &header.journal_id, &selection, + &retained_index_runs, config, )?; let from = if checkpointed && checkpoint.active_journal_id == header.journal_id { @@ -2615,11 +2616,22 @@ pub(crate) struct RecoveryGenerations { /// `.recovery--.prefix`. Either fixes the logical generation: /// finishing an interrupted seal must reuse the identity it already chose, not /// pick a fresh one and orphan the artifact. +/// +/// # When the identity is already taken +/// +/// A `.seg` occupies a logical generation whether or not it is readable, so the +/// identity the tail wants can be held by an orphan. The frames must then be +/// renamed, and `published_runs` — the runs the selected manifest names, already +/// open at the one call site — decides what that costs. If one of them holds a +/// location against the displaced identity, recovery refuses with `Corruption` +/// rather than publish a run that resolves to nothing; otherwise it falls back +/// to a free generation and succeeds (contract review 2026-07-30-B). fn recovery_generations_for_journal( paths: &ShardPaths, root_uuid: &[u8; 16], active_journal_id: &[u8; 16], selection: &Option, + published_runs: &[RetainedIndexRun], config: &RecoveryConfig<'_>, ) -> Result { let per_manifest = usize::try_from(config.max_index_runs) @@ -2780,17 +2792,40 @@ fn recovery_generations_for_journal( } // The identity is taken — by an orphan from an interrupted seal, or by a - // segment no manifest references. Recovery must still succeed: an orphan is - // a documented state in which the active journal remains the authority, and - // refusing here would turn a recoverable root into an outage. + // segment no manifest references. Whatever recovery does now, the frames + // cannot keep the name they have, and the two states part here on what that + // costs (contract review 2026-07-30-B). // - // So the seal falls back to a free generation, and the frames are renamed. - // That is the pre-2026-07-30-A behaviour and it carries its hazard with it: - // an index run holding locations against `preferred` no longer resolves. The - // closure is to drop such runs during recovery rather than let a reader find - // nothing through them — recorded in scope §6.5 and not attempted here, - // because it is a change to what recovery *discards* and belongs with the - // checkpointing work that will exercise it. + // If a published run holds locations against `preferred`, displacing the + // frames makes that run authoritative and unresolvable in one step: the + // manifest goes on naming it, `object_source` answers `None` for every + // location in it, and a reader that reaches the run rather than the replay + // delta above it reads nothing and reports nothing. Replay masks it for + // exactly as long as nothing consumes runs directly, which is not a property + // to build a checkpointer on. + // + // So recovery refuses. Not because refusing is good — it is an outage on a + // root whose data is all present — but because the alternative is a store + // that opens and lies. The closure is for recovery to discard a run whose + // covered identity was not preserved, at which point this becomes successful + // reclamation; it is a change to what recovery *reclaims* and belongs with + // the checkpointing work that will exercise it (scope §6.5). + if let Some(retained) = published_runs + .iter() + .find(|retained| retained.run().references_segment_generation(preferred)) + { + return Err(StoreError::Corruption(format!( + "logical generation {preferred} is occupied by a segment this recovery must displace, \ + and published index run {} names it; recovery cannot yet discard a run whose covered \ + identity was not preserved", + retained.path().display() + ))); + } + + // Nothing published depends on the displaced identity, so the seal falls + // back to a free generation and the frames are renamed. An orphan segment is + // a documented state in which the active journal remains the authority, and + // recovery succeeds through it exactly as it did before the split. let logical = occupied_segments .iter() .copied() diff --git a/doc/instance-throughput-rewrite-plan.md b/doc/instance-throughput-rewrite-plan.md index 3fbb830..eb39a62 100644 --- a/doc/instance-throughput-rewrite-plan.md +++ b/doc/instance-throughput-rewrite-plan.md @@ -1500,8 +1500,11 @@ Separated: Collision detection moved with them. The old check compared a resumable generation against a maximum that mixed all three namespaces, which an index run could raise on its own; only a segment can -collide with a segment, so the check is now against segment generations alone, and claiming one that -another journal holds is `Corruption`. +collide with a segment, so occupancy is tracked over `.seg` names alone — and recorded from the +parsed name even when the file does not open, because an unreadable `.seg` holds its name as firmly +as a readable one. Two recovery artifacts for the same journal in different generations remains +`Corruption`. An identity held by some *other* `.seg` is a different question, answered by contract +review 2026-07-30-B below and not by this one. **What this closes.** The scope §6.5 carry-forward "a run may only cover frames a segment already holds" is gone, and with it the restriction in `ShardWriter::coverable_through`: a run may now cover @@ -1517,14 +1520,67 @@ identity the tail wants may be exactly the one it holds. `frame_golden`'s reason, which is also the test that documents why recovery must not refuse in this state: the active journal is still the authority and an outage here would be the wrong answer. The seal therefore falls back to a free generation and renames the frames, exactly as before this review, and an index run -against the old identity dangles. Confined to roots carrying an orphan segment, where it was -previously universal; the closure — recovery discarding runs whose identity was not preserved — is -recorded in scope §6.5 for the checkpointing work. +against the old identity dangles. + +**That last state was disclosed here and rejected on review; 2026-07-30-B below is what landed.** +The disclosure conflated two cases under one orphan: an orphan alone, where the fallback is right and +recovery still succeeds, and an orphan holding an identity a *published* run names, where the +fallback opens a store whose manifest points at a run that resolves to nothing. This review's own +finding — that replay masks it — was the argument against leaving it, since `StoreEngine::checkpoint` +consumes runs directly. The other §6.5 carry-forward stands: sealing still moves no frame out of `active/`, so the replay ceiling still bounds admission and an entry-pressure seal still lands on it. That is what `StoreEngine::checkpoint` is for, and it is now unblocked. +##### Contract review 2026-07-30-B + +**A displaced identity is a refusal, not a disclosure.** Review of 2026-07-30-A declined the silent +state it disclosed and chose `Corruption` for orphan-plus-published-run until recovery can discard +such a run. Granted, with one frozen seam, and landed before the checkpointing dispatch it would +otherwise have poisoned. + +**What the two states cost, which is why they part.** When a `.seg` occupies the logical generation +the active tail carries, the frames cannot keep their name whatever recovery does. + +- *Orphan alone.* Nothing persisted names the displaced identity, so renaming the frames costs + nothing. Recovery falls back to a free generation and **succeeds**, which is what + `an_orphan_segment_leaves_the_active_journal_the_authority` requires: the active journal is still + the authority and an outage would be the wrong answer. +- *Orphan plus a run the manifest names.* Recovery would publish a manifest naming a run whose every + location resolves to nothing — authoritative and unreadable in one step. Recovery **refuses** with + `Corruption`, naming the run file rather than only the generation. + +Refusing is not good; it is an outage on a root whose data is all present. It is chosen because the +alternative is a store that opens and lies, and because the masking is temporary in the worst way: +the replay delta sits above the run and answers every lookup that would otherwise expose it, so the +first consumer to read a run directly — the checkpointer — is also the first to find out. + +**The frozen seam.** `IndexRun::references_segment_generation` in `index.rs` (A2), read-only, with +recovery as its one caller. Exact rather than a range test over section headers: entries pack a +16-bit delta from the section base, so `[base, base + u16::MAX]` says only what a section *could* +name, and answering `true` on that alone would refuse recoveries over a generation no entry +mentions. `index.rs` is otherwise untouched. + +**`coverable_through` keeps its widened coverage.** The restriction 2026-07-30-A removed does not +come back: a run may cover locations naming the active tail, because the unsound state that +restriction existed to avoid is now refused at the one point it can arise rather than designed +around at every seal. Its doc comment described the pre-split world and is rewritten to this one. + +**Evidence.** `an_orphan_holding_a_published_runs_identity_refuses_the_open` and +`an_orphan_holding_no_published_identity_still_opens` are the two states, and the refusing one +asserts the damage rather than an expectation: with the guard disabled the open succeeds and the +reopened root pins `None` at the generation the run names. The exactness test is +`a_run_reports_only_the_segment_generations_its_entries_actually_name`, whose negative cases include +a generation inside a section's packed span that no entry uses. A first draft of the refusing test +passed for the wrong reason — its workload re-pushed the genesis object id as a blob, so the reopen +failed on a duplicate-object `Conflict` whether or not the guard existed; the mutation is what +exposed it. + +**Still open, unchanged.** Recovery discarding a run whose covered identity was not preserved, which +turns this refusal into successful reclamation. Recorded in scope §6.5 with the checkpointing work +that will exercise it. + ##### Contract review 2026-07-28-C B4 re-pointed `store-bench` at a real `StoreEngine::submit` and found that four verification diff --git a/doc/phase1-storage-spine-scope.md b/doc/phase1-storage-spine-scope.md index 7876ef6..2aaf66b 100644 --- a/doc/phase1-storage-spine-scope.md +++ b/doc/phase1-storage-spine-scope.md @@ -1696,15 +1696,23 @@ sealed segment inherits the **tail's** logical generation, so moving frames from next free **manifest** generation, so an index run's manifest cannot collide with a recovery's. Sealing therefore covers current frames, in the session that wrote them. -**Residual, one case wide.** If a `.seg` already occupies the tail's identity — an orphan from an -interrupted seal, or a segment no manifest references — the seal cannot use that name and falls back -to a free generation, renaming the frames. An index run holding locations against the old identity -then resolves to nothing. Recovery must not refuse here: an orphan segment is a documented state in -which the active journal stays the authority, and refusing would turn a recoverable root into an -outage. The closure is for recovery to **discard index runs whose covered identity was not -preserved**, which is a change to what recovery reclaims and belongs with the checkpointing work that -will exercise it. Until then the hazard is confined to roots carrying an orphan segment, where it was -previously universal. +**Residual, one case wide, and it refuses rather than proceeds.** If a `.seg` already occupies the +tail's identity — an orphan from an interrupted seal, or a segment no manifest references — the seal +cannot use that name and the frames must be renamed. Contract review 2026-07-30-B splits what that +costs: + +- **Nothing published names the displaced identity.** Recovery falls back to a free generation and + succeeds. An orphan segment is a documented state in which the active journal stays the authority, + and refusing here would turn a recoverable root into an outage. +- **An index run the manifest names holds locations against it.** Recovery refuses the open with + `Corruption`, naming the run. Proceeding would publish a manifest whose run is authoritative and + resolves to nothing; the replay delta above it hides that from every lookup until the first + consumer that reads runs directly, which is the checkpointer. + +The closure is for recovery to **discard an index run whose covered identity was not preserved**, +which turns the refusal into successful reclamation. It is a change to what recovery reclaims and +belongs with the checkpointing work that will exercise it. Until then a root carrying both an orphan +segment and a run against the identity it holds does not open. ### 6.5 B3 — StagingSessions