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

(8 comments)

http://gerrit.cloudera.org:8080/#/c/21277/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21277/7//COMMIT_MSG@22
PS7, Line 22: During executor group set
            : selection, Frontend should use the maximum between bounded CpuAsk 
and
            : unbounded CpuAsk numbers to avoid assigning a query to a small 
executor
            : group set too soon. The last executor group set stays as the 
"catch-all"
            : executor group set.
> Testing so far does confirm that unbounding will cause queries to flow to l
David and I discuss about using sublinear function today. Will test it in next 
patch set.


http://gerrit.cloudera.org:8080/#/c/21277/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/21277/7/be/src/scheduling/scheduler.cc@1210
PS7, Line 1210: xe
> nit: remove "is" or change to "that is"
Done


http://gerrit.cloudera.org:8080/#/c/21277/7/be/src/scheduling/scheduler.cc@1210
PS7, Line 1210: n
> nit: change to "is with"
Done


http://gerrit.cloudera.org:8080/#/c/21277/7/be/src/scheduling/scheduler.cc@1211
PS7, Line 1211: ance_count'.
> nit: "the schedule would only assign"
Done


http://gerrit.cloudera.org:8080/#/c/21277/7/be/src/scheduling/scheduler.cc@1213
PS7, Line 1213:       if (dominant_instance_count >= be_max_instances) {
> Comment/terminology is overall hard to follow here. Is this referring to an
Rephrase and refactor a a bit at https://gerrit.cloudera.org/c/21257/14
Hope it is clearer.


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@133
PS7, Line 133: The sched
> nit: The scheduler
Done


http://gerrit.cloudera.org:8080/#/c/21277/7/common/thrift/Query.thrift@1037
PS7, Line 1037: Used by Frontend to decide which
              :   // executor group to run the query.
> Yes, meant to say we should probably say: "Used by Fronted to decide ..."
Done


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);
> Yes, if PROCESSING_COST_MIN_THREADS is set high, planner can come up with c
Next commit in this relation chain shows the possibility:
https://gerrit.cloudera.org/c/21279/10/tests/custom_cluster/test_executor_groups.py#1104



--
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: 8
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 20:12:53 +0000
Gerrit-HasComments: Yes

Reply via email to