Skip to content

fix: tolerate concurrent manifest cleanup - #8213

Merged
Xuanwo merged 1 commit into
mainfrom
gatekeeper/fix-8212-1
Aug 4, 2026
Merged

fix: tolerate concurrent manifest cleanup#8213
Xuanwo merged 1 commit into
mainfrom
gatekeeper/fix-8212-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • Ignore a typed NotFound only when a manifest listed by cleanup is older than the cleanup snapshot and has been removed concurrently.
  • Keep failures for the current snapshot manifest, newer manifests, and all other I/O errors fatal.
  • Add regression coverage for a manifest deleted between listing and reading.

Root cause

V2 manifest filenames are intentionally encoded as u64::MAX - version, so 18446744073709548523.manifest represents logical version 3092 rather than an underflowed version. Two concurrent cleanup calls can race after one lists that old manifest: the other removes it before the first reads it, and the first propagated the resulting NotFound.

Validation

  • cargo test -p lance cleanup_ignores_old_manifest_removed_after_listing
  • cargo test -p lance dataset::cleanup::tests
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

Fixes #8212

@github-actions github-actions Bot added the bug Something isn't working label Aug 4, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Gate recommendation: approve with a non-blocking risk.

The patch addresses the reported list/read race at the right boundary: it uses parsed logical versions, keeps current/newer manifests and non-NotFound failures fatal, and covers deletion between the manifest and index reads.

The production report does not timestamp-confirm the competing deletion, and the test exercises the stale location directly rather than two full concurrent cleanups. An end-to-end concurrency regression would reduce that remaining uncertainty, but the implementation and focused coverage support this fix.

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Xuanwo
Xuanwo merged commit 3c50724 into main Aug 4, 2026
39 checks passed
@Xuanwo
Xuanwo deleted the gatekeeper/fix-8212-1 branch August 4, 2026 08:07
@Xuanwo Xuanwo added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

optimize(cleanup_older_than) intermittently reads a manifest at an underflowed u64 version (2^64-3093)

1 participant