You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sailesh Mukil (Code Review)" <ge...@cloudera.org> on 2018/07/03 16:49:18 UTC

[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class

Hello Michael Ho, Joe McDonnell, Dan Hecht, 

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

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

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

Change subject: IMPALA-7163: Implement a state machine for the QueryState class
......................................................................

IMPALA-7163: Implement a state machine for the QueryState class

This patch adds a state machine for the QueryState class. The motivation
behind this patch is to make the query lifecycle from the point of
view of an executor much easier to reason about and this patch is key
for a follow on patch for IMPALA-2990 where the status reporting will
be per-query rather than per-fragment-instance. Currently, the state
machine provides no other purpose, and it will mostly be used for
IMPALA-2990.

We introduce 7 possible states for the QueryState which include 3
terminal states (FINISHED, CANCELLED and ERROR) and 4 non-terminal
states (INITIALIZED, PREPARED). The transition from one state to the
next is always handled by a single thread which is also the QueryState
thread. This thread will additionally bear the purpose of sending
periodic updates after IMPALA-2990, which is the primary reason behind
having only this thread modify the state of the query.

2 counting barriers are introduced which are used to indicate how many
fragment instances have finished Preparing and Executing. We
use CountingBarriers to allow fragment instance threads to communicate
their completion of their respective states or errors with the QueryState
thread. Due to this design, it's possible for the fragment instances to
have collectively moved on to future states while the query state thread
lags behind in realizing that. However, that's not a concern for us
since we only care about showing a unified view of what's happening on
this executor to the coordinator (post IMPALA-2990).

The status reporting protocol has not been changed and remains exactly
as it was.

Testing: Ran 'core' and 'exhaustive' tests. TODO: Run stress tests.

Future related work:
1) IMPALA-2990: Make status reporting per-query.
2) Try to logically align the FIS states with the QueryState states.
3) Consider mirroring the QueryState state machine to CoordinatorBackendState

Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
---
M be/src/runtime/coordinator.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
5 files changed, 328 insertions(+), 38 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>