You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2024/04/08 17:07:38 UTC

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slot to use

Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21257


Change subject: IMPALA-12980: Translate CpuAsk into admission control slot to use
......................................................................

IMPALA-12980: Translate CpuAsk into admission control slot to use

Admission control slot accounting is described in IMPALA-8998. On each
backend, number of. It compute 'slots_to_use' for each backend based on
the max number of instances of any fragment on that backend. This is
simplistic, because multiple fragments with same number of instance
count, say 4 non-blocking fragments each with 12 instances, only request
the max instance (12) admission slots rather than sum of it (48).

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represent the cpu requirement of that query over a
particular executor group set. This number is an estimation of what is
the largest number of query fragments that can run in-parallel given the
blocking operator analysis. Therefore, the fragment trace that sums into
that CpuAsk number can be translated into 'slots_to_use' as well, which
will be a closer resemblance of maximum parallel execution of fragment
instances.

This patch add new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which do the default algorithm from IMPALA-8998.
  This still the default value for SLOT_COUNT_STRATEGY option.
- PLANNER_EDOP, which will follow fragment trace that contributes
  towards CpuAsk number.

To do PLANNER_EDOP strategy, Planner will mark fragments that contribute
into CpuAsk as dominant fargment. It also pass max_slot_per_executor
information that it knows about the executor group set to scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
think the average 'slots_to_use' per backend will be. Actual
'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what actually scheduled on
that backend.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_tpcds_queries.py
16 files changed, 299 insertions(+), 86 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/1
-- 
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: newchange
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21257/5/fe/src/main/java/org/apache/impala/planner/CostingSegment.java@84
PS5, Line 84:       Preconditions.checkState(!nodes_.isEmpty());
> Can this just get computed once at the top and passed down as a recursion a
The recursive call happens along traverseBlockingAwareCores(), and we already have pass recursion args fragmentCoreState and subtreeCoreBuilder.
CoreCount is just a light container holding list of PlanFragmentId/PlanNodeId.



-- 
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: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2024 16:27:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG@19
PS11, Line 19:  rather
             : than sum of it (48)
> So if the configuration is correct, then cpuask will be always <= the number of cores?

Yes, CpuAsk should be <= total cores in selected executor group set. An exception is when CpuAsk still larger than biggest executor group set. In that case, we assign it anyway to largest executor group set since the biggest one is treated as "catch-all" group.

> I think that what needs a bit of explanation is "12 cores oversubscribed by 4x".

Lets assume that an executor host has exactly 48 cpu cores. What I want to say with the example is that, if an executor is assigned with 48 fragments instances that all can run in-parallel without blocking each other, they should be assigned 48 slots rather than 12 slots in that executor node.
Note that admission control slots inherently control how many concurrent query can run in a cluster. If this query is given 12 slots, then there can be 4 same queries running concurrently. But if query is given 48 slots, only 1 query can run at a time.


http://gerrit.cloudera.org:8080/#/c/21257/11/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/21257/11/tests/custom_cluster/test_executor_groups.py@1245
PS11, Line 1245:     #   CoreCount={total=16 trace=F15:3+F01:1+F14:3+F03:1+F13:3+F05:1+F12:3+F07:1},
Right, tpcds_cpu_cost/tpcds-q01.test is confusing here because the test has stats injection. I will add a new Planner test without stats injection.

> Does the cost not matter in this case, so we give a parallel fragment a full slot even if we estimate it to process just a couple of rows?

The role of processing cost stop after Planner select parallelism for each fragment. Naively, planner/scheduler can just total num fragment instances assigned to each node as slots requirement, but that will be wasteful if only subset of fragment can run in parallel while others blocked waiting. Therefore, planner/scheduler only select largest subset of fragments that does not block each other, and sum instances of that fragments subset as slot requirement.

> Another thing I don't get is that if F15 (a builder for a broadcast join) is included, then why F02 is not included, which has the source scan node of F15 and should run in parallel? (+it has a much higher cost than F15)

