924060929 commented on code in PR #66530:
URL: https://github.com/apache/doris/pull/66530#discussion_r3850590925
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -418,8 +420,39 @@ public boolean supportsFileCache() {
*/
@Override
public List<ConnectorScanRange> planScan(ConnectorSession session,
ConnectorScanRequest request) {
- return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
- request.getFilter(), request.isCountPushdown());
+ IcebergTableHandle icebergHandle = (IcebergTableHandle)
request.getTableHandle();
+ if (session == null) {
+ return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
+ request.getFilter(), request.isCountPushdown());
+ }
+ if (icebergHandle.isSystemTable()) {
+ // System tables read connector metadata through their own
readers; never reuse them.
+ return planScanInternal(session, request.getTableHandle(),
request.getColumns(),
+ request.getFilter(), request.isCountPushdown());
+ }
+ // Statement-scoped reuse: within one statement the identical scan
(same table, same
+ // snapshot/ref/schema pin, same filter, same COUNT pushdown) plans
once and every
+ // duplicated relation shares the result. The scope is NONE for
offline planning and tests,
+ // in which case the loader runs on every call. Session variables are
constant within a
+ // statement and deliberately absent from the key.
+ //
+ // The ranges are memoized in a map HUNG INSIDE the statement scope
rather than cached
+ // directly: planScanInternal re-enters the scope itself (sharedTable
and the v3
+ // rewritableDeleteSupply are scope-backed), and a loader of the
scope's
+ // ConcurrentHashMap must not touch that map (same-bin re-entry throws
+ // IllegalStateException("Recursive update"); a mid-computation resize
silently drops the
+ // outer entry). The scope loader only constructs the memo map;
planning then runs on that
+ // separate map, so every scope call from planScanInternal is
top-level again. The memo
+ // key is catalog-scoped, which also isolates same-named tables across
a cross-catalog
+ // statement.
+ String memoKey = "iceberg.scan-reuse:" + session.getCatalogId() + ":"
+ session.getQueryId();
Review Comment:
Rechecked against current head 1c983f2e1e6: this path no longer calls the
filtered scan.planFiles() per alias. getScanNodeProperties uses
cachedApplicableEqualityDeleteFieldIds, which delegates to
IcebergManifestCache.getOrLoadEqualityDeleteFieldIds keyed by table location
and immutable snapshot ID and reads delete manifests only. Duplicate aliases
therefore share the snapshot projection without retaining data tasks or
defeating streaming. Adding the suggested exact-scan memo would reintroduce
per-alias planning/memory retention, so no code change is needed for the
current implementation.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveScanPlanProvider.java:
##########
@@ -238,6 +260,20 @@ public List<ConnectorScanRange> planScanForPartitionBatch(
ConnectorSession session,
ConnectorScanRequest request,
List<String> partitionBatch) {
+ if (session == null) {
+ return doPlanScanForPartitionBatch(session, request,
partitionBatch);
+ }
+ HiveScanReuseKey reuseKey = new
HiveScanReuseKey(session.getCatalogId(), session.getQueryId(),
+ (HiveTableHandle) request.getTableHandle(), partitionBatch);
+ return session.getStatementScope().computeIfAbsent(reuseKey,
Review Comment:
Fixed in 1c983f2e1e6. Both getSplits() and startSplit() now register
query-finish cleanup. All async partition/streaming producers are admitted
through SplitAssignment.submitProducer; stop atomically rejects later work,
closes the streaming source to unblock it, and waits for every admitted
runnable to exit. Query-finish stop callbacks are priority callbacks, so for
multiple scan nodes every producer quiesces before the first
read-transaction/scope cleanup callback. The nested untracked batch futures
were removed. Focused JDK 17 tests passed 37/37, including stop/wait, late
rejection/resource close, executor rejection accounting, batch/streaming
behavior, and multi-scan callback ordering; FE Checkstyle reports 0 violations.
--
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]
