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

Guard the identity a recovery chooses, not the branch it chose it in 

The displaced-run refusal sat only on the fresh fallback. The resumable branch
returned before it, so an interrupted recovery that had already fallen back to
generation 2 resumed at 2 — correctly, since finishing an interrupted seal must
not orphan its artifact — and stranded a published run naming generation 1
exactly as a fresh fallback would, one crash later. Review reproduced it with a
run at 1, an orphan at 1, and a `.recovery-<journal>-2.prefix`: the store opened
with `object_source(0, 1) == None`.

`preferred` is now computed before either path can return, and both call one
closure keyed on choosing anything other than the identity the frames already
carry. A resumable artifact at `preferred` displaces nothing and the guard is a
no-op on it, which is its own test — a guard keyed on "a resumable artifact
exists" would refuse every interrupted recovery on a root that has ever sealed,
and that test fails rather than letting the over-broad version pass.

Both refusing tests share one assertion helper, so the paths cannot drift in the
tests either. Each guard call is separately mutation-checked: disabling the
resumed one reproduces the reported open with the run's source unpinned.

Contract review 2026-07-30-B amended with the finding and the general form of
it — the check belongs on the outcome, that the frames are being renamed, not
on the branch that produced the rename.

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 16:23:58 -04:00
parent 4aa610dc8b
commit 851927289c
4 changed files with 218 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -6503,14 +6503,8 @@ mod index_maintenance_tests {
/// 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,
active_journal_path(&paths),
paths
.segments()
.join(crate::segment::segment_filename(generation, 0, 9)),
@ -6518,6 +6512,69 @@ mod index_maintenance_tests {
.expect("link the orphan");
}
fn active_journal_path(paths: &crate::segment::ShardPaths) -> std::path::PathBuf {
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")
}
/// A zero-length `.recovery-<journal>-<generation>.prefix`.
///
/// An interrupted recovery that crashed just after creating its
/// construction artifact, which is the state that fixes the identity a
/// resumed recovery must reuse. Zero-length is a valid one: the seal
/// compares the bytes that exist — none — and copies the rest.
fn resumable_prefix_at(root: &Path, shard: u16, generation: u64) {
use std::io::Read;
let paths = shard_paths(root, shard);
let mut bytes = [0u8; crate::format::JOURNAL_HEADER_LEN];
std::fs::File::open(active_journal_path(&paths))
.expect("open the journal")
.read_exact(&mut bytes)
.expect("read its header");
let header = crate::format::JournalHeader::decode(&bytes).expect("decode its header");
std::fs::write(
paths.segments().join(format!(
".recovery-{}-{generation}.prefix",
hex::encode(header.journal_id)
)),
[],
)
.expect("write the interrupted artifact");
}
/// Every path that names the frames something other than the identity they
/// 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");
match StoreEngine::open(options) {
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 committed = opened.committed_root();
let pinned = committed.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"
);
}
}
}
/// 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
@ -6549,30 +6606,79 @@ mod index_maintenance_tests {
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");
assert_the_open_refuses_naming_the_run(configure(), temporary.path());
}
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 same displacement, one crash later, which is the case the first
/// version of this guard let through.
///
/// A previous recovery already fell back to generation 2 and left its
/// construction artifact behind. Resuming reuses that identity — correctly,
/// since finishing an interrupted seal must not orphan the artifact — but
/// the identity it reuses is still not the one the frames carry, so the
/// published run naming generation 1 is stranded exactly as it would be by a
/// fresh fallback. Resumption is a reason to keep a choice, not a reason to
/// skip the check on it.
#[test]
fn a_resumed_fallback_refuses_on_the_identity_it_resumes() {
let serial = writer_serial();
let temporary = tempfile::tempdir().expect("tempdir");
let namespace = NamespaceId([0x4C; 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);
resumable_prefix_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.
///
/// Here the interrupted seal had chosen the identity the frames already
/// carry, so resuming it displaces nothing and the published run resolves
/// through the segment the resumed seal installs.
#[test]
fn a_resumed_seal_at_the_frames_own_identity_still_opens() {
let serial = writer_serial();
let temporary = tempfile::tempdir().expect("tempdir");
let namespace = NamespaceId([0x4D; 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");
resumable_prefix_at(temporary.path(), 0, 1);
}
let engine = StoreEngine::open(configure()).expect("resuming its own identity must open");
let root = engine.committed_root();
assert!(
root.object_source(0, 1)
.expect("resolve generation 1")
.is_some(),
"the resumed seal kept the identity the run names, so it must resolve"
);
}
/// The other half, and the reason the refusal is conditioned on the run

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

@ -2626,6 +2626,11 @@ pub(crate) struct RecoveryGenerations {
/// 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).
///
/// **Resumption is not exempt.** The identity a resumable artifact carries may
/// itself be a fallback an earlier session chose, so it strands the same run one
/// crash later. Both paths take the guard, keyed on choosing anything other than
/// the identity the frames already have.
fn recovery_generations_for_journal(
paths: &ShardPaths,
root_uuid: &[u8; 16],
@ -2777,13 +2782,59 @@ fn recovery_generations_for_journal(
StoreError::Corruption("no generation remains for recovery manifests".into())
})?;
// The identity the frames already have, computed before either path can
// choose against it. It is what every decision below is measured from.
let preferred = active_tail_logical_generation(selection);
// Contract review 2026-07-30-B, and the reason it is a closure rather than
// two copies: displacement can be decided here or in a session that crashed,
// and the two paths must not drift.
//
// Naming the frames anything but `preferred` is what breaks a published run
// holding locations against `preferred` — the manifest goes on naming the
// run, `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 that 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).
let refuse_if_displacement_strands_a_run = |chosen: u64| -> Result<(), StoreError> {
if chosen == preferred {
return Ok(());
}
match published_runs
.iter()
.find(|retained| retained.run().references_segment_generation(preferred))
{
Some(retained) => Err(StoreError::Corruption(format!(
"this recovery must name its frames generation {chosen} rather than {preferred}, \
and published index run {} holds locations against {preferred}; recovery cannot \
yet discard a run whose covered identity was not preserved",
retained.path().display()
))),
None => Ok(()),
}
};
// Resuming: the identity was already chosen and the artifact carries it.
//
// The choice is not therefore safe. An interrupted recovery that had already
// fallen back carries the same displacement one crash later, and resuming it
// without the guard was how a store with an unresolvable run still opened. A
// resumable artifact *at* `preferred` displaces nothing and the guard is a
// no-op on it.
if let Some(logical) = resumable.into_iter().next() {
refuse_if_displacement_strands_a_run(logical)?;
return Ok(RecoveryGenerations { logical, manifest });
}
// Otherwise the frames keep the name they already have.
let preferred = active_tail_logical_generation(selection);
if !occupied_segments.contains(&preferred) {
return Ok(RecoveryGenerations {
logical: preferred,
@ -2792,40 +2843,11 @@ fn recovery_generations_for_journal(
}
// The identity is taken — by an orphan from an interrupted seal, or by a
// 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).
//
// 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.
// segment no manifest references — so the frames are renamed to a free
// generation and the guard decides whether that is affordable. An orphan
// segment alone 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()
@ -2836,6 +2858,7 @@ fn recovery_generations_for_journal(
.ok_or_else(|| {
StoreError::Corruption("no logical generation remains for a recovery segment".into())
})?;
refuse_if_displacement_strands_a_run(logical)?;
Ok(RecoveryGenerations { logical, manifest })
}

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

@ -1567,10 +1567,27 @@ come back: a run may cover locations naming the active tail, because the unsound
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
**Amendment, on review of the first landing: resumption is not exempt.** The guard sat only on the
fresh fallback, and the resumable branch returned before it. An interrupted recovery that had already
fallen back to generation 2 resumed at 2 — correctly, since finishing an interrupted seal must not
orphan its artifact — and stranded the run naming generation 1 exactly as a fresh fallback would,
one crash later. Reproduced by review with a published run, an orphan at 1, and a
`.recovery-<journal>-2.prefix`; the store opened with `object_source(0, 1) == None`.
The guard is now one closure both paths call, keyed on **choosing anything other than the identity
the frames already carry** rather than on how the choice was reached. `preferred` is computed before
either path can return. A resumable artifact *at* `preferred` displaces nothing and the guard is a
no-op on it — pinned by `a_resumed_seal_at_the_frames_own_identity_still_opens`, so a future guard
that keyed on "a resumable artifact exists" would fail rather than quietly refuse every interrupted
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.
**Evidence.** `an_orphan_holding_a_published_runs_identity_refuses_the_open`,
`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
`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

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

@ -1709,6 +1709,10 @@ costs:
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 test is what the recovery *names its frames*, not how it got there: an interrupted recovery
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 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