[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Reviewed-on: http://gerrit.cloudera.org:8080/12530 Reviewed-by: Lars Volker Tested-by: Impala Public Jenkins --- M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 6 files changed, 91 insertions(+), 87 deletions(-) Approvals: Lars Volker: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 14 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 13: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 18 Apr 2019 03:11:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2828/ : 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/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 12 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 17 Apr 2019 22:36:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4039/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 17 Apr 2019 22:07:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 13: Code-Review+2 PS 12 fixes Bikram's last comment, PS13 rebases the change. Carrying Bikram's +2. -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 17 Apr 2019 22:07:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#12). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 6 files changed, 91 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/12 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 12 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py@314 PS11, Line 314: "FINISHED_STATE" > TCLIService.TOperationState is an enum and we need to use _VALUES_TO_NAMES I agree it wont be good for readability. Maybe just mention in a comment that it comes from TOperationState so we have an idea of where to look for other states http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py@221 PS11, Line 221: assert any(client.get_state(handle) == 'RUNNING_STATE' or sleep(1) :for _ in range(5)), 'Query failed to start' > ImpalaTestSuite.wait_for_state uses self.client, which is a beeswax client. wfm. didnt notice it was using beeswax. -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 17 Apr 2019 21:41:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py@314 PS11, Line 314: "FINISHED_STATE" > do we have a constant for this? like in TCLIService.TOperationState TCLIService.TOperationState is an enum and we need to use _VALUES_TO_NAMES to map it to a string. This makes this whole check: return cursor.status() == TOperationState._VALUES_TO_NAMES[TOperationState.FINISHED_STATE] Since this is thrift generated, the enum value and its string will always be the same and using the constant+map will not give us any more flexibility in the future, but is much less readable. Let me know if you feel strongly about it or if I'm missing anything, but otherwise I think I prefer the string. http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py@221 PS11, Line 221: assert any(client.get_state(handle) == 'RUNNING_STATE' or sleep(1) :for _ in range(5)), 'Query failed to start' > we can use wait_for_state() here and at Line 228 ImpalaTestSuite.wait_for_state uses self.client, which is a beeswax client. Here we want to use the hs2_client though, and I thought that this was easier than adding another set of methods to the test suite. Let me know if you prefer adding a new method wait_for_state_using_client() there. Once we switch our tests to HS2, we can update the usage here. -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 17 Apr 2019 18:08:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: Code-Review+1 LGTM once you've addressed bikram's comments -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 16 Apr 2019 05:57:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: Code-Review+1 (2 comments) Looks good http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/common/impala_connection.py@314 PS11, Line 314: "FINISHED_STATE" do we have a constant for this? like in TCLIService.TOperationState http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/11/tests/query_test/test_cancellation.py@221 PS11, Line 221: assert any(client.get_state(handle) == 'RUNNING_STATE' or sleep(1) :for _ in range(5)), 'Query failed to start' we can use wait_for_state() here and at Line 228 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 16 Apr 2019 00:32:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2771/ : 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/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 05:41:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2770/ : 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/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 10 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 05:37:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2769/ : 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/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 9 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 05:32:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12530 )
Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc@693
PS8, Line 693: Status status = Status::Expected("Cancelled by client");
> I think we should fix the issue separately is all - it's subtle and I'd wan
That makes sense, I added a loop to wait for the status to reach ERROR_STATE
and filed IMPALA-8411 for the follow up work. I think it would be great if we
could simplify the query state handling to make it easier to reason about.
--
To view, visit http://gerrit.cloudera.org:8080/12530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e
Gerrit-Change-Number: 12530
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 13 Apr 2019 04:50:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/12530/10/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/10/tests/query_test/test_cancellation.py@221 PS10, Line 221: s flake8: F841 local variable 'state' is assigned to but never used -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 10 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 04:57:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/12530/9/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/9/tests/query_test/test_cancellation.py@227 PS9, Line 227: # > flake8: F841 local variable 'state' is assigned to but never used Done -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 04:57:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#11). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 6 files changed, 89 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/11 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 11 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#10). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 6 files changed, 91 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/10 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 10 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/12530/9/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12530/9/tests/query_test/test_cancellation.py@227 PS9, Line 227: s flake8: F841 local variable 'state' is assigned to but never used -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 9 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 13 Apr 2019 04:50:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#9). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 6 files changed, 92 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/9 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 9 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/12530 )
Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc@693
PS8, Line 693: Status status = Status::Expected("Cancelled by client");
> I observed that under load the check in https://gerrit.cloudera.org/#/c/125
I think we should fix the issue separately is all - it's subtle and I'd want to
spend some time studying surrounding code to figure out any possible fallout. I
agree it would be better if it was synchronous and I think the race you
describe is a real issue since it obscures the real reason the query was
cancelled.
The eos_ thing is something we want to fix too, maybe it's possible to fix that
at the same time.
--
To view, visit http://gerrit.cloudera.org:8080/12530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e
Gerrit-Change-Number: 12530
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 13 Apr 2019 04:37:59 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12530 )
Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc@693
PS8, Line 693: Status status = Status::Expected("Cancelled by client");
I observed that under load the check in
https://gerrit.cloudera.org/#/c/12530/8/tests/query_test/test_cancellation.py@229
would sometimes fail. After some poking it appears that the client's
cancellation request does not update the operation state (because "cause", the
third parameter to Cancelnternal, defaults to nullptr). Then the client
disconnects and the connection close triggers another call to CancelInternal,
this time with cause = "Session closed". This is also what shows up in the
profile. If the server is under load, handling the closing connection can take
long enough so that a concurrent request for the state still returns RUNNING.
The aim of this fix was to make CancelOperation synchronous, i.e. when it
returns the subsequent call to GetOperationStatus would not return RUNNING.
The comment on ClientRequestState::Cancel() suggests that this is omitted when
handling client cancellation because we expect that to happen regularly for
finished queries. In that case however, eos_ would be set and we wouldn't
update the state either, no?
> /// Cancels the child queries and the coordinator with the given cause.
/// If cause is NULL, it assume this was deliberately cancelled by the user
while in
/// FINISHED operation state. Otherwise, sets state to ERROR_STATE (TODO:
IMPALA-1262:
/// use CANCELED_STATE). Does nothing if the query has reached EOS or already
cancelled.
--
To view, visit http://gerrit.cloudera.org:8080/12530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e
Gerrit-Change-Number: 12530
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 13 Apr 2019 03:54:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/12530 )
Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
..
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12530/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12530/8//COMMIT_MSG@19
PS8, Line 19: This change also makes a change to update the query operation
state to
See my comment elsewhere. My understanding is that currently the state
transition is asynchronous, but should happen without any other client
intervention. If you have a counterexample to that, that's really interesting.
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/12530/8/be/src/service/impala-hs2-server.cc@693
PS8, Line 693: Status status = Status::Expected("Cancelled by client");
This fix isn't quite right, although the current behaviour is confusing and
imperfect in some ways - client initiated cancellation goes through a different
path in CancelInternal() that sets coordinator_->exec_status_ to
Status::CANCELLED, which then gets bubbled up to the query status.
I'm not sure off the top of my head why it is propagated differently from other
errors, but I think there's some reason. It does seem like we could pass in
Status::CANCELLED here and get the "right" behaviour., but I'm probably missing
something subtle.
Maybe there was some idea that the status shouldn't transition until we've made
some effort to clean up the backends, but the same reasoning would apply to
cancellation upon errors. So we should probably revisit this.
Bikram knows this code fairly well so he might have some insight.
--
To view, visit http://gerrit.cloudera.org:8080/12530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e
Gerrit-Change-Number: 12530
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 13 Apr 2019 02:51:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#8). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 This change also makes a change to update the query operation state to ERROR_STATE upon the client cancellation. Previously we would wait to update the state until the client disconnected, which led to a race where the client could disconnect, then ask for a state and still see STATE_RUNNING while backend was handling the disconnect event. Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M be/src/service/impala-hs2-server.cc M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 7 files changed, 92 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/8 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 8 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2764/ : 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/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 12 Apr 2019 22:06:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12530 ) Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. Patch Set 7: I have bumped impyla and tested PS5 with the exhaustive tests. PS 7 is only the rebase. Please let me know if you'd like me to run exhaustive test on PS7. -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 12 Apr 2019 21:52:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12530 to look at the new patch set (#6). Change subject: IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 .. IMPALA-8158: Retrieve thrift profiles through Impyla 0.15.0 This change updates Impyla to 0.15.0 and then uses Impyla to retrieve thrift profiles through the HS2 api. Unfortunately, some of the current usages of get_thrift_profile rely on the Beeswax query states and the ImpylaHS2Connection does not have the required functionality yet. We will have to update these in a future change, once we unified the query states. This change also adds a self-contained test for IMPALA-2063 Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e --- M be/src/service/impala-hs2-server.cc M infra/python/deps/compiled-requirements.txt M tests/common/impala_connection.py M tests/common/impala_service.py M tests/common/impala_test_suite.py M tests/query_test/test_cancellation.py M tests/query_test/test_observability.py 7 files changed, 92 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/12530/6 -- To view, visit http://gerrit.cloudera.org:8080/12530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I769a99f0843297dd2b20f2f5b1a9046c97bb131e Gerrit-Change-Number: 12530 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
