Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11387 )
Change subject: IMPALA-6568 add missing Query Compilation section to profiles. ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/11387/3/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/11387/3/fe/src/main/java/org/apache/impala/service/Frontend.java@a1069 PS3, Line 1069: > so was this the main culprit (early return) for omitting the timeline for a Yes, the early returns were not setting the timeline correctly, which is the source of the bug. Do you want me to split out the cleanup into a separate jira so it can be better reviewed? http://gerrit.cloudera.org:8080/#/c/11387/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1034 PS3, Line 1034: t > nit: uppercase to getTExecRequest Done http://gerrit.cloudera.org:8080/#/c/11387/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1035 PS3, Line 1035: StringBuilder explainString) > indentation here should be 4 spaces from the beginning of the line I think you mean 6 spaces from the beginning of the line? So 4 spaces indented from the method's indentation? I see I was doing this wrong everywhere so I fixed the other cases too. http://gerrit.cloudera.org:8080/#/c/11387/3/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/11387/3/tests/query_test/test_observability.py@221 PS3, Line 221: def test_query_profile_contains_all_events(self): > use unique_database fixture here so that the various objects here are scope Thanks for making me learn about pytest :-) http://gerrit.cloudera.org:8080/#/c/11387/3/tests/query_test/test_observability.py@224 PS3, Line 224: self.hdfs_client.delete_file_dir('tmp/impala6568') : se > would it simplify things here if you used an existing file? I have tidied this up so it is more consistent with other tests -- 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: comment Gerrit-Change-Id: I869eaeb4be4291b6b938f91847f624c76ec90ea5 Gerrit-Change-Number: 11387 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 07 Sep 2018 23:40:06 +0000 Gerrit-HasComments: Yes
