neuwirth
/
LeVCS
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Keep a surviving journal on the identity the manifest implies 

The sealing fallback was reaching a journal that seals nothing. A recovery
keeping an empty tail gave it the fallback's generation, but no TailRange
recorded the move — nothing was sealed — so the next open derived the identity
the manifest still implied while the session in between had appended frames
under the moved one and sealed a run over them. The guard searched published
runs for the derived generation, found none, and let the store open with the
run unresolvable. Review confirmed it: empty tail plus an orphan at 1, reopen
taking 2, frames and a run at 2, an orphan at 2, then an open at 3 with
`object_source(0, 2) == None`.

`RecoveryGenerations` carries a third number. `tail` is what a surviving
journal keeps and is always the tail's own identity: segment-name occupancy is
a fact about `segments/`, and a journal that seals nothing does not go there.
`logical` remains the sealing identity and still falls back under the guard.

That restores the invariant the split rests on — the active tail's identity is
always derivable from the manifest — so the guard and the frames are always
naming the same generation. A tail may keep a generation an orphan occupies;
the collision is only real when frames are sealed under that name, and it is
refused then, with the run in hand.

The regression follows the reported sequence and reads the run's generation out
of the run rather than assuming it, since which identity the frames ended up
with is the thing under test. Mutation-checked in both places it can fail: the
run drifts to generation 2, and with the mid-assertion relaxed the open
succeeds with None pinned there, reproducing the report exactly. The shared
helper now reports the run's own generation, so all three refusal paths fail
legibly rather than against a hard-coded number.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XKzM69CHmBuDcA3qN1jFdh
This commit is contained in:
Levi Neuwirth 2026-07-30 17:12:16 -04:00
parent 851927289c
commit 0d2fd6d986
4 changed files with 159 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
crates/levcs-store/src/engine.rs
View File

