924060929 commented on code in PR #70410:
URL: https://github.com/apache/doris/pull/70410#discussion_r4545789942
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -653,9 +650,48 @@ private CloseableIterable<FileScanTask>
streamingFileScanTasks(TableScan scan, C
}
}
scanContext.checkCancelled();
+ if (!isManifestCacheEnabled() && canPlanManifestsWithoutCache(scan,
session)) {
+ return TableScanUtil.splitFiles(
+ manifestFileScanTasks(scan, session, table, filter, null,
readControl, readHandle, scanContext),
Review Comment:
Fixed in `03f344b6a7db`. Delete manifests now reuse the same inclusive
row-to-partition projection helper as data manifests. The new tests exercise
unpartitioned and bucket-source row predicates with position deletes under both
cache policies, compare with SDK-planned paths, and require the cached path to
populate its cache rather than silently fall back. Before the fix, the uncached
cases threw ValidationException and cached cases fell back; all now pass.
Validation: full Iceberg connector suite, 1810 tests / 0 failures / 0 errors
/ 5 existing skips; standard ARM64 FE build and Checkstyle passed.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -653,9 +650,48 @@ private CloseableIterable<FileScanTask>
streamingFileScanTasks(TableScan scan, C
}
}
scanContext.checkCancelled();
+ if (!isManifestCacheEnabled() && canPlanManifestsWithoutCache(scan,
session)) {
+ return TableScanUtil.splitFiles(
+ manifestFileScanTasks(scan, session, table, filter, null,
readControl, readHandle, scanContext),
+ sliceSize);
+ }
return TableScanUtil.splitFiles(sdkStreamingFileScanTasks(scan,
scanContext), sliceSize);
}
+ private boolean canPlanManifestsWithoutCache(TableScan scan,
ConnectorSession session) {
+ if (!SchemaAwareDataTableScan.supportsLocalManifestPlanning(scan)) {
+ return false;
+ }
+ ConnectorTaskScheduler scheduler = manifestReadScheduler(session);
+ // Standalone callers have no engine-owned workers/lifetime. Keep the
SDK's executor and scan
+ // semantics rather than silently turning its parallel planning into
inline serial reads.
+ if (scheduler == null || scheduler == ConnectorTaskScheduler.DIRECT) {
+ return false;
+ }
+ Snapshot snapshot = scan.snapshot();
+ return snapshot != null &&
canBufferManifestFiles(snapshot.dataManifests(scan.table().io()),
+ manifestReadConcurrency(session));
+ }
+
+ static boolean canBufferManifestFiles(List<ManifestFile> manifests, int
window) {
+ // A single manifest already streams without ParallelIterable in the
SDK. Preserve early delivery
+ // within large manifests too: our prefetcher retains whole parsed
manifests, whereas SDK consumers
+ // can take the first file before its manifest finishes. Bound the
window to the SDK's default
+ // 30,000-file planning buffer; unknown counts retain the streaming
SDK path.
+ if (manifests.size() <= 1) {
+ return false;
+ }
+ int filesPerManifest = MAX_UNCACHED_MANIFEST_WINDOW_FILES / window;
+ for (ManifestFile manifest : manifests) {
+ Integer added = manifest.addedFilesCount();
+ Integer existing = manifest.existingFilesCount();
Review Comment:
Fixed in `03f344b6a7db`. Eligibility now includes added + existing + deleted
entry counts, uses long arithmetic, and falls back conservatively if any count
is unknown. Boundary tests cover 3750 versus 3751 entries at an eight-manifest
window, deleted-entry-heavy manifests, unknown deleted counts and
integer-overflow-sized totals. The documentation now distinguishes physical
entries traversed from retained live files.
Validation: full Iceberg connector suite, 1810 tests / 0 failures / 0 errors
/ 5 existing skips; standard ARM64 FE build and Checkstyle passed.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -653,9 +650,48 @@ private CloseableIterable<FileScanTask>
streamingFileScanTasks(TableScan scan, C
}
}
scanContext.checkCancelled();
+ if (!isManifestCacheEnabled() && canPlanManifestsWithoutCache(scan,
session)) {
Review Comment:
Fixed in `03f344b6a7db`. Local enumeration now runs inside the SDK
SnapshotScan.planFiles lifecycle through an overridden doPlanFiles, preserving
its ScanEvent, planning timer and close-time table reporter. The planner
supplies native manifest/file/delete counters. Streaming tests cover current
and historical schemas, full consumption and early close, report
snapshot/schema/file counts and bytes, and verify exactly one report after
repeated close. All four reporting cases failed before the fix and now pass.
Validation: full Iceberg connector suite, 1810 tests / 0 failures / 0 errors
/ 5 existing skips; standard ARM64 FE build and Checkstyle passed.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -3251,10 +3286,10 @@ private CloseableIterable<FileScanTask>
cacheBackedFileScanTasks(TableScan scan,
if (!ManifestEvaluator.forPartitionFilter(filterExpr, spec,
caseSensitive).eval(manifest)) {
continue;
}
- ManifestCacheValue value =
manifestCache.getCachedManifestValue(manifest, statsQueryId);
+ ManifestCacheValue value = cached.apply(manifest);
if (value == null) {
try {
- value = deleteReadContext.read(manifest, m ->
manifestCacheGet(m, table, statsQueryId));
+ value = deleteReadContext.read(manifest, loader);
Review Comment:
Fixed in `03f344b6a7db`. Delete manifests now use the existing bounded
completion-ordered prefetcher and query-wide read budget. Their window starts
immediately instead of waiting for the first inline read or BE demand, while
data manifests retain their existing first-read policy. Latch-based tests
require even the first two delete reads to overlap, cover 2/6 manifests with
caching on/off and a budget of 2 with 4 workers, and verify cancellation wakes
the producer and prevents every queued read from opening a file. No new
executor or blocking same-pool child wait was added.
Validation: full Iceberg connector suite, 1810 tests / 0 failures / 0 errors
/ 5 existing skips; standard ARM64 FE build and Checkstyle passed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