In the new planner test that I will submit next, we will see that parallelism of F02 (3) is lower than parallelism of its child fragments F15 + F01 (3 + 1). F02 and F01 is a blocking fragment because they have AGGREGATE node in it. They also can not begin work until F15, the join builder, is complete.

F02:PLAN FRAGMENT [HASH(sr_customer_sk,sr_store_sk)] hosts=3 instances=3 (adjusted from 384)
Per-Instance Resources: mem-estimate=10.49MB mem-reservation=1.94MB thread-reservation=1
max-parallelism=3 segment-costs=[327730, 110283] cpu-comparison-result=4 [max(3 (self) vs 4 (sum children))]
17:AGGREGATE [FINALIZE]
|  output: sum:merge(SR_RETURN_AMT)
|  group by: sr_customer_sk, sr_store_sk
|  mem-estimate=10.00MB mem-reservation=1.94MB spill-buffer=64.00KB thread-reservation=0
|  tuple-ids=2 row-size=24B cardinality=53.52K cost=315877
|  in pipelines: 17(GETNEXT), 00(OPEN)
|
16:EXCHANGE [HASH(sr_customer_sk,sr_store_sk)]
|  mem-estimate=502.09KB mem-reservation=0B thread-reservation=0
|  tuple-ids=2 row-size=24B cardinality=53.52K cost=11853
|  in pipelines: 00(GETNEXT)
|
F00:PLAN FRAGMENT [RANDOM] hosts=3 instances=3 (adjusted from 384)
Per-Host Shared Resources: mem-estimate=4.00MB mem-reservation=4.00MB thread-reservation=0 runtime-filters-memory=4.00MB
Per-Instance Resources: mem-estimate=26.33MB mem-reservation=2.12MB thread-reservation=1
max-parallelism=3 segment-costs=[351629, 110283] cpu-comparison-result=4 [max(3 (self) vs 4 (sum children))]
03:AGGREGATE [STREAMING]
|  output: sum(SR_RETURN_AMT)
|  group by: sr_customer_sk, sr_store_sk
|  mem-estimate=10.00MB mem-reservation=2.00MB spill-buffer=64.00KB thread-reservation=0
|  tuple-ids=2 row-size=24B cardinality=53.52K cost=315877
|  in pipelines: 00(GETNEXT)
|
02:HASH JOIN [INNER JOIN, BROADCAST]
|  hash-table-id=04
|  hash predicates: sr_returned_date_sk = d_date_sk
|  fk/pk conjuncts: sr_returned_date_sk = d_date_sk
|  mem-estimate=0B mem-reservation=0B spill-buffer=64.00KB thread-reservation=0
|  tuple-ids=0,1 row-size=24B cardinality=53.52K cost=23423
|  in pipelines: 00(GETNEXT), 01(OPEN)
|
|--F15:PLAN FRAGMENT [RANDOM] hosts=3 instances=3
|  |  Per-Instance Resources: mem-estimate=2.95MB mem-reservation=2.94MB thread-reservation=1 runtime-filters-memory=1.00MB
|  |  max-parallelism=3 segment-costs=[520]
|  JOIN BUILD
...



-- 
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: 13
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Apr 2024 15:45:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15852/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 14:25:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10522/ DRY_RUN=true


-- 
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 <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 14:06:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21257 )

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


Patch Set 17: Code-Review+2


-- 
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: 17
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 16:46:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#3).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Impala has a concept of "admission control slots" - the amount of
parallelism that should be allowed on an Impala daemon. This defaults to
the number of processors and can be overridden with
-–admission_control_slots flag.

Admission control slot accounting is described in IMPALA-8998. It
compute 'slots_to_use' for each backend based on the max number of
instances of any fragment on that backend. This is simplistic, because
multiple non-blocking fragments with the same number of instance counts,
say 4 non-blocking fragments each with 12 instances, only request the
max instance (12) admission slots rather than sum of it (48), making
each of 12 cores oversubscribed by 4x.

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represents the cpu requirement of that query over a
particular executor group set. This number is an estimation of the
largest number of query fragment instances that can run in parallel
without waiting, given by the blocking operator analysis. Therefore, the
fragment trace that sums into that CpuAsk number can be translated into
'slots_to_use' as well, which will be a closer resemblance of maximum
parallel execution of fragment instances.

