You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2018/09/05 19:22:46 UTC

[Impala-ASF-CR] WIP: IMPALA-6568 add missing Query Compilation section to profiles.

Andrew Sherman has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11387 )

Change subject: WIP: IMPALA-6568 add missing Query Compilation section to profiles.
......................................................................

WIP: IMPALA-6568 add missing Query Compilation section to profiles.

WIP: so Todd can look at refactor
TODO: change tests to check that "Planning finished" is present for explain queries

The profile command is used to display low-level information about the
most recent query. When a client makes request for the Profile, it
sends a GetRuntimeProfile request for the last queryId to to the server.
The queryId is used to find the ClientRequestState, an object which tracks
information about the execution, including Profile data which is stored in
several RuntimeProfile objects. The reply to the GetRuntimeProfile message
contains the Profile, pretty-printed as a string.

When a query is sent to the Front End to be compiled, the TExecRequest
that is returned from createExecRequest() in the JVM contains a Timeline,
which is a named sequence of events with timing information. This Timeline
is added to the Summary Profile in the ClientRequestState.

In the following cases the Front End was not setting the Timeline in the TExecRequest:
 - All DDL operations
 - Load data statements
 - Set statements
 - Explain statements
And this meant that the profile would not contain the "Query Compilation" timeline.

ALTERNATIVES:

To fix this I set the Timeline in TExecRequest before returning it.

>    result.setTimeline(timeline.toThrift());
>    return result;

I considered writing this as

>    return result.setTimeline(timeline.toThrift());

To try to make sure this code gets copied in any future return statements,
but I didn’t think it was worth the cost to readability.

REFACTOR

I refactored the main createExecRequest() method to try to make the flow clearer.

TESTING:

Add a new test to test_observability.py which checks that the "Query
Compilation" timeline appears in the profile for various queries designed
to exercise the new code paths in createExecRequest.

Change-Id: I869eaeb4be4291b6b938f91847f624c76ec90ea5
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
2 files changed, 134 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/11387/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11387
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I869eaeb4be4291b6b938f91847f624c76ec90ea5
Gerrit-Change-Number: 11387
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>