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

Read the run's identity from an entry, not from a guessed range 

The test oracle probed generations 0..16 to discover what a published run
names, which reproduced the defect it was written to catch: it could not report
an identity outside the range it guessed. Review drifted the identity to 101 and
the test died on "a run with entries names some generation" before reaching
either the identity assertion or the damage assertion. It would also have picked
the lowest of several once a run spans generations.

The helper now takes an IndexKey the test knows is covered and reads
`segment_generation` off `IndexRun::get`, which answers exactly and has no
range to outgrow. It fails loudly if the key is absent, since a missing entry
means the test is measuring nothing.

The surviving-journal regression keeps a second orphan at generation 100, so the
fallback it must not take is 101 — a number that cannot be read as an
off-by-one. Verified by reverting the tail fix under it: the assertion now
reports left: 101, right: 1 instead of failing to find a generation at all.

No production change.

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 18:34:36 -04:00
parent 0d2fd6d986
commit dccc1859b0
2 changed files with 60 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -6550,8 +6550,12 @@ mod index_maintenance_tests {
/// 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 (run, named) = the_published_runs_generation(root);
fn assert_the_open_refuses_naming_the_run(
options: StoreOptions,
root: &Path,
covered: IndexKey,
) {
let (run, named) = the_published_runs_generation(root, covered);
let runs = [run];
match StoreEngine::open(options) {
@ -6576,23 +6580,34 @@ mod index_maintenance_tests {
}
}
/// The generation a published run's entries name — discovered, not assumed.
/// The generation the published run names for `covered` — read out of the
/// entry rather than assumed, and rather than searched for.
///
/// 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) {
/// test that hard-codes it reports the number it expected instead of the one
/// the store chose. That is how the fallback leaking onto a surviving
/// journal stayed invisible.
///
/// Probing generations in a range was the first attempt and had the same
/// defect one level up: it could not report an identity outside the range it
/// guessed, so a drift past it failed as "no generation found" before any
/// assertion about the identity ran. It would also pick the lowest of
/// several once a run spans generations. A named entry answers exactly.
fn the_published_runs_generation(root: &Path, covered: IndexKey) -> (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)
let location = run.get(&covered).unwrap_or_else(|| {
panic!(
"{} is not in the published run, so this test is not measuring the identity \
of anything",
covered.object.to_hex()
)
});
(runs[0].clone(), location.segment_generation)
}
/// Contract review 2026-07-30-B: the two states an occupied identity leaves.
@ -6626,7 +6641,11 @@ mod index_maintenance_tests {
orphan_segment_at(temporary.path(), 0, 1);
}
assert_the_open_refuses_naming_the_run(configure(), temporary.path());
assert_the_open_refuses_naming_the_run(
configure(),
temporary.path(),
IndexKey::new(namespace, ObjectId([0x60; 32])),
);
}
/// The same displacement, one crash later, which is the case the first
@ -6661,7 +6680,11 @@ mod index_maintenance_tests {
resumable_prefix_at(temporary.path(), 0, 2);
}
assert_the_open_refuses_naming_the_run(configure(), temporary.path());
assert_the_open_refuses_naming_the_run(
configure(),
temporary.path(),
IndexKey::new(namespace, ObjectId([0x60; 32])),
);
}
/// The identity a surviving journal is given must be one the next open can
@ -6676,7 +6699,10 @@ mod index_maintenance_tests {
///
/// 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.
/// fails the same way whether the identity drifts by one or by a hundred.
/// The distant orphan is there to make the difference unmistakable — a
/// fallback reaching the surviving journal yields 101, which cannot be read
/// as an off-by-one.
#[test]
fn a_surviving_journal_keeps_an_identity_the_next_open_can_derive() {
let serial = writer_serial();
@ -6689,9 +6715,11 @@ mod index_maintenance_tests {
options
};
// An empty tail, and an orphan on the identity it carries.
// An empty tail, an orphan on the identity it carries, and a second one
// far away so the fallback this must not take is a distinctive number.
drop(StoreEngine::open(configure()).expect("open a fresh root"));
orphan_segment_at(temporary.path(), 0, 1);
orphan_segment_at(temporary.path(), 0, 100);
// 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.
@ -6704,7 +6732,10 @@ mod index_maintenance_tests {
assert_eq!(engine.index_maintenance().sealed_runs, 1, "the seal ran");
}
let (_, named) = the_published_runs_generation(temporary.path());
let (_, named) = the_published_runs_generation(
temporary.path(),
IndexKey::new(namespace, ObjectId([0x60; 32])),
);
assert_eq!(
named, 1,
"a journal that seals nothing keeps the identity the manifest implies; \
@ -6714,7 +6745,11 @@ mod index_maintenance_tests {
// 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());
assert_the_open_refuses_naming_the_run(
configure(),
temporary.path(),
IndexKey::new(namespace, ObjectId([0x60; 32])),
);
}
/// Resumption itself is not the hazard, and a guard that treated it as one

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

@ -1611,7 +1611,14 @@ one assertion helper so the paths cannot drift in the tests either. It asserts t
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
reports the run's own generation with `None` pinned at it.
The helper reads that generation from a **named entry**, `IndexRun::get` on a key the test knows is
covered. Its first version probed generations `0..16` and had the original defect one level up: it
could not report an identity outside the range it guessed, so review's drift to generation 101 failed
as "no generation found" before any assertion about the identity ran, and a run spanning generations
would have reported the lowest. The regression now keeps a distant orphan precisely so the fallback
it must not take is 101 rather than a number that reads as an off-by-one. 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