-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[core] Reuse sidecar block cache for unfiltered scans #9924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
JingsongLi
merged 2 commits into
apache:master
from
JingsongLi:codex/manifest-sidecar-block-cache
Sep 17, 2026
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we preserve the cache hit/miss metrics when routing unfiltered scans through the block cache?
Previously, an unfiltered scan used
ObjectsCache.read(), which updates theCacheMetricsattached bywithCacheMetrics(). With this change, a usable sidecar produces a non-null selection, soManifestFile.read()bypasses that method.SelectedBlockInputaccessesSegmentsCachedirectly and never updates these counters.I reproduced this with one sidecar-backed manifest and an enabled manifest cache: perform one cold unfiltered read and then one warm unfiltered read. The second read performs no file I/O, but both
manifestHitCacheandmanifestMissedCacheremain 0, rather than recording one hit and one miss. The regression test fails on this commit and passes when only the previousselectBlocks()condition is restored.This does not change query results, but it makes the existing cache observability silently stop working for unfiltered sidecar-backed scans as well. Please carry the metrics into the block-cache path and preserve the per-manifest accounting, with a regression test covering cold and warm reads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d3e178e. The selected-block reader now tracks actual block-cache accesses and reports exactly one manifest-level hit or miss after each read: any uncached selected block is a miss, while an all-block cache hit is a hit. The regression test covers one cold unfiltered read followed by one warm unfiltered read and verifies both I/O and the hit/miss counters. Five focused ManifestFile tests and the paimon-core compile with Checkstyle/Spotless pass.