Yongjun Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11591 )

Change subject: IMPALA-6742: Profiles of running queries should include 
execution summary.
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11591/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11591/1//COMMIT_MSG@9
PS1, Line 9: Currently execution summary is not included in the profiles of 
running
           : queries, and it's only reported when the query is finished. This 
jira makes
           : the execution summary to the profile reported wh
> Use shorter lines. My recommendation is to wrap at 70 characters.
Thanks, addressed in new rev.


http://gerrit.cloudera.org:8080/#/c/11591/1/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/11591/1/be/src/service/impala-server.h@637
PS1, Line 637: void UpdateE
> You don't want [[noreturn]]
Thanks Joe, my misunderstanding of this annotation. Fixed in new rev.


http://gerrit.cloudera.org:8080/#/c/11591/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/11591/1/be/src/service/impala-server.cc@1128
PS1, Line 1128:                    << PrettyPrinter::Print(cpu_limit_
> I'm thinking this might not hold for some calls to GetRuntimeProfileStr() t
Good catch Joe. Indeed. Fixed in new rev by adding a check.

I tried to run test_observability both locally and at jenkins, the former had 
some failures, however, the latter is clean. Looking into why it failed locally.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699
Gerrit-Change-Number: 11591
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Yongjun Zhang <[email protected]>
Gerrit-Comment-Date: Tue, 30 Oct 2018 21:51:06 +0000
Gerrit-HasComments: Yes

Reply via email to