This patch adds a new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which is the original algorithm from IMPALA-8998.
  This is still the default value for the SLOT_COUNT_STRATEGY option.
- PLANNER_CPU_ASK, which will follow fragment trace that contributes
  towards CpuAsk number.

To do the PLANNER_CPU_ASK strategy, the Planner will mark fragments that
contribute to CpuAsk as dominant fragments. It also passes
max_slot_per_executor information that it knows about the executor group
set to the scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
think the average 'slots_to_use' per backend will be. Actual
'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what scheduled on that
backend.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_tpcds_queries.py
16 files changed, 311 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/3
-- 
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/21257/4/be/src/scheduling/scheduler.cc@1169
PS4, Line 1169: 'dominant_instan
> 'dominant_instance_count'
Done


http://gerrit.cloudera.org:8080/#/c/21257/4/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/4/fe/src/main/java/org/apache/impala/planner/CoreCount.java@80
PS4, Line 80: a set of 
> This is not a copy. drop this.
Done


http://gerrit.cloudera.org:8080/#/c/21257/4/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/21257/4/fe/src/main/java/org/apache/impala/service/Frontend.java@2396
PS4, Line 2396: getAvgSlotsUsePerBac
> getAvgSlotsUsePerBackend
Done



-- 
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: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2024 22:46:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.

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

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


Patch Set 5:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG@11
PS5, Line 11: the number of processors and can be overridden with
processors per executor?


http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG@16
PS5, Line 16: instances of any fragment on that backend. This is simplistic, because
change "is simplistic" to "can lead to underestimation"


http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG@44
PS5, Line 44: AvgAdmissionSlotsPerExecutor
It's not clear how this is calculated. Is it the maximum expected parallelism from above?


http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG@45
PS5, Line 45: think
thinks


http://gerrit.cloudera.org:8080/#/c/21257/5//COMMIT_MSG@47
PS5, Line 47: AvgAdmissionSlotsPerExecutor, depending on what scheduled on that
is scheduled


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

