[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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 6: > Change has been successfully rebased and submitted as > e9652a48dd00c3c076ddccaa940d074b6970b7fc by Joe McDonnell Many thanks Joe and Tim for the review and commit! --Yongjun -- 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: 6 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Wed, 09 Jan 2019 17:31:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11591 ) Change subject: IMPALA-6742: Profiles of running queries should include execution summary. .. IMPALA-6742: Profiles of running queries should include execution summary. 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 when queries are still running. Testing: Added unit test. Done with real cluster. Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Reviewed-on: http://gerrit.cloudera.org:8080/11591 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/query_test/test_observability.py 3 files changed, 42 insertions(+), 7 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- 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: merged Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Gerrit-Change-Number: 11591 Gerrit-PatchSet: 6 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 5: Verified+1 -- 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: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Sat, 05 Jan 2019 03:29:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1717/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 23:54:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3613/ DRY_RUN=true -- 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: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 23:19:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Joe McDonnell 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 5: Code-Review+2 -- 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: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 23:35:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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 5: > Patch Set 4: Code-Review+1 Many thanks again for the review Joe. I just updated a new rev that fixes the flake8 issue. -- 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: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 23:19:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Yongjun Zhang has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11591 ) Change subject: IMPALA-6742: Profiles of running queries should include execution summary. .. IMPALA-6742: Profiles of running queries should include execution summary. 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 when queries are still running. Testing: Added unit test. Done with real cluster. Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 --- M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/query_test/test_observability.py 3 files changed, 42 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11591/5 -- 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: newpatchset Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Gerrit-Change-Number: 11591 Gerrit-PatchSet: 5 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Joe McDonnell 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 4: > > I would like there to be a test that breaks if this change is not > > there (or somehow this breaks later). > > query_test/test_observability.py is the right place to add this > > test. > > Here are a couple ways to do this: > > 1. Use a sleep in the query. See query_test/test_rows_availability.py: > > "select * from functional.alltypestiny where month = 1 and > bool_col > > = sleep(1000);" will take two seconds. Read that test and > > understand why the sleep works. > > 2. Execute a query that returns rows, and get the profile before > > fetching any of the rows. If we haven't called fetch, then the > > client has not called ImpalaServer::UnregisterQuery(). > > Of these, I think I prefer #2. See test_exec_summary() for an > > example. > > Unrelated to your change, I would like you to hand test your > change > > by putting a sleep between these two lines: > > https://github.com/apache/impala/blob/master/be/src/service/client-request-state.cc#L516-L517 > > Then fetch the profile and verify it doesn't crash. I have a > theory > > that this might not work. > Hi Joe, > > Thanks a lot for the review and comments and sorry for getting back > to this late. Very good suggestions! > > I just uploaded a new rev to address it by adding a new unit test > per your comment #2. I also did the manual test of adding a sleep > call between L516-L517, I did not observe behavior difference. > Would you please help taking a look? > > Thanks. To be clear, for the test of the sleep between L516-L517, you fetched the profile while it was sleeping between those two lines? Either way, the code change looks good. Please fix the flake8 comment, rebase and rerun tests, then I will +2. -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 20:51:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Joe McDonnell 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 4: Code-Review+1 -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 04 Jan 2019 20:52:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 4: Verified+1 -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 08:16:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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 4: BTW, the newly added unit test does fail without my fix. Thanks. -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 05:43:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1678/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 04:52:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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 4: > I would like there to be a test that breaks if this change is not > there (or somehow this breaks later). > query_test/test_observability.py is the right place to add this > test. > Here are a couple ways to do this: > 1. Use a sleep in the query. See query_test/test_rows_availability.py: > "select * from functional.alltypestiny where month = 1 and bool_col > = sleep(1000);" will take two seconds. Read that test and > understand why the sleep works. > 2. Execute a query that returns rows, and get the profile before > fetching any of the rows. If we haven't called fetch, then the > client has not called ImpalaServer::UnregisterQuery(). > Of these, I think I prefer #2. See test_exec_summary() for an > example. > Unrelated to your change, I would like you to hand test your change > by putting a sleep between these two lines: > https://github.com/apache/impala/blob/master/be/src/service/client-request-state.cc#L516-L517 > Then fetch the profile and verify it doesn't crash. I have a theory > that this might not work. Hi Joe, Thanks a lot for the review and comments and sorry for getting back to this late. Very good suggestions! I just uploaded a new rev to address it by adding a new unit test per your comment #2. I also did the manual test of adding a sleep call between L516-L517, I did not observe behavior difference. Would you please help taking a look? Thanks. -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 04:19:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11591/4/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/11591/4/tests/query_test/test_observability.py@228 PS4, Line 228: n flake8: E713 test for membership should be 'not in' -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 04:17:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3600/ DRY_RUN=true -- 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: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 27 Dec 2018 04:16:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Yongjun Zhang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11591 ) Change subject: IMPALA-6742: Profiles of running queries should include execution summary. .. IMPALA-6742: Profiles of running queries should include execution summary. 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 when queries are still running. Testing: Added unit test. Done with real cluster. Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 --- M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/query_test/test_observability.py 3 files changed, 41 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11591/4 -- 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: newpatchset Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Gerrit-Change-Number: 11591 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Yongjun Zhang
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Joe McDonnell 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 3: I would like there to be a test that breaks if this change is not there (or somehow this breaks later). query_test/test_observability.py is the right place to add this test. Here are a couple ways to do this: 1. Use a sleep in the query. See query_test/test_rows_availability.py: "select * from functional.alltypestiny where month = 1 and bool_col = sleep(1000);" will take two seconds. Read that test and understand why the sleep works. 2. Execute a query that returns rows, and get the profile before fetching any of the rows. If we haven't called fetch, then the client has not called ImpalaServer::UnregisterQuery(). Of these, I think I prefer #2. See test_exec_summary() for an example. Unrelated to your change, I would like you to hand test your change by putting a sleep between these two lines: https://github.com/apache/impala/blob/master/be/src/service/client-request-state.cc#L516-L517 Then fetch the profile and verify it doesn't crash. I have a theory that this might not work. -- 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: 3 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Wed, 14 Nov 2018 20:57:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 3: Verified+1 -- 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: 3 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Sat, 03 Nov 2018 03:40:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3415/ DRY_RUN=true -- 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: 3 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 02 Nov 2018 23:39:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3407/ -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 02 Nov 2018 05:23:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3407/ DRY_RUN=true -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Fri, 02 Nov 2018 01:41:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Tue, 30 Oct 2018 21:51:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1213/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Tue, 30 Oct 2018 18:06:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Impala Public Jenkins 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 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1212/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 1 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Tue, 30 Oct 2018 17:45:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
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: Hi Joe, thanks a lot for the very good review and sorry for late update. I just uploaded a new rev. Interestingly, some tests in test_observability failed locally but all is clean in jenkins. One question about your comment about line 637, I saw other places included [[noreturn]], what's the guideline for having it or not? thanks. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Tue, 30 Oct 2018 17:36:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary.
Hello Tim Armstrong, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11591 to look at the new patch set (#2). Change subject: IMPALA-6742: Profiles of running queries should include execution summary. .. IMPALA-6742: Profiles of running queries should include execution summary. 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 when queries are still running. Testing: Done with real cluster. Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 --- M be/src/service/impala-server.cc M be/src/service/impala-server.h 2 files changed, 19 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11591/2 -- 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: newpatchset Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Gerrit-Change-Number: 11591 Gerrit-PatchSet: 2 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6742: Profiles of running queries should include execution summary
Yongjun Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11591 Change subject: IMPALA-6742: Profiles of running queries should include execution summary .. IMPALA-6742: Profiles of running queries should include execution summary 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 when queries are still running. Testing: This is a draft, tests are yet to be done. Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 --- M be/src/service/impala-server.cc M be/src/service/impala-server.h 2 files changed, 15 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/11591/1 -- 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: newchange Gerrit-Change-Id: Idc7f714c9427d4b26d4e78cf27ceca2b0b336699 Gerrit-Change-Number: 11591 Gerrit-PatchSet: 1 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
