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

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


Patch Set 6:

(4 comments)

Addressed the review comments and improved on scan nodes and exchange node.

http://gerrit.cloudera.org:8080/#/c/19033/5/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/19033/5/common/thrift/Query.thrift@873
PS5, Line 873: cpu_usage_bytes
> cpu usage is proportional to the total bytes of data for the query. Should
Yes, the evaluation cost per row is being worked on.


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

http://gerrit.cloudera.org:8080/#/c/19033/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@2147
PS5, Line 2147: long cpuUsageBytesPerInstance = 
computeCpuUsageBytes(numOfInstancesToComputeCpuUsage);
              :
              :     nodeResourceProfile_ = new ResourceProfileBuilder()
              :         .setMemEstimateBytes(perInstanceMemEstimate)
              :         .setMinMemReservationBytes(computeMinMemReservation(
> Don't understand these statement.
Sorry about the copy/paste error. The formula is improved further. Done.


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

http://gerrit.cloudera.org:8080/#/c/19033/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@421
PS5, Line 421:
> ?
Fixed.


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

http://gerrit.cloudera.org:8080/#/c/19033/5/fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java@96
PS5, Line 96: probeCpuUsage = probePerInstanceMemEstimate / numInstances_;
            :     long buildCpuUsage
> Why need these two variables?
Good catch. Fixed.



--
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: 6
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Sep 2022 19:06:37 +0000
Gerrit-HasComments: Yes

Reply via email to