[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 20:55:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified so the 'profile' command has a new set of options. The syntax is now: PROFILE [ALL | LATEST | ORIGINAL] If 'ALL' is specified, both the latest and original profiles are printed. If 'LATEST' is specified, only the latest profile is printed. If 'ORIGINAL' is printed, only the original profile is printed. The default behavior is equivalent to specifying 'LATEST' (which is the current behavior before this patch as well). Support for this has only been added to HS2 given that Beeswax is being deprecated soon. The new 'profile' options have no affect when the Beeswax protocol is used. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Reviewed-on: http://gerrit.cloudera.org:8080/16406 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 505 insertions(+), 125 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 10 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Hit IMPALA-9923 again -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 15:34:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 15:34:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6439/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 15:34:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6434/ -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 03:24:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py@87 PS6, Line 87: proc.sendline(profile_cmd) > Done. pexpect.expect takes a regex, so I just used regexes instead. Cool! -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 17 Sep 2020 00:36:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6434/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 23:21:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Hit IMPALA-9923 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 23:21:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6432/ -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 18:24:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7187/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 14:45:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 14:26:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6432/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 14:26:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 7: Code-Review+2 Carrying +2. -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 14:26:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py
File tests/custom_cluster/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py@87
PS6, Line 87: proc.expect("Query State: FINISHED")
> Can we verify the absence of the original profile? I.e. something like this
Done. pexpect.expect takes a regex, so I just used regexes instead.
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 16 Sep 2020 14:24:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#7). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified so the 'profile' command has a new set of options. The syntax is now: PROFILE [ALL | LATEST | ORIGINAL] If 'ALL' is specified, both the latest and original profiles are printed. If 'LATEST' is specified, only the latest profile is printed. If 'ORIGINAL' is printed, only the original profile is printed. The default behavior is equivalent to specifying 'LATEST' (which is the current behavior before this patch as well). Support for this has only been added to HS2 given that Beeswax is being deprecated soon. The new 'profile' options have no affect when the Beeswax protocol is used. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 505 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/7 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 6: Code-Review+2
(1 comment)
LGTM. Just have a minor comment on the test. Feel free to carry on my +2 when
it's resolved.
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py
File tests/custom_cluster/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py@87
PS6, Line 87: proc.expect("Query State: FINISHED")
Can we verify the absence of the original profile? I.e. something like this:
import pexpect
try:
proc.expect("Failed Query Runtime Profile", timeout=3)
assert False
except pexpect.TIMEOUT:
assert True
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 16 Sep 2020 03:20:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7183/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 02:09:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: reviewers* -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 01:50:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: > Do you plan to let anyone else to take a look? If not, I can bump to +2 after > tests are added. No plans to add other reviews. -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 01:49:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1042
PS5, Line 1042: failed_thrift_profiles.emplace_back(*thrift_profi
> nit: this is already done in __set_failed_thrift_profiles()
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1052
PS5, Line 1052: original_profiles.emplace_back(ss->str());
> nit: this is already done in __set_failed_profiles()
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1056
PS5, Line 1056:
> nit: only the 'format' of the request is used. We can refactor it to a TRun
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1121
PS5, Line 1121: GetRuntimeProfileOu
> nit: 4 spaces indention
Not sure why, but ClangFormat makes it like this.
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1132
PS5, Line 1132: if (was_retried && request.include_query_attempts) {
: if (request.format == TRuntimeProfileFormat::THRIFT) {
: SetFailedProfile(_profile, return_val);
: } else if (request.format == TRuntimeProfileFormat::JSON) {
: JSONProfileToStringProfile(, _profile);
: SetFailedProfile(, return_val);
: } else {
: DCHECK(request.format == TRuntimeProfileFormat::STRING
: || request.format == TRuntimeProfileFormat::BASE64);
:
> nit: the 3 if branches have the same conditions as SetProfile() at line 105
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@110
PS5, Line 110: ons
> typo then?
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@111
PS5, Line 111: thi
> typo then?
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1004
PS5, Line 1004: db_
> nit: 2 spaces indention
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1071
PS5, Line 1071: prettytable =
self.construct_table_with_header(column_names)
: formatter = PrettyOutputFormatter(pret
> nit: may be helpful to print the three valid values as well.
Done
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 16 Sep 2020 01:49:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#6). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified so the 'profile' command has a new set of options. The syntax is now: PROFILE [ALL | LATEST | ORIGINAL] If 'ALL' is specified, both the latest and original profiles are printed. If 'LATEST' is specified, only the latest profile is printed. If 'ORIGINAL' is printed, only the original profile is printed. The default behavior is equivalent to specifying 'LATEST' (which is the current behavior before this patch as well). Support for this has only been added to HS2 given that Beeswax is being deprecated soon. The new 'profile' options have no affect when the Beeswax protocol is used. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 498 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/6 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 5: Code-Review+1
(10 comments)
> Patch Set 3:
>
> (12 comments)
>
> I updated the patch to include your suggestions from
> https://issues.apache.org/jira/browse/IMPALA-9870?focusedCommentId=17188243=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17188243
> so the format of the profile command is:
>
> PROFILE [ALL | LATEST | ORIGINAL]
>
> The core functionality is done, I haven't added tests yet. Just looking for
> any feedback first to make sure the implementation is correct, then I will
> add tests.
Thanks for implementing the extended PROFILE command! The patch looks good to
me. Just have some minor comments.
Do you plan to let anyone else to take a look? If not, I can bump to +2 after
tests are added.
http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG@27
PS2, Line 27: Beeswax is being
: deprecated soon.
> I know there is a review out for changing the default: https://gerrit.cloud
Thanks for the info!
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1042
PS5, Line 1042: return_val.__isset.failed_thrift_profiles = true;
nit: this is already done in __set_failed_thrift_profiles()
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1052
PS5, Line 1052: return_val.__isset.failed_profiles = true;
nit: this is already done in __set_failed_profiles()
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1056
PS5, Line 1056: const TGetRuntimeProfileReq& get_profile_req
nit: only the 'format' of the request is used. We can refactor it to a
TRuntimeProfileFormat::type parameter.
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1121
PS5, Line 1121:
nit: 4 spaces indention
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1132
PS5, Line 1132: if (request.format == TRuntimeProfileFormat::THRIFT) {
: SetFailedProfile(_profile, return_val);
: } else if (request.format == TRuntimeProfileFormat::JSON) {
: JsonProfileToStringProfile(json_profile, );
: SetFailedProfile(, return_val);
: } else {
: DCHECK(request.format == TRuntimeProfileFormat::STRING
: || request.format == TRuntimeProfileFormat::BASE64);
: SetFailedProfile(, return_val);
: }
nit: the 3 if branches have the same conditions as SetProfile() at line 1055.
Maybe it's better to refactor them as one method (e.g. by adding a
"is_failed_attempt" parameter.
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@110
PS5, Line 110: the
typo then?
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@111
PS5, Line 111: the
typo then?
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1004
PS5, Line 1004:
nit: 2 spaces indention
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1071
PS5, Line 1071: print("Invalid value for query profile display mode:
\'" +
: profile_display_mode + "\'")
nit: may be helpful to print the three valid values as well.
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Tue, 15 Sep 2020 01:55:39 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7147/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 10 Sep 2020 03:08:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7146/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 10 Sep 2020 03:04:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 3: (12 comments) I updated the patch to include your suggestions from https://issues.apache.org/jira/browse/IMPALA-9870?focusedCommentId=17188243=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17188243 so the format of the profile command is: PROFILE [ALL | LATEST | ORIGINAL] The core functionality is done, I haven't added tests yet. Just looking for any feedback first to make sure the implementation is correct, then I will add tests. http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG@23 PS2, Line 23: impala-shell has been modified to dump both the most recent query : attempt and the original query attempt. > I think dumping all profiles may break some tools that grep results from th I updated the patch to include your suggestion to modify the profile command. http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG@27 PS2, Line 27: Beeswax is being : deprecated soon. > BTW, is there a timeline or any blockers for this? I know there is a review out for changing the default: https://gerrit.cloudera.org/#/c/16327/ http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@773 PS2, Line 773: std:: > nit: can use string directly Done http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@784 PS2, Line 784: JsonProfileToStringProfile > nit: JsonProfileToStringProfile Done http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@785 PS2, Line 785: std::stringstream* string_profile); > nit: I think our code style tries to put the input vars before the output v Done http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@787 PS2, Line 787: /// Set the profile (or thrift_profile) field for the given TGetRuntimeProfileResp > Could you add a method comment for this? Also move the 'request' to be the Done http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py@997 PS3, Line 997: > flake8: E251 unexpected spaces around keyword / parameter equals Done http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py@997 PS3, Line 997: > flake8: E251 unexpected spaces around keyword / parameter equals Done http://gerrit.cloudera.org:8080/#/c/16406/2/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/16406/2/tests/custom_cluster/test_shell_interactive.py@71 PS2, Line 71: > Also test about the -p or --show_profiles option? Done http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@61 PS3, Line 61: " > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@77 PS3, Line 77: > flake8: W293 blank line contains whitespace Done http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@77 PS3, Line 77: > line has trailing whitespace Done -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 10 Sep 2020 02:48:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#4). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified to dump both the most recent query attempt and the original query attempt. It sets 'include_query_attempts' to true in the TGetRuntimeProfileReq request. Support for this has only been added to HS2 given that Beeswax is being deprecated soon. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests TODO: * Add tests for 'profile' command options * Revise commit message to mention changes to 'profile' command Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 473 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/4 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py@997 PS3, Line 997: flake8: E251 unexpected spaces around keyword / parameter equals http://gerrit.cloudera.org:8080/#/c/16406/3/shell/impala_shell.py@997 PS3, Line 997: flake8: E251 unexpected spaces around keyword / parameter equals http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@61 PS3, Line 61: " flake8: E501 line too long (91 > 90 characters) http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@77 PS3, Line 77: flake8: W293 blank line contains whitespace http://gerrit.cloudera.org:8080/#/c/16406/3/tests/custom_cluster/test_shell_interactive.py@77 PS3, Line 77: line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 10 Sep 2020 02:44:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#3). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified to dump both the most recent query attempt and the original query attempt. It sets 'include_query_attempts' to true in the TGetRuntimeProfileReq request. Support for this has only been added to HS2 given that Beeswax is being deprecated soon. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests TODO: * Add tests for 'profile' command options * Revise commit message to mention changes to 'profile' command Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 471 insertions(+), 124 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/3 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG@23 PS2, Line 23: impala-shell has been modified to dump both the most recent query : attempt and the original query attempt. I think dumping all profiles may break some tools that grep results from the output. However, we can extend the profile command in later patches to support different requirements. http://gerrit.cloudera.org:8080/#/c/16406/2//COMMIT_MSG@27 PS2, Line 27: Beeswax is being : deprecated soon. BTW, is there a timeline or any blockers for this? http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@773 PS2, Line 773: std:: nit: can use string directly http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@784 PS2, Line 784: JSONProfileToStringProfile nit: JsonProfileToStringProfile http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@785 PS2, Line 785: std::stringstream* string_profile, rapidjson::Document* json_profile nit: I think our code style tries to put the input vars before the output vars, so we should put 'json_profile' the first arg here. Also use const reference for inputs. So it's void JsonProfileToStringProfile(const rapidjson::Document& json_profile, std::stringstream* string_profile) https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs http://gerrit.cloudera.org:8080/#/c/16406/2/be/src/service/impala-server.h@787 PS2, Line 787: void SetProfile(RuntimeProfileOutput* profile, TGetRuntimeProfileResp& return_val, Could you add a method comment for this? Also move the 'request' to be the first arg? http://gerrit.cloudera.org:8080/#/c/16406/2/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/16406/2/tests/custom_cluster/test_shell_interactive.py@71 PS2, Line 71: vector, ['-q', query_options + query + "; profile", '-B'])) Also test about the -p or --show_profiles option? -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 04 Sep 2020 14:25:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7089/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 19:08:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/16406/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/16406/1/be/src/service/impala-server.cc@701 PS1, Line 701: Status status = > line too long (94 > 90) Done http://gerrit.cloudera.org:8080/#/c/16406/1/tests/shell/util.py File tests/shell/util.py: http://gerrit.cloudera.org:8080/#/c/16406/1/tests/shell/util.py@327 PS1, Line 327: > flake8: E302 expected 2 blank lines, found 1 Done -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 03 Sep 2020 18:48:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#2). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified to dump both the most recent query attempt and the original query attempt. It sets 'include_query_attempts' to true in the TGetRuntimeProfileReq request. Support for this has only been added to HS2 given that Beeswax is being deprecated soon. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 411 insertions(+), 121 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/2 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