http://gerrit.cloudera.org:8080/#/c/21257/5/be/src/scheduling/scheduler.cc@1184
PS5, Line 1184:     for (auto& finstance : backend.second.exec_params->instance_params()) {
This is kind of hard to follow. There seems be assumptions that fragment indexes are ordered and not unique.


http://gerrit.cloudera.org:8080/#/c/21257/5/be/src/scheduling/scheduler.cc@1193
PS5, Line 1193:       be_max_instances = max(be_max_instances, curr_instance_count);
Can this be moved out of the loop?


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

http://gerrit.cloudera.org:8080/#/c/21257/5/fe/src/main/java/org/apache/impala/planner/CostingSegment.java@84
PS5, Line 84:       Preconditions.checkState(!nodes_.isEmpty());
Can this just get computed once at the top and passed down as a recursion arg?


http://gerrit.cloudera.org:8080/#/c/21257/5/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/21257/5/fe/src/main/java/org/apache/impala/service/Frontend.java@2338
PS5, Line 2338: derived
is derived


http://gerrit.cloudera.org:8080/#/c/21257/5/fe/src/main/java/org/apache/impala/service/Frontend.java@2399
PS5, Line 2399: cores_requirement
may want to assert cores_requirement > 0 and numExecutors > 0 here to detect problems



-- 
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: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2024 14:26:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#8).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Impala has a concept of "admission control slots" - the amount of
parallelism that should be allowed on an Impala daemon. This defaults to
the number of processors per executor and can be overridden with
-–admission_control_slots flag.

Admission control slot accounting is described in IMPALA-8998. It
compute 'slots_to_use' for each backend based on the max number of
instances of any fragment on that backend. This can lead to
underestimation, because multiple non-blocking fragments with the same
number of instance counts, say 4 non-blocking fragments each with 12
instances, only request the max instance (12) admission slots rather
than sum of it (48), making each of 12 cores oversubscribed by 4x.

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represents the cpu requirement of that query over a
particular executor group set. This number is an estimation of the
largest number of query fragment instances that can run in parallel
without waiting, given by the blocking operator analysis. Therefore, the
fragment trace that sums into that CpuAsk number can be translated into
'slots_to_use' as well, which will be a closer resemblance of maximum
parallel execution of fragment instances.

This patch adds a new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which is the original algorithm from IMPALA-8998.
  This is still the default value for the SLOT_COUNT_STRATEGY option.
- PLANNER_CPU_ASK, which will follow fragment trace that contributes
  towards CpuAsk number.

To do the PLANNER_CPU_ASK strategy, the Planner will mark fragments that
contribute to CpuAsk as dominant fragments. It also passes
max_slot_per_executor information that it knows about the executor group
set to the scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

  AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

Actual 'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what is scheduled on that
backend. 'slots_to_use' will be shown as 'AdmissionSlots' counter under
each executor profile node.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_tpcds_queries.py
16 files changed, 347 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/8
-- 
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21257/11/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/21257/11/tests/custom_cluster/test_executor_groups.py@1245
PS11, Line 1245:     #   CoreCount={total=16 trace=F15:3+F01:1+F14:3+F03:1+F13:3+F05:1+F12:3+F07:1},
> I think that it would could be useful to be able to set a cost limit so tha
That is a great suggestion. Filed https://issues.apache.org/jira/browse/IMPALA-13010 to investigate this further.
Thanks!



-- 
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: 15
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Apr 2024 18:34:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Michael Smith, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#16).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Impala has a concept of "admission control slots" - the amount of
parallelism that should be allowed on an Impala daemon. This defaults to
the number of processors per executor and can be overridden with
-–admission_control_slots flag.

Admission control slot accounting is described in IMPALA-8998. It
computes 'slots_to_use' for each backend based on the maximum number of
instances of any fragment on that backend. This can lead to slot
underestimation and query overadmission. For example, assume an executor
node with 48 CPU cores and configured with -–admission_control_slots=48.
It is assigned 4 non-blocking query fragments, each has 12 instances
scheduled in this executor. IMPALA-8998 algorithm will request the max
instance (12) slots rather than the sum of all non-blocking fragment
instances (48). With the 36 remaining slots free, the executor can still
admit another fragment from a different query but will potentially have
CPU contention with the one that is currently running.

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represents the cpu requirement of that query over a
particular executor group set. This number is an estimation of the
largest number of query fragment instances that can run in parallel
without waiting, given by the blocking operator analysis. Therefore, the
fragment trace that sums into that CpuAsk number can be translated into
'slots_to_use' as well, which will be a closer resemblance of maximum
parallel execution of fragment instances.

This patch adds a new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which is the original algorithm from IMPALA-8998.
This is still the default value for the SLOT_COUNT_STRATEGY option.
- PLANNER_CPU_ASK, which will follow the fragment trace that contributes
towards CpuAsk number. This strategy will schedule more or equal
admission control slots than the LARGEST_FRAGMENT strategy.

To do the PLANNER_CPU_ASK strategy, the Planner will mark fragments that
contribute to CpuAsk as dominant fragments. It also passes
max_slot_per_executor information that it knows about the executor group
set to the scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

Actual 'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what is scheduled on that
backend. 'slots_to_use' will be shown as 'AdmissionSlots' counter under
each executor profile node.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.
- Add EE test test_processing_cost.py.
- Add FE test PlannerTest#testProcessingCostPlanAdmissionSlots.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
A testdata/workloads/functional-planner/queries/PlannerTest/processing-cost-plan-admission-slots.test
A testdata/workloads/functional-query/queries/QueryTest/processing-cost-admission-slots.test
M tests/custom_cluster/test_executor_groups.py
A tests/query_test/test_processing_cost.py
M tests/query_test/test_tpcds_queries.py
21 files changed, 1,505 insertions(+), 111 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/16
--
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 16
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

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

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


Patch Set 16: Code-Review+2

Carrying prior +2.


-- 
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: 16
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 16:44:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21257 )

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


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10517/ DRY_RUN=true


