Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21277 )
Change subject: IMPALA-12988: Calculate an unbounded version of CpuAsk ...................................................................... Patch Set 15: (10 comments) http://gerrit.cloudera.org:8080/#/c/21277/13//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21277/13//COMMIT_MSG@35 PS13, Line 35: ing COMPUTE_PROCES > max-parallelism is only shown on CPC=1 plan fragments? Yes. Rephrased this paragraph. http://gerrit.cloudera.org:8080/#/c/21277/13//COMMIT_MSG@37 PS13, Line 37: " fie > show Done http://gerrit.cloudera.org:8080/#/c/21277/13/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/21277/13/common/thrift/Query.thrift@1038 PS13, Line 1038: assignment for > "Used by Frontend to do executor group-set assignment for the query. Should Done http://gerrit.cloudera.org:8080/#/c/21277/13/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/21277/13/fe/src/main/java/org/apache/impala/planner/CostingSegment.java@102 PS13, Line 102: if (topNode == null) { > Change to if (topnode == null) Done http://gerrit.cloudera.org:8080/#/c/21277/13/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/21277/13/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@949 PS13, Line 949: * A fragment is either distributed evenly to all nodes or only at subset of nodes > This is existing logic, but not clear why we use this approach for step cou Done http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1133 PS13, Line 1133: if (!scanNodes.isEmpty()) { > Some comment explaining why RF based estimation is used for unbounded case. Added an explanation comment. http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1206 PS13, Line 1206: if (LOG.isTraceEnabled()) { > maxScannerThreads is set to int max then never updated. Probably can be rem It should be assigned with scanNodes.get(0).maxScannerThreads_. Thanks for catching this bug. http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@1304 PS13, Line 1304: ImmutableList.Builder<CoreCount> subtreeCoreBuilder = > Not entirely clear why calculating total here is a bounded case. Same for s Added an explanation comment. http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/service/Frontend.java@2197 PS13, Line 2197: while (i < numExecutorGroupSets) { > i == (numExecutorGroupSets - 1) Done http://gerrit.cloudera.org:8080/#/c/21277/13/fe/src/main/java/org/apache/impala/service/Frontend.java@2315 PS13, Line 2315: addCounter( > We should report the original values somewhere before scaling. Added "MaxParallelism" counter. -- To view, visit http://gerrit.cloudera.org:8080/21277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5441e31088f90761062af35862be4ce09d116923 Gerrit-Change-Number: 21277 Gerrit-PatchSet: 15 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Fri, 19 Apr 2024 16:30:53 +0000 Gerrit-HasComments: Yes
