Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19656 )
Change subject: IMPALA-12029: Relax scan fragment parallelism on first planning ...................................................................... Patch Set 5: (8 comments) http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG@9 PS4, Line 9: In a setup with multiple executor gr > nit: In a setup with multiple executor group set Done http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG@11 PS4, Line 11: are > nit: kinds Done http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG@18 PS4, Line 18: relax > nit: relaxes Done http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG@21 PS4, Line 21: we replan once again with that executor group set but with : scan fragment parallelism returned back to MT_DOP. > Does this mean we need one more replan for any query plan with scan fragmen Yes, there will be one extra replan, except when we arrive at largest executor group set where we immediately fix scan node parallelism to MT_DOP. Looking at test_min_processing_per_thread_small, a single round of query planning there is about 35ms. http://gerrit.cloudera.org:8080/#/c/19656/4//COMMIT_MSG@27 PS4, Line 27: MT_DOP. > nit: fragments Done http://gerrit.cloudera.org:8080/#/c/19656/4/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/19656/4/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@262 PS4, Line 262: public void computeCostingSegment( > Need a better name for the flag. Maybe limitScanParallelism? Done http://gerrit.cloudera.org:8080/#/c/19656/4/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1061 PS4, Line 1061: rNode, int m > here and other places, should we name is as fixedLeafNodes? Renamed to limitScanParallelism. http://gerrit.cloudera.org:8080/#/c/19656/4/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/19656/4/fe/src/main/java/org/apache/impala/planner/ScanNode.java@361 PS4, Line 361: Thr > <= 0. Otherwise if getInputCardinality() equals 0, syntheticCardinality equ cardinality == 0 is actually legal for empty scan. Added one more case above this. -- To view, visit http://gerrit.cloudera.org:8080/19656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7a2276fbd344d00caa67103026661a3644b9a1f9 Gerrit-Change-Number: 19656 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 30 Mar 2023 15:36:00 +0000 Gerrit-HasComments: Yes
