perf: stop listing the entire _versions directory for limited version queries - #8679
Open
LuQQiu wants to merge 3 commits into
Open
perf: stop listing the entire _versions directory for limited version queries#8679LuQQiu wants to merge 3 commits into
LuQQiu wants to merge 3 commits into
Conversation
… queries DirectoryNamespace::list_versions_under always collected every object under the table's _versions/ prefix before sorting and applying the caller's limit. On object stores this means paginating the whole directory even when the caller only wants the latest version (descending, limit=1), which is exactly what get_latest_version does on every dataset resolution. For a table with hundreds of thousands of versions on S3 this is ~340 sequential LIST pages (~25s of pure I/O wait) per resolution. On lexically-ordered stores the V2 manifest naming scheme (u64::MAX - version, zero-padded) already yields newest-first listings, so when the requested order matches the stream's natural order we can stop consuming the stream after `limit` entries instead of collecting everything. Measured against a real ~340k-version table on S3, resolving the latest version drops from ~25s (344 paginated LIST requests) to ~0.2s (a single page). Stores without lexical ordering guarantees and mismatched-order requests keep the existing collect-then-sort behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…first raw entry
The commit protocol retains staging blobs ({manifest}-<uuid>) in
_versions/ after ambiguous publish failures, and their inverted keys sort
ahead of the newest committed V2 manifest on lexically-ordered stores.
Detecting the naming scheme from the first raw entry misclassified such a
stream as non-V2 and silently disabled the early-stop, degrading the
latest-version hot path back to a full directory listing. Detect from the
first entry that parses as a committed manifest instead, and prove the
bound with an entry-consumption-counting regression (staging entry plus
one manifest).
Also ignore negative limits instead of clamping them to 0, matching
apply_pagination (the old code effectively ignored them via the usize
cast wrap).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The retained-staging case is fixed: ordered V2 latest-version lookup now detects its naming scheme from the first committed manifest and remains bounded after skipping retained non-manifest entries. No Gate blocker remains.
xuanyu-z
approved these changes
Aug 20, 2026
Member
|
What happens on non-ordered stores like S3 express? Does it just fall back to the old behavior? |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Author
yep,,, Like S3 express, it's still the old behavior, still reading all the version pages |
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.
Problem
DirectoryNamespace::list_versions_underalways collects every object under the table's_versions/prefix (read_dir_all(...).try_collect()) before sorting and applying the caller'slimit. On object stores this paginates the entire directory even when the caller only wants the latest version (descending=true, limit=1) — which is exactly whatget_latest_versiondoes on every dataset resolution through the namespace commit path.For a table with ~340k versions on S3 this is ~344 sequential
ListObjectsV2pages per resolution: ~25s of pure I/O wait (idle≈26s / busy≈1sper span), paid by everyopen_table/describe/merge_insertthat resolves the latest version. The equivalent non-namespace path (resolve_version_from_listing) already resolves the latest V2 manifest from roughly one list page.Fix
On lexically-ordered stores the V2 manifest naming scheme (
u64::MAX - version, zero-padded) already yields newest-first listings. When the requested order matches the stream's natural order, consume the stream and stop afterlimitentries instead of collecting everything:descending + limiton an ordered store with V2 naming → stream is already in the right order → stop afterlimitentries (theget_latest_versionhot path becomes a single list page).Also fixes a latent edge: the first stream entry is read for naming-scheme detection, so the limit is re-enforced afterward (covers
limit=0).Measured
Against a real ~340k-version table on S3 (
list_table_versionswithdescending=true, limit=1, same binary, 3 runs):Tests
test_list_versions_under_ordering_and_limitcovers both paths (memory://ordered → early-stop; local fs unordered → collect-then-sort) for every descending/limit combination includinglimit=0and over-limit.cargo test -p lance-namespace-impls --lib: 299 passed, 0 failed.cargo clippy --no-deps -p lance-namespace-impls --all-targets -- -D warningsclean.🤖 Generated with Claude Code