-- 
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: 8
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 06:24:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10522/


-- 
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 <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 19:28:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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 <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2024 01:46:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#10).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Impala has a concept of "admission control slots" - the amount of
parallelism that should be allowed on an Impala daemon. This defaults to
the number of processors per executor and can be overridden with
-–admission_control_slots flag.

Admission control slot accounting is described in IMPALA-8998. It
compute 'slots_to_use' for each backend based on the max number of
instances of any fragment on that backend. This can lead to
underestimation, because multiple non-blocking fragments with the same
number of instance counts, say 4 non-blocking fragments each with 12
instances, only request the max instance (12) admission slots rather
than sum of it (48), making each of 12 cores oversubscribed by 4x.

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represents the cpu requirement of that query over a
particular executor group set. This number is an estimation of the
largest number of query fragment instances that can run in parallel
without waiting, given by the blocking operator analysis. Therefore, the
fragment trace that sums into that CpuAsk number can be translated into
'slots_to_use' as well, which will be a closer resemblance of maximum
parallel execution of fragment instances.

This patch adds a new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which is the original algorithm from IMPALA-8998.
  This is still the default value for the SLOT_COUNT_STRATEGY option.
- PLANNER_CPU_ASK, which will follow fragment trace that contributes
  towards CpuAsk number.

To do the PLANNER_CPU_ASK strategy, the Planner will mark fragments that
contribute to CpuAsk as dominant fragments. It also passes
max_slot_per_executor information that it knows about the executor group
set to the scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

  AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

Actual 'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what is scheduled on that
backend. 'slots_to_use' will be shown as 'AdmissionSlots' counter under
each executor profile node.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/processing_cost_admission_slots.test
M tests/custom_cluster/test_executor_groups.py
A tests/query_test/test_processing_cost.py
M tests/query_test/test_tpcds_queries.py
19 files changed, 536 insertions(+), 96 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/10
-- 
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 10
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(7 comments)

http://gerrit.cloudera.org:8080/#/c/21257/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/1//COMMIT_MSG@30
PS1, Line 30: 
> I'm considering changing this name. Maybe simply CPU_ASK is better.
Renamed to PLANNER_CPU_ASK.


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

http://gerrit.cloudera.org:8080/#/c/21257/4/be/src/scheduling/scheduler.cc@1169
PS4, Line 1169: be_max_instances
'dominant_instance_count'


