Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19033 )

Change subject: IMPALA-11604 Planner changes for CPU usage
......................................................................


Patch Set 16:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/19033/16/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

http://gerrit.cloudera.org:8080/#/c/19033/16/common/thrift/Frontend.thrift@740
PS16, Line 740: select
nit: selecting


http://gerrit.cloudera.org:8080/#/c/19033/16/common/thrift/Frontend.thrift@750
PS16, Line 750: amount of data
What's the unit? in bytes?


http://gerrit.cloudera.org:8080/#/c/19033/16/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/16/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@311
PS16, Line 311: CPU
processing


http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfile.java:

http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java@137
PS16, Line 137: TODO:
Processing cost is already reported. What's the TODO?


http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/planner/SubplanNode.java
File fe/src/main/java/org/apache/impala/planner/SubplanNode.java:

http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/planner/SubplanNode.java@106
PS16, Line 106: if (LOG.isTraceEnabled()) {
              :       
LOG.trace(AutoScaleUtil.getProcessingCostComputationDetails(getDisplayLabel(),
              :           getChild(0).getCardinality(), 0, getAvgRowSize(), 
numInstances_));
              :     }
              :
              :     long processingCost = Math.max(cardinality_, 0) * ((long) 
(getAvgRowSize()))
              :         / Math.max(numInstances_, 1);
Call AutoScaleUtil.computeAndLogProcessingCost()


http://gerrit.cloudera.org:8080/#/c/19033/16/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/19033/16/fe/src/main/java/org/apache/impala/service/Frontend.java@1728
PS16, Line 1728: cpu usage
processing cost


http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/service/Frontend.java@1799
PS16, Line 1799: 64*MEGABYTE
Why set this value?


http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/service/Frontend.java@1846
PS16, Line 1846: poolConfig.getMax_query_mem_limit()
Why set the same value as query memory limit?


http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/util/AutoScaleUtil.java
File fe/src/main/java/org/apache/impala/util/AutoScaleUtil.java:

http://gerrit.cloudera.org:8080/#/c/19033/16/fe/src/main/java/org/apache/impala/util/AutoScaleUtil.java@39
PS16, Line 39: if (LOG.isTraceEnabled()) {
             :       LOG.trace(getProcessingCostComputationDetails(
             :           label, cardinality, exprsCost, avgRowSize, 
numInstances));
             :     }
             :     return computeProcessingCost(cardinality, exprsCost, 
avgRowSize, numInstances);
This function call computeProcessingCost() twice. Could we reorganize the code 
to call computeProcessingCost() once?



--
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: 16
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: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Mon, 03 Oct 2022 21:25:50 +0000
Gerrit-HasComments: Yes

Reply via email to