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
The three collector-retry fixtures in failpoints.rs become helpers over a graph uri. The file tests keep their names and call them on a temporary directory; one RustFS/S3 test and one Azure/Azurite test run all three, each on a fresh graph root, and both CI jobs require them by name. The first fixture now also stops a pass at cleanup.sweep_pre_file_delete, the one sweep window no failpoint test reached: the pass deletes nothing and the next pass plans the same sweep. Its retry asserts what the other two already did, no error on any table and nothing left to remove. Index-directory presence goes through the storage adapter's exists rather than std::fs, so the same assertion holds on an object store. The RFC's crash-window gate is satisfied and leaves blocked_on; the evidence bullet names the owners per backend.
This was referenced Oct 17, 2026
This was referenced Oct 17, 2026
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1027: this branch contains its commits, so merge #1027 first. The diff of this PR alone is the last commit.
What
Closes the S3 and Azure part of the Detached-only tables RFC gate "Crash windows inside the sweep, on file, S3 and Azure".
crates/omnigraph/tests/failpoints.rsare now helpers over a graph uri. The file tests keep their names (collector_retry_treats_an_index_directory_an_interrupted_pass_removed_as_swept,collector_retry_after_a_pass_stopped_between_manifests_finishes_the_sweep,collector_retry_after_a_pass_stopped_before_the_dead_manifests_removes_them) and call the helpers on a temporary directory.s3_collector_retry_converges_after_every_sweep_window(RustFS failpoints job) andazure_collector_retry_converges_after_every_sweep_window(Azurite job) run all three helpers, each on a fresh graph root. Both jobs now require them by name.cleanup.sweep_pre_file_delete, the fourth sweep window, which no failpoint test reached before. The stopped pass deletes nothing, and the next pass plans the same sweep. Its retry now asserts what the other two already asserted: no error on any table and nothing left to remove.StorageAdapter::existsinstead ofstd::fs, so the same assertion works on an object store.helpers::init_and_load_at(uri, schema)is the uri form ofinit_and_load_with_schema.graph_with_unbuilt_indexesnow takes a uri.Storageentry ofblocked_on. The evidence bullet names the owner for each backend.Every run checks that the retry reports no error, removes the remaining manifests, and leaves
cleanup_plan_missing_pathsempty.Expected green
These tests are coverage. They are not a reproduction of a defect. On S3 and Azure, lance-io 11's
ObjectStore::remove_dir_alllists the prefix and batch-deletes what it finds (object_store.rs, the arm for a store with no local directory operations). An absent_indices/<uuid>lists empty, so the retry's delete succeeds without theNotFoundarm that #1011 added for the local store. An absent data file or manifest takesdelete_object's existingNotFoundarm on every backend.Evidence
Run locally with
RUST_MIN_STACK=16777216on the canonical feature graph (--workspace --exclude omnigraph-gqt --exclude omnigraph-gqt-served --exclude omnigraph-dst --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints --test failpoints):collector_retry_*tests and the five othercollector_*failpoint tests pass. Without a bucket or container, the S3 and Azure cells took their skip.1.0.0-beta.12(the same digest CI pins), with theenvblock of the RustFS job.s3_passes 3 of 3, includings3_collector_retry_converges_after_every_sweep_window, with no skip line. The fixtures wrote 64, 64 and 35 objects under their roots, including 26, 26 and 4 under_indices/.azure_collector_retry_converges_after_every_sweep_windowruns only in the Azurite CI job. Locally it hit the unconfigured skip.cargo clippyon the same graph with-D warnings -W clippy::dbg_macro,cargo fmt --all --check,failpoint_names_guard,check-ci-cells.py(77 names) andcheck-docs.pyall pass.Reviewer question
With this clause gone, the rest of the
Storageentry (the DST oracle and the seven fixtures) appears to be satisfied too, according to the RFC's own Evidence section. I removed only the clause this PR proves and left removing the whole entry to the RFC's owners.