Repository navigation
fix(namespace): handle legacy auto-cleanup policy on directory manifest - #9600
Merged
Merged
Conversation
Directory manifests created by Lance before 7.0 carry the default lance.auto_cleanup.* config, and writers of that era remove old versions under it. A reader that only probes the immediate successor manifest then stops reloading once that successor is cleaned up. Resolve the latest version while the held manifest carries the policy, drop the policy on the next manifest rewrite, and replace the debug assertion that rejected such manifests.
Xuanwo
marked this pull request as ready for review
October 1, 2026 05:57
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
BubbleCal
approved these changes
Oct 1, 2026
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The legacy freshness failure is reproduced by the released fixture and fixed across all three access paths. The policy transition preserves the existing fast path for current catalogs. No merge-blocking issue found.
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.
Directory
__manifestdatasets created before Lance 7.0 carry a default auto-cleanup policy. If an old writer removes version N+1 while a reader holds N, the successor-only reload check leaves the reader on an obsolete catalog, potentially referencing deleted data. Debug builds also panic when opening these manifests.This change removes the assertion and resolves the latest version while the held manifest carries
lance.auto_cleanup.*. Policy-free manifests retain the successor-probe fast path. Successful manifest rewrites remove the inherited policy, so subsequent writers stop automatic old-version cleanup.The branch incorporates main, including #9285. Both the read-side probe and
reload_under_write_lock()use the compatibility-aware check;get_mut()still acquires the write lock before reloading.Validation:
cargo test --locked -p lance-namespace-impls --lib: 319 passed, including the fix(namespace): avoid write starvation under heavy reads #9285 fairness regression.get(),get_refreshed()andget_mut()after cleanup removes the immediate successor, then verifies refresh continues after the policy is removed. All three pass; temporarily restoring successor-only logic makes all three fail with held version 5 instead of latest version 7. The correct code was restored and the full suite rerun.cargo fmt --all -- --check,git diff --check, and Ruff 0.11.2 lint/format checks for the fixture generator pass.cargo clippy --all --tests --benches -- -D warningsis blocked by existingsingle_range_in_vec_initandimplicit_cloneerrors inlance-io. A separate namespace-only clippy run is blocked by 12 existingimplicit_cloneerrors in unchanged code. No lint suppression or unrelated refactoring is included; fresh PR CI provides the full platform checks.Fresh CI for
608146047: 36 checks passed and the Draft Lance Gatekeeper was skipped. This includes Rust platform tests and clippy, Python platform, compatibility and AWS tests, and Java 11/17/21/25 tests.Manual cleanup of a policy-free
__manifestremains outside this module's contiguous-version contract. This PR remains Draft.