Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21257 )

Change subject: IMPALA-12980: Translate CpuAsk into admission control slots
......................................................................


Patch Set 9:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/21257/9/be/src/scheduling/scheduler.cc@1212
PS9, Line 1212:       be_max_instances =
> Should we skip this computation for coordinator fragments? Does CPC cpu cos
CPC cpu costing also apply to coordinator fragments, but excluded during EG 
comparison (deduct CpuAsk by 1) in anticipation of dedicated coordinator 
assignment.

If it is a dedicated coordinator assignment, dominant_instance_count will 
likely be 0 and we fallback to default AdmissionSlot accounting at L1210.

If it is not a dedicated coordinator assignment, it is likely that some 
dominant fragment is assigned in coordinator node as well, therefore this 
min(dominant_instance_count, state->request().max_slot_per_executor) logic is 
correct.

I will look around if we can add BE test, maybe in scheduler-test.cc


http://gerrit.cloudera.org:8080/#/c/21257/9/fe/src/main/java/org/apache/impala/planner/CoreCount.java
File fe/src/main/java/org/apache/impala/planner/CoreCount.java:

http://gerrit.cloudera.org:8080/#/c/21257/9/fe/src/main/java/org/apache/impala/planner/CoreCount.java@148
PS9, Line 148:       // If core1 and core2 has equal total(), return one that 
does not have coordinator
> Should this be more consistent in removing coordinator core count? Seems it
I think totalWithoutCoordinator() method will be useful. I'll test this.



--
To view, visit http://gerrit.cloudera.org:8080/21257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 9
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Thu, 11 Apr 2024 01:46:16 +0000
Gerrit-HasComments: Yes

Reply via email to