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 7:

(2 comments)

Will reply to other comments in next patch set.

http://gerrit.cloudera.org:8080/#/c/21277/7/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/21277/7/common/thrift/Query.thrift@1037
PS7, Line 1037: Used by admission control to decide which
              :   // executor group to run the query.
> The decision is already made in the planner, no? Admission Controller will
cores_required and cores_required_unbounded fields here are how Planner 
communicate core requirement to Frontend.
They are used to assign value to cores_requirement and 
cores_requirement_unbounded (see L2278 to L2280 in Frontend.java, patch set 7).

        cores_requirement = req.query_exec_request.getCores_required();
        cores_requirement_unbounded =
            req.query_exec_request.getCores_required_unbounded();

https://gerrit.cloudera.org/c/21277/7/fe/src/main/java/org/apache/impala/service/Frontend.java#2278


http://gerrit.cloudera.org:8080/#/c/21277/7/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/7/fe/src/main/java/org/apache/impala/service/Frontend.java@2304
PS7, Line 2304:         coreReqToUse = Math.max(cores_requirement, 
cores_requirement_unbounded);
> Is it even possible to have `cores_requirement` to be more than `cores_requ
Yes, if PROCESSING_COST_MIN_THREADS is set high, planner can come up with 
cores_requirement_unbounded < cores_requirement. I will add assertion in 
test_query_cpu_count_divisor_default to highlight this.

Since the query plan itself always follows bounded count, simply using 
cores_requirement_unbounded can potentially hurt query performance in that case.



--
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: 7
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: Tue, 16 Apr 2024 15:12:13 +0000
Gerrit-HasComments: Yes

Reply via email to