[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Tim Armstrong (Code Review)
Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Also includes a fix for IMPALA-9054 where we increase
the timeout.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Reviewed-on: http://gerrit.cloudera.org:8080/14357
Tested-by: Impala Public Jenkins 
Reviewed-by: Tim Armstrong 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_cancellation.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
20 files changed, 416 insertions(+), 88 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 22
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 21: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 21
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 20:26:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 21: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 21
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 20:18:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 19:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4808/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 19
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 16:44:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 21
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 16:03:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 20:

Ran into IMPALA-9054, including a fix


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 16:03:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Also includes a fix for IMPALA-9054 where we increase
the timeout.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_cancellation.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
20 files changed, 416 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/20
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 20
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-16 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_cancellation.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
20 files changed, 416 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/19
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 19
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 18: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 04:48:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 18:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 00:33:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 18: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 Oct 2019 00:33:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-15 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 17: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 Oct 2019 23:51:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 17:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4776/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 10 Oct 2019 00:35:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-09 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 17:

Rebased onto https://gerrit.cloudera.org/#/c/14384/ and pulled in the test 
change to mt-dop-parquet-admission-slots.test from the older version of that 
patch


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 09 Oct 2019 23:56:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-09 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 411 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/17
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 16: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 08:54:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 15:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4745/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 05:06:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 412 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/15
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 16:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 04:27:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc@247
PS12, Line 247: const string HOST_SLOT_NOT_AVAILABLE = "Not enough query slots 
available on host $0. "
> I think we should remove all usages of the term 'query slot'.
Done. Good point.


http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc@573
PS12, Line 573:  << " admission_slots=" << admission_slots;
> Nit: "executor admission_slots" might be more readable
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 08 Oct 2019 04:24:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 13:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4736/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 17:05:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc@247
PS12, Line 247: const string HOST_SLOT_NOT_AVAILABLE = "Not enough query slots 
available on host $0. "
I think we should remove all usages of the term 'query slot'.
The new terms 'admission control slot' and 'admission slot' are clearer, and we 
should use those. Ideally we might just use one of these two newer terms, but 
they are pretty clear as is.


http://gerrit.cloudera.org:8080/#/c/14357/12/be/src/scheduling/admission-controller.cc@573
PS12, Line 573:  << " admission_slots=" << admission_slots;
Nit: "executor admission_slots" might be more readable



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 17:00:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 14:

Disabled the test temporarily


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 16:25:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 14:

Here's the WIP of the follow-on scheduling fix 
https://gerrit.cloudera.org/#/c/14381/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 16:32:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 410 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/13
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14357/12/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
File 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test:

http://gerrit.cloudera.org:8080/#/c/14357/12/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test@29
PS12, Line 29: row_regex:.*AdmissionSlots: 2.*
> This appears to be somewhat flaky - the scheduling isn't consistent on diff
I filed IMPALA-9015. I have an initial version of a fix for this that solves 
the problem. I think I might remove these checks and add them back as part of 
the fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 07 Oct 2019 16:04:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-05 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14357/12/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
File 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test:

http://gerrit.cloudera.org:8080/#/c/14357/12/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test@29
PS12, Line 29: row_regex:.*AdmissionSlots: 2.*
This appears to be somewhat flaky - the scheduling isn't consistent on 
different systems. I think this is actually a deficiency in the scheduling 
algorithm, where it doesn't guarantee that each instance gets a scan range. So 
I'm gonna look at changing that before merging this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 05 Oct 2019 17:16:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 12:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4726/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Oct 2019 01:14:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 11: Code-Review+1

(5 comments)

Carry bikram's +1

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

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/scheduler.cc@689
PS8, Line 689:   for (auto& backend : per_backend_params) {
 : int be_max_instances = 0;
 : // Instances for a fragment are clustered together because 
of how the vector is
 : // constructed above. So we can compute the max # of 
instances of any fragment
 : // with a single pass over the vector.
 : const FragmentExecParams* curr_fragment = nullptr;
 : int curr_instance_count = 0; // Number of instances of the 
current fragment seen.
 : for (auto& finstance : backend.second.instance_params) {
 :   if (curr_fragment == nullptr ||
 :   curr_fragment != >fragment_exec_params) {
 : curr_fragment = >fragment_exec_params;
 : curr_instance_count = 0;
 :   }
 :   ++curr_instance_count;
 :   be_max_instances = max(be_max_instances, 
curr_instance_count);
 : }
 : backend.second.slots_to_use = be_max_instances;
 :   }
> nit: seems like we are iterating over per_backend_params a few times after
I'd prefer multiple passes that do one thing each rather than a single pass 
that does multiple things. I don't think the loop overhead should be 
significant.


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py@882
PS8, Line 882: Test that queue details appear in the profile when queued based 
on num_queries
> nit: update description
Done


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py@903
PS8, Line 903: num_queries
> nit: update message
Done


http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/metric_verifier.py@69
PS11, Line 69: wait_for_backend_admission_control_state
> nit: (feel free to ignore) how about "wait_for_idle_backend_state"?
I think I'll stick with this name, even though it's a little awkward. I kinda 
wanted capture that it wasn't the state of the actual backends, but rather the 
admission control state.


http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/test_verify_metrics.py
File tests/verifiers/test_verify_metrics.py:

http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/test_verify_metrics.py@48
PS11, Line 48: test_verify_backends
> nit: (feel free to ignore) how about "test_backends_are_idle"
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 04 Oct 2019 00:32:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 410 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/12
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 11:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4725/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:50:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 9:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4724/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:48:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 11: Code-Review+1

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/scheduler.cc@689
PS8, Line 689:   for (auto& backend : per_backend_params) {
 : int be_max_instances = 0;
 : // Instances for a fragment are clustered together because 
of how the vector is
 : // constructed above. So we can compute the max # of 
instances of any fragment
 : // with a single pass over the vector.
 : const FragmentExecParams* curr_fragment = nullptr;
 : int curr_instance_count = 0; // Number of instances of the 
current fragment seen.
 : for (auto& finstance : backend.second.instance_params) {
 :   if (curr_fragment == nullptr ||
 :   curr_fragment != >fragment_exec_params) {
 : curr_fragment = >fragment_exec_params;
 : curr_instance_count = 0;
 :   }
 :   ++curr_instance_count;
 :   be_max_instances = max(be_max_instances, 
curr_instance_count);
 : }
 : backend.second.slots_to_use = be_max_instances;
 :   }
nit: seems like we are iterating over per_backend_params a few times after its 
construction. Do you think we can merge them or would that impact readability?


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py@882
PS8, Line 882: Test that queue details appear in the profile when queued based 
on num_queries
nit: update description


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_admission_controller.py@903
PS8, Line 903: num_queries
nit: update message


http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/metric_verifier.py@69
PS11, Line 69: wait_for_backend_admission_control_state
nit: (feel free to ignore) how about "wait_for_idle_backend_state"?


http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/test_verify_metrics.py
File tests/verifiers/test_verify_metrics.py:

http://gerrit.cloudera.org:8080/#/c/14357/11/tests/verifiers/test_verify_metrics.py@48
PS11, Line 48: test_verify_backends
nit: (feel free to ignore) how about "test_backends_are_idle"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:43:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 406 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/9
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

--admission_control_slots is added as a new flag that
replaces --max_concurrent_queries, since the name better
reflects the concept. --max_concurrent_queries is kept
for backwards compatibility and has the same meaning
as --admission_control_slots.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/util/auto_scaler.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
19 files changed, 409 insertions(+), 85 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/11
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 8:

(7 comments)

I won't carry +1 since the startup flag change was kinda significant.

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.h@267
PS8, Line 267:   /// The maximum number of admission slots that should be used 
on this host.
> Maybe explain (here or elsewhere) what "slot" means, e.g. that it's concept
I added a bit more of an explanation here. I don't want to get too in depth in 
this comment, but agree some more context is useful. Tried to make it roughly 
consistent with admit_mem_limit as far as the detail level.


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.cc@90
PS8, Line 90: "(Advanced) The maximum degree of parallelism to run queries 
with on this backend. "
> Maybe we should create a new flag with a better name and the same behavior,
That's a good point. I just added a new flag that takes precedence over the old 
one. We can probably leave the old one around indefinitely TBH, the code to do 
the fallback isn't complex.


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.h@252
PS8, Line 252:
> nit: double space
Done


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.cc@563
PS8, Line 563: HasAvailableSlot
> nit: rename to HasAvailableSlots()?
Done


http://gerrit.cloudera.org:8080/#/c/14357/8/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
File 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test:

http://gerrit.cloudera.org:8080/#/c/14357/8/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test@3
PS8, Line 3: 4
> This is because we set mt_dop=4 and 24/3 > 4, right? It might be good to me
Done


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_executor_groups.py@218
PS8, Line 218: gets
> nit: get
Done


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/verifiers/metric_verifier.py@69
PS8, Line 69: wait_for_backends_state
> maybe this method could have a more descriptive name?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:07:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14357/9/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14357/9/tests/custom_cluster/test_executor_groups.py@57
PS9, Line 57: t
flake8: E501 line too long (91 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/14357/9/tests/custom_cluster/test_executor_groups.py@67
PS9, Line 67: ]
flake8: E501 line too long (91 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:07:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 8: Code-Review+1

(7 comments)

LGTM, only had a bunch of nits

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.h@267
PS8, Line 267:   /// The maximum number of admission slots that should be used 
on this host.
Maybe explain (here or elsewhere) what "slot" means, e.g. that it's 
conceptually equivalent with the number of threads or cpu cores?


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/runtime/exec-env.cc@90
PS8, Line 90: "(Advanced) The maximum degree of parallelism to run queries 
with on this backend. "
Maybe we should create a new flag with a better name and the same behavior, and 
then deprecate this one on the next major release. With the slot concept, its 
name is somewhat awkward. If we already mark it as deprecated in 3.4, hopefully 
not many people would start using it.


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.h@252
PS8, Line 252:
nit: double space


http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/14357/8/be/src/scheduling/admission-controller.cc@563
PS8, Line 563: HasAvailableSlot
nit: rename to HasAvailableSlots()?


http://gerrit.cloudera.org:8080/#/c/14357/8/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
File 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test:

http://gerrit.cloudera.org:8080/#/c/14357/8/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test@3
PS8, Line 3: 4
This is because we set mt_dop=4 and 24/3 > 4, right? It might be good to 
mention that mt_dop is 4 in the comment.


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/custom_cluster/test_executor_groups.py@218
PS8, Line 218: gets
nit: get


http://gerrit.cloudera.org:8080/#/c/14357/8/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/8/tests/verifiers/metric_verifier.py@69
PS8, Line 69: wait_for_backends_state
maybe this method could have a more descriptive name?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 21:11:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4718/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 18:05:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4719/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:59:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4717/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:32:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
18 files changed, 360 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/8
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4716/ : 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/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:15:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14357/7/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/7/tests/verifiers/metric_verifier.py@72
PS7, Line 72: L
flake8: F821 undefined name 'LOG'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:13:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 7:

I think this is ready for review.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:13:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
18 files changed, 361 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/7
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14357 )

Change subject: IMPALA-8998: admission control accounting for mt_dop
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14357/6/tests/verifiers/metric_verifier.py
File tests/verifiers/metric_verifier.py:

http://gerrit.cloudera.org:8080/#/c/14357/6/tests/verifiers/metric_verifier.py@72
PS6, Line 72: L
flake8: F821 undefined name 'LOG'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 03 Oct 2019 17:12:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8998: admission control accounting for mt dop

2019-10-03 Thread Tim Armstrong (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-8998: admission control accounting for mt_dop
..

IMPALA-8998: admission control accounting for mt_dop

This integrates mt_dop with the "slots" mechanism that's used
for non-default executor groups.

The idea is simple - the degree of parallelism on a backend
determines the number of slots consumed. The effective
degree of parallelism is used, not the raw mt_dop setting.
E.g. if the query only has a single input split and executes
only a single fragment instance on a host, we don't want
to count the full mt_dop value for admission control.

The admission control logic is extended to take this into
account. We also add an immediate rejection code path
since it is now possible for queries to not be admittable
based on the # of available slots.

We only factor in the "width" of the plan - i.e. the number
of instances of fragments. We don't account for the number
of distinct fragments, since they may not actually execute
in parallel with each other because of dependencies.

This number is added to the per-host profile as the
"AdmissionSlots" counter.

Testing:
Added unit tests for rejection and queue/admit checks.

Added end-to-end tests:
* test_admission_slots in test_mt_dop.py that checks the
  admission slot calculation via the profile.
* End-to-end admission test that exercises the admit
  immediately and queueing code paths.

Added checks to test_verify_metrics (which runs after
end-to-end tests) to ensure that the per-backend
slots in use goes to 0 when the cluster is quiesced.

Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M common/thrift/StatestoreService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-admission-slots.test
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_executor_groups.py
M tests/query_test/test_mt_dop.py
M tests/verifiers/metric_verifier.py
M tests/verifiers/test_verify_metrics.py
M www/backends.tmpl
18 files changed, 361 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14357/6
--
To view, visit http://gerrit.cloudera.org:8080/14357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b6b6262ef238df26b491352656a26e4163e46e5
Gerrit-Change-Number: 14357
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker