JingsongLi commented on code in PR #9897:
URL: https://github.com/apache/paimon/pull/9897#discussion_r4031828654
##########
paimon-core/src/main/java/org/apache/paimon/manifest/ManifestFile.java:
##########
@@ -140,9 +144,33 @@ public <T> List<T> read(
Filter<InternalRow> readFilter,
Filter<ManifestEntry> readTFilter,
Function<ManifestEntry, T> convertor) {
+ return read(
+ fileName,
+ fileSize,
+ partitionFilter,
+ bucketFilter,
+ readFilter,
+ readTFilter,
+ convertor,
+ null);
+ }
+
+ public <T> List<T> read(
+ String fileName,
+ @Nullable Long fileSize,
+ @Nullable PartitionPredicate partitionFilter,
+ @Nullable BucketFilter bucketFilter,
+ Filter<InternalRow> readFilter,
+ Filter<ManifestEntry> readTFilter,
+ Function<ManifestEntry, T> convertor,
+ @Nullable ManifestSidecar.Selection selected) {
+ if (selected != null && selected.blocks().isEmpty()) {
+ return java.util.Collections.emptyList();
+ }
try {
Path path = pathFactory.toPath(fileName);
- if (cache != null) {
+ // Sidecar selections use the block cache, even when every block
is selected.
+ if (cache != null && selected == null) {
Review Comment:
[P2] Reuse an existing whole-manifest cache hit before reading selected
blocks
With `manifest.sidecar.enabled=true` and
`sink.writer-coordinator.prefetch-manifests=true`,
`TableWriteCoordinator.refresh()` warms the whole-manifest entry cache through
an unfiltered scan. Its subsequent restore requests use
`withPartitionBucket(...)`, so sidecar selection becomes non-null and this
condition bypasses those already-cached entries. The selected-block reader only
checks `BlockCacheKey`, whereas prefetch populated the `Path` key, so the first
access to each uncached block reads the manifest again—even when every block is
selected. This defeats the existing prefetch behavior and adds storage I/O and
Avro decoding during writer recovery.
I reproduced this with a recording `FileIO`: an unfiltered scan populated
`ManifestEntrySegments`, then a bucket-filtered scan reopened both the sidecar
and the manifest despite the complete entry cache still being present.
Could we reuse an existing whole-manifest cache hit first and use
selected-block reads only on a miss? Partial results should still never
populate the whole-manifest cache.
--
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]
