Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24665 )
Change subject: IMPALA-15243: Fix costing for CTEs ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/24665/2/fe/src/main/java/org/apache/impala/planner/CostingSegment.java File fe/src/main/java/org/apache/impala/planner/CostingSegment.java: http://gerrit.cloudera.org:8080/#/c/24665/2/fe/src/main/java/org/apache/impala/planner/CostingSegment.java@182 PS2, Line 182: segmentCore = CoreCount.sum(segmentCore, childCores.first); Could we avoid counting a shared producer more than once here? DistributedPlanner attaches the same CTEProducerNode to every consumer, so each consumer reaches the same child fragment and CoreCount.sum() adds it again. Q39 shows this: F13 has 120 instances, while both F05 and F00 report 240 self cores after adding the same producer, and the root reaches 480. The producer fragment only runs once. Could we deduplicate it and check the final cores_required in the test? http://gerrit.cloudera.org:8080/#/c/24665/2/fe/src/main/java/org/apache/impala/planner/PlanFragment.java File fe/src/main/java/org/apache/impala/planner/PlanFragment.java: http://gerrit.cloudera.org:8080/#/c/24665/2/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1346 PS2, Line 1346: verdict = ScalingVerdict.SCAN_FRAGMENT_BOUNDED; Would it make sense to preserve the consumer's original instance count here? SCAN_FRAGMENT_BOUNDED skips step 2, but the cost-based count was already computed above and is still assigned to selectedParallelism below. A cheap consumer can therefore end up with fewer instances than its fixed producer. ConnectCTEProducersAndConsumers() requires the per-host consumer count to be at least the producer count and an even multiple, so this would violate the scheduler mapping invariant. Could we enforce that constraint here and add a low-cost/many-scan-ranges test? -- To view, visit http://gerrit.cloudera.org:8080/24665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21b60af313070029a1fa8bf835107bef88f54912 Gerrit-Change-Number: 24665 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Stamatis Zampetakis <[email protected]> Gerrit-Comment-Date: Thu, 06 Aug 2026 04:27:35 +0000 Gerrit-HasComments: Yes