@ -6551,8 +6551,8 @@ mod index_maintenance_tests {
/// carry owes the same refusal, so the tests assert it through one function
/// rather than through copies that can drift apart the way the code did.
fn assert_the_open_refuses_naming_the_run(options: StoreOptions, root: &Path) {
let runs = manifest_runs(root, 0, root_uuid_of(root));
assert_eq!(runs.len(), 1, "one published run is the whole premise");
let (run, named) = the_published_runs_generation(root);
let runs = [run];
match StoreEngine::open(options) {
Err(StoreError::Corruption(message)) => assert!(
@ -6565,16 +6565,36 @@ mod index_maintenance_tests {
// the refusal reports what it costs rather than a bare expectation.
Ok(opened) => {
let committed = opened.committed_root();
let pinned = committed.object_source(0, 1).expect("resolve generation 1");
let pinned = committed.object_source(0, named).expect("resolve");
panic!(
"the open succeeded with a published run naming generation 1, and the \
"the open succeeded; published run {} names generation {named} and the \
reopened root pins {pinned:?} there — every lookup reaching the run \
rather than the replay delta above it reads nothing"
rather than the replay delta above it reads nothing",
runs[0]
);
}
}
}
/// The generation a published run's entries name — discovered, not assumed.
///
/// Which identity the frames ended up with is the thing under test, so a
/// test that hard-codes it reports the number it expected rather than the
/// one the store chose. That is precisely how the fallback leaking onto a
/// surviving journal stayed invisible.
fn the_published_runs_generation(root: &Path) -> (String, u64) {
let uuid = root_uuid_of(root);
let runs = manifest_runs(root, 0, uuid);
assert_eq!(runs.len(), 1, "one published run is the whole premise");
let run =
crate::index::IndexRun::open(&shard_paths(root, 0).indexes().join(&runs[0]), &uuid)
.expect("open the published run");
let named = (0..16u64)
.find(|generation| run.references_segment_generation(*generation))
.expect("a run with entries names some generation");
(runs[0].clone(), named)
}
/// 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
@ -6644,6 +6664,59 @@ mod index_maintenance_tests {
assert_the_open_refuses_naming_the_run(configure(), temporary.path());
}
/// The identity a surviving journal is given must be one the next open can
/// derive, or the guard searches for a generation nothing carries.
///
/// Review's sequence. An empty journal seals nothing, so a fallback applied
/// to it moved an identity that no `TailRange` recorded — the session went
/// on to write frames and seal a run under the moved number while the next
/// open recomputed the old one. The guard then looked for the wrong
/// generation, found no run naming it, and let the store open with the run
/// unresolvable.
///
/// The assertion is deliberately about what the *store* names things: the
/// run's generation is read out of the run rather than assumed, so this
/// fails the same way whether the identity drifts by one or by ten.
#[test]
fn a_surviving_journal_keeps_an_identity_the_next_open_can_derive() {
let serial = writer_serial();
let temporary = tempfile::tempdir().expect("tempdir");
let namespace = NamespaceId([0x4E; 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
};
// An empty tail, and an orphan on the identity it carries.
drop(StoreEngine::open(configure()).expect("open a fresh root"));
orphan_segment_at(temporary.path(), 0, 1);
// The session that keeps that journal writes frames into it and seals a
// run over them. Whatever identity it kept, the run now names it.
{
let engine = StoreEngine::open(configure()).expect("a surviving empty tail opens");
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");
}
let (_, named) = the_published_runs_generation(temporary.path());
assert_eq!(
named, 1,
"a journal that seals nothing keeps the identity the manifest implies; \
a run naming anything else is an identity the next open cannot derive"
);
// Now occupy the identity the frames carry, so the next open must
// displace them — and owes the refusal for the run that names them.
orphan_segment_at(temporary.path(), 0, 2);
assert_the_open_refuses_naming_the_run(configure(), temporary.path());
}
/// Resumption itself is not the hazard, and a guard that treated it as one
/// would refuse every interrupted recovery on a root that has ever sealed.
///

57
crates/levcs-store/src/recovery.rs
View File

@ -1938,6 +1938,7 @@ fn recover_shard_under_lock(
ActiveJournalDisposition::Replay => {
let RecoveryGenerations {
logical: logical_generation,
tail: tail_generation,
manifest: manifest_generation,
} = recovery_generations_for_journal(
&paths,
@ -2114,17 +2115,28 @@ fn recover_shard_under_lock(
complete_interrupted_seal(&path, &paths, &counters)?;
must_create_fresh = true;
} else {
// This journal survives; nothing is sealed and nothing is
// renamed, so it keeps the identity the manifest implies —
// `tail_generation`, not the sealing fallback beside it.
//
// Taking `logical_generation` here was a real defect and a
// quiet one: a fallback would move the tail's identity while
// no `TailRange` recorded the move, so the next open derived
// the old number and the guard above searched published runs
// for a generation the frames no longer carried. An
// identity nothing persists is not an identity (contract
// review 2026-07-30-B).
let shared_file = Arc::new(file);
let retained_tail =
PinnedFile::from_shared(path.clone(), Arc::clone(&shared_file));
recovered_tail = Some(RecoveredTail {
logical_generation,
logical_generation: tail_generation,
journal_id: header.journal_id,
path,
validated_through: JOURNAL_HEADER_LEN as u64,
file: shared_file,
});
retained_tails.push(RetainedTail::new(logical_generation, retained_tail));
retained_tails.push(RetainedTail::new(tail_generation, retained_tail));
}
}
}
@ -2579,21 +2591,35 @@ fn apply_recovered_refs(
Ok(())
}
/// The two generations a recovery needs, which are not the same number.
/// The generations a recovery needs, which are not the same number.
///
/// Contract review 2026-07-30-A. They were one, and the conflation was invisible
/// for as long as nothing outlived a session holding an `IndexLocation`.
/// Contract review 2026-07-30-A separated the first two; 2026-07-30-B separated
/// `tail` from `logical` after review found a fallback leaking onto a journal
/// that seals nothing.
pub(crate) struct RecoveryGenerations {
/// The **logical** generation of the frames: the identity an `IndexLocation`
/// names, inherited from the active tail so that sealing it into a segment
/// moves the bytes without changing what they are called.
/// The **logical** generation of frames this recovery seals into a segment:
/// the identity an `IndexLocation` names, inherited from the active tail so
/// that sealing moves the bytes without changing what they are called. It
/// is the tail's identity unless a `.seg` already occupies that name, in
/// which case the frames are renamed and the guard decides whether that is
/// affordable.
logical: u64,
/// The identity a **surviving** journal keeps, always the tail's own.
///
/// Always, because segment-name occupancy is a fact about `segments/` and a
/// journal that seals nothing does not go there. Letting the fallback reach
/// this number is how an unrecorded identity appeared: nothing sealed, so
/// no `TailRange` recorded the moved value, and the next open recomputed
/// the identity the manifest still implied — while the frames this session
/// went on to append, and the run that sealed them, carried the moved one.
/// The guard then searched for the wrong generation and found nothing.
tail: u64,
/// The generation of the manifest this recovery installs. A counter over
/// manifests, which an index run's manifest also advances.
manifest: u64,
}
/// Choose both.
/// Choose all three.
///
/// # Why they had to be separated
///
@ -2831,13 +2857,18 @@ fn recovery_generations_for_journal(
// no-op on it.
if let Some(logical) = resumable.into_iter().next() {
refuse_if_displacement_strands_a_run(logical)?;
return Ok(RecoveryGenerations { logical, manifest });
return Ok(RecoveryGenerations {
logical,
tail: preferred,
manifest,
});
}
// Otherwise the frames keep the name they already have.
if !occupied_segments.contains(&preferred) {
return Ok(RecoveryGenerations {
logical: preferred,
tail: preferred,
manifest,
});
}
@ -2859,7 +2890,11 @@ fn recovery_generations_for_journal(
StoreError::Corruption("no logical generation remains for a recovery segment".into())
})?;
refuse_if_displacement_strands_a_run(logical)?;
Ok(RecoveryGenerations { logical, manifest })
Ok(RecoveryGenerations {
logical,
tail: preferred,
manifest,
})
}
/// The logical generation of the active tail above a manifest's committed prefix.

32
doc/instance-throughput-rewrite-plan.md
View File

@ -1582,12 +1582,36 @@ that keyed on "a resumable artifact exists" would fail rather than quietly refus
recovery on a root that has ever sealed. The lesson generalizes past this fix: the check belongs on
the *outcome* — the frames are being renamed — not on the branch that produced it.
**Second amendment: the fallback was reaching a journal that seals nothing.** Review found the guard
searching for the wrong generation entirely. A recovery that keeps an *empty* journal was giving it
the sealing fallback's identity — but nothing was sealed, so no `TailRange` recorded the move, and
the next open derived the identity the manifest still implied. The session in between had appended
frames under the moved number and sealed a run over them. The guard looked for a run naming the
derived generation, found none, and opened a store whose run resolved to nothing. Confirmed by
review: empty tail plus an orphan at 1, reopen taking 2, frames and a run at 2, an orphan at 2, and
the next open choosing 3 with `object_source(0, 2) == None`.
`RecoveryGenerations` now carries **three** numbers, and the third is the point: `tail` is the
identity a surviving journal keeps and is always the tail's own. Segment-name occupancy is a fact
about `segments/`, and a journal that seals nothing does not go there — the fallback exists to avoid
a name collision that path never risks. `logical` remains the sealing identity and still falls back
under the guard. The invariant this restores is the one 2026-07-30-A was built on and did not fully
hold: **the active tail's identity is always derivable from the manifest**, so the guard and the
frames are always talking about the same number.
What the tail keeps can be a generation an orphan already occupies, and that is correct: the
collision is only real when frames are sealed under that name, and it is refused then, by the guard,
with the run in hand. An identity nothing persists is not an identity.
**Evidence.** `an_orphan_holding_a_published_runs_identity_refuses_the_open`,
`a_surviving_journal_keeps_an_identity_the_next_open_can_derive`,
`a_resumed_fallback_refuses_on_the_identity_it_resumes`, and
`an_orphan_holding_no_published_identity_still_opens` are the states, and both refusing tests share
one assertion helper so the two paths cannot drift in the tests either. It asserts the damage rather
than an expectation: with either guard call disabled the open succeeds and the reopened root pins
`None` at the generation the run names. The exactness test is
`an_orphan_holding_no_published_identity_still_opens` are the states, and every refusing test shares
one assertion helper so the paths cannot drift in the tests either. It asserts the damage rather than
an expectation, and it **reads the generation out of the published run** rather than assuming one —
hard-coding it is how a drifting identity would report the number the test expected instead of the
number the store chose. With any of the three changes reverted the open succeeds and the helper
reports the run's own generation with `None` pinned at it. 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

7
doc/phase1-storage-spine-scope.md
View File

@ -1713,6 +1713,13 @@ The test is what the recovery *names its frames*, not how it got there: an inter
resuming a fallback an earlier session chose strands the same run, and takes the same refusal. A
resumed seal at the identity the frames already carry displaces nothing and opens normally.
The fallback applies only to frames being **sealed**. A journal that survives recovery keeps the
identity the manifest implies even when a `.seg` occupies that name, because nothing is being written
to `segments/` and there is no collision yet. This is what keeps the active tail's identity derivable
from the manifest across every open — the property the split rests on. Giving a surviving journal a
fallback identity moved a number nothing recorded, and the next open, deriving the old one, searched
published runs for a generation the frames no longer carried.
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