http://gerrit.cloudera.org:8080/#/c/21257/1/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/21257/1/common/thrift/Query.thrift@714
PS1, Line 714:   // See comment in ImpalaService.thrift
> line too long (92 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/21257/2/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/21257/2/common/thrift/Query.thrift@724
PS2, Line 724:   177: optional TSlotCountStrategy slot_count_strategy =
> line too long (92 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/21257/4/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/4/fe/src/main/java/org/apache/impala/planner/CoreCount.java@80
PS4, Line 80: a copy of
This is not a copy. drop this.


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

http://gerrit.cloudera.org:8080/#/c/21257/1/fe/src/main/java/org/apache/impala/planner/Planner.java@515
PS1, Line 515: 
             :     return root.maxCore(rootCores.first, CoreCount.sum(rootCores.second));
             :   }
             : 
             :   /**
             :    * Reduce plan node cardinalities based on runtime filter information.
             :    * Valid to call after runtime filter generation and before processing cost
             :    * computation.
             :    */
             :   public static void reduceCardinalityByRuntimeFilter(
             :       List<PlanFragment> planRoots, PlannerContext planCtx) {
             :     double reductionScale = planCtx.getRootAnalyzer()
             :                                 .getQueryOptions()
             :                                 .getRuntime_filter_cardinality_reduction_scale();
             :     if (reductionScale <= 0) return;
             :     PlanFragment rootFragment = planRoots.get(0);
             :     Stack<PlanNode> nodeS
> Move this logic to caller method, computeProcessingCost().
Done


http://gerrit.cloudera.org:8080/#/c/21257/4/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/21257/4/fe/src/main/java/org/apache/impala/service/Frontend.java@2396
PS4, Line 2396: getSlotUsePerBackend
getAvgSlotsUsePerBackend



-- 
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: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2024 22:39:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21257 )

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


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15813/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2024 23:06:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21257/7/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/7/fe/src/main/java/org/apache/impala/planner/CoreCount.java@137
PS7, Line 137: return (core1.total() < core2.total()) ? core2 : core1;
If core1 and core2 has equal total(), it should favor one that does not have coordinator fragment in it, since Planner may deduct it by 1.



-- 
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: 7
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 00:05:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG@19
PS11, Line 19:  rather
             : than sum of it (48)
> Can this get higher than the amount of slots per executor? Based on IMPALA-
It can be, if individual executor is configured with mismatch --admission_control_slots value than what is expressed in llama-site.xml of coordinator (field name like "impala.admission-control.max-query-cpu-core-per-node-limit.root.large", for root.large executor group).

This patch assume correct configuration between llama-site.xml of coordinator and --admission_control_slots value of each backend executors.


http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG@28
PS11, Line 28: which will be a closer resemblance of maximum
             : parallel execution of fragment instances.
> Does PLANNER_CPU_ASK always calculate a greater or equal number of slots th
It should be most of the time, except few corner cases such as dedicated coordinator setup. In that setup, there might be no dominant fragment assigned to coordinator at all. This is handled by L1027 at scheduler.cc, patch set 11.

Now that I think again, I might miss other corner cases where dominant_instance_count < be_max_instances, I just have not found such example. I think be_max_instances should win in that case. Will fix it in next patch set.



-- 
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: 11
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Apr 2024 16:09:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21257/12/tests/query_test/test_processing_cost.py
File tests/query_test/test_processing_cost.py:

http://gerrit.cloudera.org:8080/#/c/21257/12/tests/query_test/test_processing_cost.py@22
PS12, Line 22: from tests.common.test_dimensions import (
> flake8: F401 'tests.common.test_dimensions.add_mandatory_exec_option' impor
Done



-- 
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: 13
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Apr 2024 18:48:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

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

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


Patch Set 13:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/11//COMMIT_MSG@19
PS11, Line 19:  rather
             : than sum of it (48)
> It can be, if individual executor is configured with mismatch --admission_c
So if the configuration is correct, then cpuask will be always <= the number of cores?
I think that what needs a bit of explanation is "12 cores oversubscribed by 4x" - this suggests to me that this is inherently wrong, but if the number of cores is less than the number of parallel fragments (possible in queries with lot of small build fragments), then oversubscribing is normal, right?


http://gerrit.cloudera.org:8080/#/c/21257/13//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21257/13//COMMIT_MSG@37
PS13, Line 37: more
I think that it should be "more or equal"
(same applies to comment in Query.thrift)


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

http://gerrit.cloudera.org:8080/#/c/21257/13/be/src/scheduling/scheduler.cc@1209
PS13, Line 1209: in
nit: "is"


http://gerrit.cloudera.org:8080/#/c/21257/13/be/src/scheduling/scheduler.cc@1209
PS13, Line 1209: is
nit: "is" not needed



-- 
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: 13
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Apr 2024 06:52:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#15).

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

IMPALA-12980: Translate CpuAsk into admission control slots

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/15
--
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 15
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

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

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21257/10/testdata/workloads/functional-query/queries/QueryTest/processing_cost_admission_slots.test
File testdata/workloads/functional-query/queries/QueryTest/processing_cost_admission_slots.test:

http://gerrit.cloudera.org:8080/#/c/21257/10/testdata/workloads/functional-query/queries/QueryTest/processing_cost_admission_slots.test@31
PS10, Line 31: row_regex: .* AdmissionSlots: 8 .*
             : row_regex: .* AdmissionSlots: 4 .*
This assertion failed in dockerised environment.
In regular 3 node minicluster, fragment assignment is 8, 4, 4.
In dockerised env, fragment assignment is 7, 4, 5.
Will remove this. The assertion in L26 should be sufficient.



-- 
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: 10
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Apr 2024 13:21:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#9).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Impala has a concept of "admission control slots" - the amount of
parallelism that should be allowed on an Impala daemon. This defaults to
the number of processors per executor and can be overridden with
-–admission_control_slots flag.

Admission control slot accounting is described in IMPALA-8998. It
compute 'slots_to_use' for each backend based on the max number of
instances of any fragment on that backend. This can lead to
underestimation, because multiple non-blocking fragments with the same
number of instance counts, say 4 non-blocking fragments each with 12
instances, only request the max instance (12) admission slots rather
than sum of it (48), making each of 12 cores oversubscribed by 4x.

When COMPUTE_PROCESSING_COST is enabled, Planner will generate a CpuAsk
number that represents the cpu requirement of that query over a
particular executor group set. This number is an estimation of the
largest number of query fragment instances that can run in parallel
without waiting, given by the blocking operator analysis. Therefore, the
fragment trace that sums into that CpuAsk number can be translated into
'slots_to_use' as well, which will be a closer resemblance of maximum
parallel execution of fragment instances.

This patch adds a new query option called SLOT_COUNT_STRATEGY to control
which admission control slot accounting to use. There are two possible
values:
- LARGEST_FRAGMENT, which is the original algorithm from IMPALA-8998.
  This is still the default value for the SLOT_COUNT_STRATEGY option.
- PLANNER_CPU_ASK, which will follow fragment trace that contributes
  towards CpuAsk number.

To do the PLANNER_CPU_ASK strategy, the Planner will mark fragments that
contribute to CpuAsk as dominant fragments. It also passes
max_slot_per_executor information that it knows about the executor group
set to the scheduler.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

  AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

Actual 'slots_to_use' in each backend may differ than
AvgAdmissionSlotsPerExecutor, depending on what is scheduled on that
backend. 'slots_to_use' will be shown as 'AdmissionSlots' counter under
each executor profile node.

Testing:
- Update test_executors.py with AvgAdmissionSlotsPerExecutor assertion.
- Pass test_tpcds_queries.py::TestTpcdsQueryWithProcessingCost.

Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Planner.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/planner/CoreCount.java
M fe/src/main/java/org/apache/impala/planner/CostingSegment.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_tpcds_queries.py
17 files changed, 349 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/9
-- 
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 9
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.

Wenzhe Zhou 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: Code-Review+1


-- 
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 <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Apr 2024 22:29:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12980: Translate CpuAsk into admission control slots

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Kurt Deschler, Abhishek Rawat, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/21257

to look at the new patch set (#14).

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

IMPALA-12980: Translate CpuAsk into admission control slots

Admission control slot accounting is described in IMPALA-8998. It
computes 'slots_to_use' for each backend based on the maximum number of
instances of any fragment on that backend. This can lead to slot
underestimation query overadmission. For example, assume an executor
node with 48 CPU cores and configured with -–admission_control_slots=48.
It is assigned 4 non-blocking query fragments, each has 12 instances
scheduled in this executor. IMPALA-8998 algorithm will request the max
instance (12) slots rather than the sum of all non-blocking fragment
instances (48). With the 36 remaining slots free, the executor can still
admit another fragment from a different query but will potentially have
CPU contention with the one that is currently running.

AvgAdmissionSlotsPerExecutor counter is added to describe what Planner
thinks the average 'slots_to_use' per backend will be, which follows
this formula:

AvgAdmissionSlotsPerExecutor = ceil(CpuAsk / num_executors)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/21257/14
--
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: newpatchset
Gerrit-Change-Id: I338ca96555bfe8d07afce0320b3688a0861663f2
Gerrit-Change-Number: 21257
Gerrit-PatchSet: 14
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>