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/06/09 22:47:48 UTC

[Impala-ASF-CR] IMPALA-9692 (part 3): Model QuerySchedule as a protobuf

Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9692 (part 3): Model QuerySchedule as a protobuf
......................................................................

IMPALA-9692 (part 3): Model QuerySchedule as a protobuf

In order to support the new admission control service, we need to be
able to return the results of an admission attempt, i.e. the query
schedule, to the coordinator.

To enable this, this patch moves all parts of the QuerySchedule class
and related classes that are required by the coordinator into a new
message QuerySchedulePB. The main admission control interface,
SubmitForAdmission(), now returns a QuerySchedulePB.

Some notable changes:
- Previously, QuerySchedule was used by Coordinator as a way to pass
  around references to parts of the TExecRequest to places like
  Coordinator::ExecSummary and Coordinator::BackendState. This has
  been replaced with the ExecParams class, which is a container for
  references to the TExecRequest and QuerySchedulePB along with
  convenience functions for accessing them.
- Similarly, FragmentExecParams, which is part of QuerySchedule,
  contains references to the associated TPlanFragment, owned by the
  TExecRequest, which were used by the Coordinator when iterating over
  the schedule to initiate the query. Since FragmentExecParamsPB can't
  contain these references, they were replaced by a map between
  fragment idx and TPlanFragment in ExecParams.
- AdmissionController::ReleaseQuery() and ReleaseQueryBackend() now
  take a query id as a parameter instead of a QuerySchedule. To
  facilitate this, AdmissionController now maintains a map from query
  ids of running queries to the resources that were allocated for them
  so that it can look the resources up when releasing them. This map
  will be necessary when implementing the admission control service to
  facilitate proper accounting of resouces in cases like coordinator
  failures.
- As scheduling is currently organized, we first construct the
  FragmentExecParams with the FInstanceExecParams as their children,
  then we construct the BackendExecParams which get references to
  their FInstanceExecParams. Since we can't send references like these
  through an rpc, we know instead Swap() the FInstanceExecParamsPB
  into the BackendExecParamsPB.

Testing:
- Updated related tests.
- Passed a full run of existing tests.

Change-Id: I1db64e72f84604b1d8ac24e0bdd4ad6bedd6bcd9
---
M be/src/runtime/CMakeLists.txt
M be/src/runtime/coordinator-backend-resource-state.cc
M be/src/runtime/coordinator-backend-state-test.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
A be/src/runtime/exec-params.cc
A be/src/runtime/exec-params.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/query-driver.cc
M be/src/scheduling/CMakeLists.txt
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/cluster-membership-mgr-test.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/cluster-membership-test-util.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/debug-util.h
M be/src/util/uid-util.h
M common/protobuf/CMakeLists.txt
A common/protobuf/admission_control_service.proto
34 files changed, 1,057 insertions(+), 775 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/15961/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15961
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1db64e72f84604b1d8ac24e0bdd4ad6bedd6bcd9
Gerrit-Change-Number: 15961
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>