You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2020/11/18 22:46:22 UTC

[Impala-ASF-CR] IMPALA-9930 (part 2): Introduce new admission control rpc service

Hello Sahil Takiar, Joe McDonnell, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service
......................................................................

IMPALA-9930 (part 2): Introduce new admission control rpc service

This patch introduces a new krpc service, AdmissionControlService,
which coordinators can use to submit queries for admission.

This patch adds some simple configuration flags that make it possible
to have coordinators use this service to submit their queries for
admission to other coordinators. These flags are only to make this
patch testable will be replaced when the separate admission control
daemon is introduced in IMPALA-9975.

The interface consists of the following RPCs:
- AdmitQuery: takes a TQueryExecRequest and a TQueryOptions
  (serialized into sidecars), places the request on a queue to be
  processed by a thread pool and then immediately returns.
- GetQueryStatus: takes a query id and returns the current admission
  status, including the QuerySchedulePB if admission has completed
  successfully but the query has not been released yet.
- ReleaseQueryBackends: called when individual backends complete but
  the overall query is still running to release resources
  incrementally. This RPC will be called at most O(log(# backends))
  per query due to BackendResourceState, which batches backends to
  release together.
- ReleaseQuery: called when the query has completely finished.
  Releases all remaining resources.
- CancelAdmission: called if a query is cancelled before an admission
  decision has been made to indicate that it should no longer be
  considered for admission.

The majority of the patch consists of two classes:
- AdmissionControlClient: used to abstract whether admission is being
  performed locally or remotely. In the local case, it is basically
  just a wrapper around AdmissionController. In the remote case, it
  handles serializing/deserializing of RPC params, polling
  GetQueryStatus() until a decision has been made, etc.
- AdmissionControlService: exports the RPC interface and acts as a
  wrapper around AdmissionController.

Some notable changes involved:
- AdmissionController::SubmitForAdmission() no longer blocks while a
  query is queued. Instead, a new function CheckQueued() can be used
  to monitor the admission status of a queued query.
- Adding events to the query timeline is moved out of
  AdmissionController and into the AdmissionControlClient classes, so
  that it always happens on the coordinator.
- When a cluster is run in the new admission control service mode,
  only the impalad that is performing admission control exposes the
  /admission http endpoint. Observability will be cleaned up in a
  subsequent patch.

Testing:
- Modified existing admission control tests to run both with and
  without the admission control service enabled, including both the
  functional and stress tests. The 'num_queries' param in the stress
  test is modified to only use a single value to reduce the number of
  tests that are run and keep the running time reasonable.
- Ran tpch10 on a local minicluster and observed no significant
  regressions.

Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-control-client.cc
M be/src/scheduling/admission-control-client.h
A be/src/scheduling/admission-control-service.cc
A be/src/scheduling/admission-control-service.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/local-admission-control-client.cc
M be/src/scheduling/local-admission-control-client.h
A be/src/scheduling/remote-admission-control-client.cc
A be/src/scheduling/remote-admission-control-client.h
M be/src/scheduling/schedule-state.cc
M be/src/scheduling/schedule-state.h
M be/src/service/client-request-state.cc
M be/src/service/impala-http-handler.cc
M be/src/util/sharded-query-map-util.cc
M common/protobuf/admission_control_service.proto
M tests/common/resource_pool_config.py
M tests/custom_cluster/test_admission_controller.py
M tests/hs2/hs2_test_suite.py
M tests/util/web_pages_util.py
24 files changed, 1,240 insertions(+), 190 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae
Gerrit-Change-Number: 16412
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>