leaves12138 commented on code in PR #9924:
URL: https://github.com/apache/paimon/pull/9924#discussion_r4035482507
##########
paimon-core/src/main/java/org/apache/paimon/manifest/ManifestFile.java:
##########
@@ -402,8 +402,8 @@ public ManifestSidecar.Selection selectBlocks(
@Nullable RowRangeIndex query,
@Nullable PartitionPredicate partitionFilter,
@Nullable BucketFilter bucketFilter) {
- return !options.manifestSidecarEnabled()
- || (query == null && partitionFilter == null &&
bucketFilter == null)
+ boolean hasFilter = query != null || partitionFilter != null ||
bucketFilter != null;
+ return !options.manifestSidecarEnabled() || (!hasFilter && cache ==
null)
Review Comment:
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 the
`CacheMetrics` attached by `withCacheMetrics()`. With this change, a usable
sidecar produces a non-null selection, so `ManifestFile.read()` bypasses that
method. `SelectedBlockInput` accesses `SegmentsCache` directly 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 `manifestHitCache` and
`manifestMissedCache` remain **0**, rather than recording one hit and one miss.
The regression test fails on this commit and passes when only the previous
`selectBlocks()` 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
