Repository navigation
Conversation
…x directory The collector deletes a swept version's files before its manifest, so a pass that stops in between leaves the manifest for the next pass. On a local store that next pass failed for the table: `delete_paths` called `remove_dir_all` again on the `_indices/<uuid>` directory the stopped pass had removed, and Lance's local provider reports an absent directory as `NotFound`. The table kept the error and deleted nothing on every later run. Object stores list the prefix empty and succeed, so S3, Azure and the DST memory store converged already. An absent index directory is now already deleted, as an absent file already was. The files-before-manifest order is unchanged, and the deletion set still holds only paths no retained root marks. The failpoints test stops a pass at `cleanup.sweep_pre_manifest_delete` after it removed an index directory only the pruned pin names, then proves the next pass removes that manifest, reports no error, and leaves every retained root's paths in place.
Reword the maintenance guide's sentence on an interrupted cleanup so the deletion order and the next run's outcome read as two steps, and point the release note at the guide's Cleanup section.
…staging manifests Cover the directory-touching windows of the collector's sweep on a local store, where Lance reports removing an absent directory as NotFound: - a pass stopped between two manifests of the published sweep, with an index directory among the files it already deleted; - a pass stopped after the dead stagings' files and before their manifests, through the new seam cleanup.sweep_pre_dead_manifest_delete. Each retry reports no error, removes the remaining manifests and keeps every retained root's paths.
…anup-sweep-crash-windows # Conflicts: # crates/omnigraph/tests/failpoints.rs
Register cleanup.sweep_pre_dead_manifest_delete in the crash-window catalog (appended, 53 windows) and give it a milestone recipe: insert, index build, insert, cleanup. The reach census's setup crash stops the index build before its manifest commit, stranding a detached staging; the second insert moves main's head past the head the staging recorded, so the recipe's cleanup judges it dead and crosses the window. A window inside cleanup's per-table pass does not fail the op: cleanup records the table's error and returns Ok. The harness treated that as a plain success, so no retry was judged and the report kept no trace of the window. crash_op now returns CleanupDeferred when the op's deferrals name the scheduled window; the universe records deferred:<window>@op<i> and runs the obligation pass a crashed cleanup gets (a clean rerun that defers nothing, the collector oracle, the world read). This covers cleanup.table_gc and the three earlier sweep windows as well. The pin dst_dead_staging_sweep_window_is_reached_and_retried probes the window and stops the recipe's cleanup at it; the sensitivity proof gains a deferred-cleanup case. Both go red with the routing reverted.
Review of
|
| Command | Before-fix variant | Head fbba88d1 |
|---|---|---|
cargo test --locked --test scenarios -- --exact dst_dead_staging_sweep_window_is_reached_and_retried dst_sensitivity_maintenance_rerun_failure_is_red |
0 passed, 2 failed (reasons below) | 2 passed |
cargo test --locked (whole crate) |
not run | lib 53 passed; integration 2 passed, 3 ignored; scenarios 52 passed, 28 ignored |
dst_window_reach_probe --ignored --nocapture |
not run | 41/53 crossed, including cleanup.sweep_pre_dead_manifest_delete; the 12 dark windows match the PR list; floor assert panics with 41 crossed (finding 6) |
cargo clippy -p omnigraph-dst --all-targets --locked -- -D warnings -W clippy::dbg_macro |
not run | clean |
cargo fmt --all --check; python3 scripts/check-docs.py |
not run | clean; Documentation OK (308 Markdown files checked) |
Why the two tests fail on the before-fix variant. Both are the predicted behavioral reasons, not setup failures:
- The pin fails with
the stopped cleanup must leave the table owed: [("crash:ensure_indices.post_phase_b_pre_manifest_commit@op4", "Applied", "query")]. Only the setup crash has a verdict. The stop at op 8 resolved as a plain success. - The sensitivity test fails with
a failing rerun MUST redden the universe. Thedeferred-cleanupcase stayed green. The four earlier cases are unchanged and passed first.
CI at fbba88d1 (merge ref onto 0b5e17fa): every check passed. DST pinned suite ran dst_dead_staging_sweep_window_is_reached_and_retried ... ok and dst_sensitivity_maintenance_rerun_failure_is_red ... ok, with the same crate counts as my local run.
Limits
- I did not run
dst_hunt_crash_window_sweepor the fleet. The statement that the eighttable_gcand sweep universes stay green on the new route comes from the PR description, not from my run. typosis not installed on my host. For that check I rely on CI.- The file-backend
NotFoundpath is outside DST. test(cleanup): interrupt the sweep between manifests and before dead-staging manifests #1027'sfailpoints.rscovers it, and test(cleanup): run the sweep crash windows on S3 and Azure #1033 adds S3 and Azure. I did not run those suites.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| })); | ||
| // A cleanup the window left owing a table is judged like a crashed one. | ||
| cases.push(( | ||
| "deferred-cleanup".to_string(), |
There was a problem hiding this comment.
Nit: if this case stays green, the panic below (line 1999) says only s20 sensitivity: a failing rerun MUST redden the universe. It does not name the case. In my before-fix run (no deferral route), I had to infer deferred-cleanup from the list order.
Suggested: panic!("s20 sensitivity [{case}]: a failing rerun MUST redden the universe").
| .lock() | ||
| .expect("cleanup deferral map") | ||
| .get(db.uri()) | ||
| .is_some_and(|deferred| deferred.values().any(|error| error.contains(&injected))) |
There was a problem hiding this comment.
Optional, can be a follow-up: this route sees only the errors that cleanup puts in its returned rows.
Two cleanup windows fail outside the rows:
cleanup.resolve_branch_snapshot(optimize.rs,reconcile_frozen_native_forks, lines 1378–1384)cleanup.reconcile_fork(optimize.rs, lines 1480–1494)
Their failures go to reconciled.failures, and cleanup_detached_only only logs that list with tracing::warn! (lines 915–921). So a stop scheduled at either window still resolves as OpSucceeded, with no verdict and no retry obligation. The census crosses cleanup.reconcile_fork.
That work is not owed table-version work ("will retry next cleanup", and safety holds), so I do not ask for it in this PR. Do one of these:
- State this limit in the doc comment of
cleanup_deferred_by. - Open a follow-up so that reconcile failures appear in the cleanup result (invariant 11: failures are observable). The same route would then cover them.
| ## Fault axes | ||
|
|
||
| - **Crash windows**: `Scenario::crash_at` arms any of the engine's 72 named | ||
| - **Crash windows**: `Scenario::crash_at` arms any of the engine's 53 named |
There was a problem hiding this comment.
Nit: the engine declares 82 seams (crates/omnigraph/src/seams/catalog.rs). crash_at accepts any of them through seams::catalog::decide. catalog::CRASH_WINDOWS holds the 53 that the hunt sweeps, so "the engine's 53 named failpoints" is not accurate.
Suggested wording: "Scenario::crash_at arms a named engine failpoint at a seeded op index. The ignored dst_hunt_crash_window_sweep test sweeps the 53 in catalog::CRASH_WINDOWS." Line 106 already states the count correctly.
| legal_rejections += 1; | ||
| None | ||
| } | ||
| CrashOutcome::CleanupDeferred => { |
There was a problem hiding this comment.
Optional: the hunt cannot see this outcome.
dst_hunt_crash_window_sweep counts a window as hit only when report.crashes > 0 (tests/scenarios.rs, line 175), and it prints verdicts only for hits. This arm adds nothing to crashes; the new pin asserts this with a.crashes == 1, where the one crash is the setup crash. So a deferred cleanup window falls through to the probe pass, and the hunt files it under "crossed-but-absorbed (self-healing verified live)". For a deferral that label is wrong. The table is owed, and only the obligation pass proves the retry. Also, the Deferred row never appears in the hunt's output, so this route leaves no trace in the hunt report.
Suggested fix: make the hunt count a deferred: verdict as a hit. Alternatively, add a deferrals counter to UniverseReport and test crashes + deferrals > 0. With either fix, the hunt prints the Deferred row and stops at the first hit.
Stacked on #1027, which adds the seam
cleanup.sweep_pre_dead_manifest_delete. Until #1027 merges this diff includes its commits; the change here is the last commit.Summary
cleanup.sweep_pre_dead_manifest_deletein DST's crash-window catalog. It is appended, so the index-derived census seeds of the other windows stay put (52 → 53 windows; README counts corrected from a stale 72).census_setupcrashes the index build atensure_indices.post_phase_b_pre_manifest_commit. That strands a detached staging, index files included. The second insert moves main's head past the head the staging's witness recorded, so the recipe's cleanup judges the staging dead and crosses the window.cleanup: the engine records the table's error in its row and returnsOk.crash_optreated that as a plain success, so the harness ran no retry and the report kept no trace of the window. Nowcrash_opreturnsCleanupDeferredwhen the op's deferrals name the scheduled window. The universe recordsdeferred:<window>@op<i>(verdictDeferred, channelcleanup) and runs the obligation pass a crashed cleanup gets: a clean rerun that defers nothing, the collector oracle (retained images and paths, no published garbage or dead staging left) and the world read. This applies tocleanup.table_gcand the three earlier sweep windows too.dst_dead_staging_sweep_window_is_reached_and_retried. One universe probes the window. The other stops the recipe's cleanup at the window (after the dead staging's files, before its manifest) and requires the owed table's retry to converge. Both replay strictly.dst_sensitivity_maintenance_rerun_failure_is_redgains adeferred-cleanupcase that must go red from the owed table's pass.Reach census
cd crates/omnigraph-dst && cargo test --locked --test scenarios dst_window_reach_probe -- --ignored --nocapturecleanup.sweep_pre_dead_manifest_deleteis in the crossed list. All three of its census seeds cross (40052, 50052, 60052). In seed 40052 the recipe lands on op2 (insert), op4 (index build, setup crash), op6 (insert) and op8 (the cleanup that crosses).The same census at the base commit
96d8b30areports 40/52 with the identical 12 dark windows, so this change adds exactly the new window. The instrument still fails its floor,crossed.len() >= 48, on both: that floor was set when the catalog had 66 windows. This PR leaves the floor alone.The deferral gap, measured
Scheduled crashes on
cleanup.table_gc,cleanup.sweep_pre_manifest_delete,cleanup.sweep_between_manifestsandcleanup.sweep_pre_file_delete(seeds 7 and 8, 24 ops, each window's recipe): all 8 universes reportedcrashes = 0, because the window became a deferred table. Before this change none of them ran a retry obligation. A later cleanup checked convergence only if the sampler happened to emit one. The final audit checks safety and trace completion, not that dead stagings were reclaimed. With the change, each universe records itsdeferred:verdict, runs the obligation pass and stays green.Tests
cd crates/omnigraph-dst && cargo test --locked: lib 53 passed; integration 2 passed, 3 ignored; scenarios 52 passed, 28 ignored.cleanup_deferred_byreturning false), the pin fails ("the stopped cleanup must leave the table owed") and the sensitivity proof fails ("a failing rerun MUST redden the universe").cargo fmt --all --check,cargo clippy -p omnigraph-dst --all-targets --locked -- -D warnings -W clippy::dbg_macro,typos,python3 scripts/check-docs.py.Not covered
NotFoundfor an absent index directory. DST's shared-memory store lists an absent directory as empty;failpoints.rsin test(cleanup): interrupt the sweep between manifests and before dead-staging manifests #1027 covers the file backend.census_setup's crash. They schedule this window on a cleanup that has no dead staging, so it stays dark there; only the reach census and the pin reach it.