Vuk Ercegovac 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 all cases (and on L1113)? Main fix is the refactor on L1034. wondering if it makes more sense to keep this change small and deal with additional cleanup separately. 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 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 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 scoped to the test (and cleaned up). 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? -- 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 <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 07 Sep 2018 18:18:36 +0000 Gerrit-HasComments: Yes
