Skip to content

fix(python): added cleanup_frag_reuse_index#7221

Open
gstamatakis95 wants to merge 6 commits into
lance-format:mainfrom
gstamatakis95:feat/python-cleanup-frag-reuse-index
Open

fix(python): added cleanup_frag_reuse_index#7221
gstamatakis95 wants to merge 6 commits into
lance-format:mainfrom
gstamatakis95:feat/python-cleanup-frag-reuse-index

Conversation

@gstamatakis95

Copy link
Copy Markdown
Contributor

Closes #7220

Expose the existing Rust frag-reuse pruner to Python as LanceDataset.cleanup_frag_reuse_index().

Deferred-remap compaction appends a generation to the __lance_frag_reuse index on every commit, and without pruning these accumulate forever and are loaded on every vector index open. The race-safe pruner from #3836 already exists in Rust but had no Python binding and is never invoked automatically.

The binding follows the migrate_manifest_paths_v2 wrapper pattern, the Python method is documented beside cleanup_old_versions as a periodic maintenance call, and a new test verifies that __lance_frag_reuse reports zero versions once all indexes have caught up and the cleanup runs.

@github-actions github-actions Bot added A-python Python bindings bug Something isn't working labels Jun 10, 2026
@gstamatakis95 gstamatakis95 force-pushed the feat/python-cleanup-frag-reuse-index branch from f2eaabb to d90747b Compare June 10, 2026 22:44

@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.

  1. The new Python cleanup entry point can panic when fragment-reuse metadata cannot be loaded. Corrupt or stale metadata can surface as a panic instead of a normal Python exception.
  2. The current test only exercises the already-caught-up index case. A cleanup implementation that drops all generations would still pass, so the regression can reappear.

@gstamatakis95

gstamatakis95 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author
  1. The new Python cleanup entry point can panic when fragment-reuse metadata cannot be loaded. Corrupt or stale metadata can surface as a panic instead of a normal Python exception.
  2. The current test only exercises the already-caught-up index case. A cleanup implementation that drops all generations would still pass, so the regression can reappear.

Thanks for the feedback @Xuanwo, addressed both issues.

@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.29730% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance/src/dataset/index/frag_reuse.rs 97.29% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@gstamatakis95

Copy link
Copy Markdown
Contributor Author

Also moved to dataset.optimize which IMO is a better fit for this.

Ok(true) => {}
Ok(false) => all_caught_up = false,
// An InvalidInput error means the reuse version is likely corrupted; drop it.
Err(Error::InvalidInput { .. }) => continue 'versions,

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.

The cleanup path treats a partially indexed rewrite group as corrupt and drops the reuse generation, while the load path treats the same straddling state as recoverable. Calling the new Python cleanup API on an older dataset can remove the generation that keeps affected indexes safe.

.await
.unwrap();
// Surface corrupt/stale metadata as a normal error instead of panicking on unwrap.
let frag_reuse_details = load_frag_reuse_index_details(dataset, frag_reuse_index_meta).await?;

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.

The cleanup path still reads external fragment-reuse details with unchecked offset/size arithmetic. Malformed metadata can still panic or produce an invalid object-store range instead of returning a Python exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No Python API to prune frag-reuse generations, so deferred-remap users accumulate read-path debt forever

2 participants