Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24852 )

Change subject: IMPALA-15358: Cap Kudu DML writers to partition count
......................................................................


Patch Set 8: Code-Review+1

(3 comments)

Few more comments, otherwise LGTM!

http://gerrit.cloudera.org:8080/#/c/24852/8/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java:

http://gerrit.cloudera.org:8080/#/c/24852/8/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java@344
PS8, Line 344: !enforceShuffle
The cap is only applied inside this block, so an insert with a shuffle hint 
still gets the KUDU exchange but neither the tablet-count cap nor 
MAX_FS_WRITERS:

 insert /* +shuffle */ into functional_kudu.alltypes
 select * from functional_parquet.alltypes;


http://gerrit.cloudera.org:8080/#/c/24852/8/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java
File fe/src/main/java/org/apache/impala/planner/KuduTableSink.java:

http://gerrit.cloudera.org:8080/#/c/24852/8/fe/src/main/java/org/apache/impala/planner/KuduTableSink.java@131
PS8, Line 131:     fragment_.setFixedInstanceCount(fragment_.getNumInstances());
This pins the fragment's parallelism for every Kudu DML under 
COMPUTE_PROCESSING_COST, including the ones that have no cap. Should we add if 
(hasInstanceLimit())?


http://gerrit.cloudera.org:8080/#/c/24852/8/testdata/workloads/functional-planner/queries/PlannerTest/kudu-insert-writer-limit.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-insert-writer-limit.test:

http://gerrit.cloudera.org:8080/#/c/24852/8/testdata/workloads/functional-planner/queries/PlannerTest/kudu-insert-writer-limit.test@1
PS8, Line 1: # insert into a HASH-partitioned Kudu table. Writer instance count 
is capped at the
All cases here run with mt_dop=10 and cost-based planning off. Can we add a few 
with COMPUTE_PROCESSING_COST=true? No existing test runs Kudu DML with that 
option, so nothing covers how the cap interacts with the parallelism adjustment.



--
To view, visit http://gerrit.cloudera.org:8080/24852
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3644fb245ceac72e91ed70c8c0392f65df75fde
Gerrit-Change-Number: 24852
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: David Rorke <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 01 Oct 2026 14:38:35 +0000
Gerrit-HasComments: Yes

Reply via email to