Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19033 )
Change subject: IMPALA-11604 Planner changes for CPU usage ...................................................................... Patch Set 45: (11 comments) http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@16 PS44, Line 16: then finally returns a > May need to explain why blocking operators are considered in the context of Removed this phase here and put more explanation in steps II and III. http://gerrit.cloudera.org:8080/#/c/19033/44//COMMIT_MSG@19 PS44, Line 19: > nit found Done http://gerrit.cloudera.org:8080/#/c/19033/37/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: http://gerrit.cloudera.org:8080/#/c/19033/37/be/src/util/backend-gflag-util.cc@201 PS37, Line 201: 2 w > The scaling factor as defined is less intuitive, since one has to inverse i Renamed to query_cpu_requirement_divisor. http://gerrit.cloudera.org:8080/#/c/19033/37/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/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@147 PS37, Line 147: Costs_ = null; : private List > nit. A positive value implies the instance count has been adjusted. Done http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@864 PS37, Line 864: r(Predicates.i > Better renamed as getMaxParallelismByTotalWorkSize(). Done http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@883 PS37, Line 883: Preconditions.checkState(p.getChildCount() > 1); : long buildRowCount = p.getChild(1).getCardinality(); : if (((JoinNode) p).getDistributionMode() == DistributionMode.BROADCAST) { : // For Broadcast join, all join receive the same work size from the build. : buildRowCount = buildRowCount * p.getNumInstances(); : } : > Repeated use from line 869. Can be refactored. Removed. http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@957 PS37, Line 957: protected int getAdjustedInstanceCount() { return adjustedInstanceCount_; } > Should add a comment. Done http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@972 PS37, Line 972: ngBuilder builder = new StringBuilder(); : for (int i = processingCosts_.size() - 1; i >= 0; i--) > I wonder if the computation can be improved here e.g. by the size of the wo In newer patch set, this is later lowered based on producer-consumer rate ratio. http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1098 PS37, Line 1098: Math.max(nodeStepCount, getMaxParallelismByTotalWorkSize()); > add a comment should be helpful. Done http://gerrit.cloudera.org:8080/#/c/19033/37/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1113 PS37, Line 1113: > does not sound right. Removed. http://gerrit.cloudera.org:8080/#/c/19033/44/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-processing-cost.test File testdata/workloads/functional-planner/queries/PlannerTest/tpcds-processing-cost.test: http://gerrit.cloudera.org:8080/#/c/19033/44/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-processing-cost.test@75 PS44, Line 75: ss_sold_date_sk = > Is it possible to show the new processing cost here too? It will be wonderf Done. Add max-parallelism and fragment-costs as well. -- To view, visit http://gerrit.cloudera.org:8080/19033 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If32dc770dfffcdd0be2b5555a789a7720952c68a Gerrit-Change-Number: 19033 Gerrit-PatchSet: 45 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Fri, 10 Feb 2023 05:28:40 +0000 Gerrit-HasComments: Yes
