Skip to content

perf: cache manifests in Dataset::load_manifest - #16

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/manifest-cache-state-reconciler
Open

perf: cache manifests in Dataset::load_manifest#16
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/manifest-cache-state-reconciler

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

state-reconciler spends a large share of its tick time in a provenance walk that calls Dataset::checkout_version(version) for each version from cursor..=latest_version_id. Each call currently re-reads and fully decodes the manifest from object storage, even when the same manifest is checked out repeatedly within the same session.

The Session already maintains a metadata_cache keyed by ManifestKey { version, e_tag } for latest_manifest and transactions. This PR extends that cache to Dataset::load_manifest so repeated checkout_version calls for the same manifest reuse the same Arc<Manifest> instead of re-decoding it.

What changed

  • Dataset::load_manifest now returns Arc<Manifest> and consults/inserts the session metadata cache before object-store read + decode.
  • Dataset::checkout_manifest and DatasetBuilder callers updated to pass the Arc<Manifest> through.
  • load_new_transactions now relies on load_manifest for caching instead of doing a second duplicate insertion.
  • do_commit_new_dataset deep-clones the manifest from the returned Arc.
  • Bumps ethnum 1.5.21.5.3, which is required to compile the workspace on Rust >= 1.97 (upstream ethnum-rs#58).

Validation

  • cargo check -p lance passes.
  • cargo test -p lance --lib test_checkout_version_uses_manifest_cache passes, asserting Arc::ptr_eq across repeated checkout_version calls.
  • Full dataset_versioning module passes.

A/B note

This change is the Lance-side fix for the Atlas state-reconciler-ab-b load-test profile: the provenance walk's read_manifest / Message::decode frames should shrink because manifests are loaded once per version and shared across the tick.

Link to Devin session: https://app.devin.ai/sessions/0401670882af4a4785541fa753378cda

state-reconciler's provenance walk repeatedly calls checkout_version,
which currently re-reads and re-decodes the full manifest each time.
The session already has a metadata_cache keyed by (version, e_tag) that
caches latest_manifest and transactions. Use it in load_manifest so
repeated checkout_version calls for the same version reuse the same Arc<Manifest>.

Also updates ethnum 1.5.2 -> 1.5.3, which is required to compile on
Rust >= 1.97 (TryFromIntError size changed; see ethnum-rs#58).

Test: unit test asserts Arc pointer equality for repeated checkout_version calls.
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant