[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3166/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 11 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 10 May 2019 01:48:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 11: (5 comments) http://gerrit.cloudera.org:8080/#/c/12926/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12926/11//COMMIT_MSG@7 PS11, Line 7: IMPALA-7031: Add additional info to query canceled from http endpoint : : When a running query is canceled from the http endpoint, the query will : be unregistered. This can result in a scenario where a client that has : a reference to the query handle is unaware that the query has been : unregistered via http endpoint, and attempts to run an operation on : that query handle but encounters "Invalid query handle" error. : From the user prospective the error is not indicative, therefore the : patch added additional information to the error to notify that the query : is already unregistered. : : Testing: : - Added a test to test_web_pages.py : - Ran all webserver tests update commit message to reflect change of approach from initially changing the http endpoint behavior to now adding info to the error message if the query was recently cancelled. http://gerrit.cloudera.org:8080/#/c/12926/11/be/src/service/impala-beeswax-server.cc File be/src/service/impala-beeswax-server.cc: http://gerrit.cloudera.org:8080/#/c/12926/11/be/src/service/impala-beeswax-server.cc@257 PS11, Line 257: nit: remove newline http://gerrit.cloudera.org:8080/#/c/12926/11/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12926/11/be/src/service/impala-server.cc@2586 PS11, Line 2586: Query is unregistered, if it can be found in query log nit: Query was recently unregistered if it can be found in the query log http://gerrit.cloudera.org:8080/#/c/12926/11/be/src/service/impala-server.cc@2586 PS11, Line 2586: // Query is unregistered, if it can be found in query log you need to hold query_log_lock_ before accessing the query_log_index_ http://gerrit.cloudera.org:8080/#/c/12926/11/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/11/tests/webserver/test_web_pages.py@547 PS11, Line 547: @pytest.mark.execute_serially : def test_cancel_query_by_http_endpoint(self): : """Verify that an indicative error message is returned when trying to use a query : handle for a recently cancelled query. That kind of message is returned if the query : is still in the query archive log, so to make sure it is not pushed out of the log by : other queries we run this test serially.""" : try: : # Runs a query and then cancels it by http endpoint : query = "select count(*) from functional.alltypes where bool_col = sleep(1000)" : query_handle = self.client.execute_async(query) : responses = self.get_and_check_status(self.ROOT_URL : + "/cancel_query?query_id=" : + query_handle.get_handle().id, : ports_to_test=[25000]) : assert "Query cancellation successful" in responses[0].text,\ : "Query is not canceled:\n" + responses[0].text : # Client requests state for the unregistered query : self.client.get_state(query_handle) : except ImpalaBeeswaxException as e: : assert "Invalid query handle. The query is already unregistered" in str(e),\ : "Encountered unexpected exception:\n"\ : + str(e) move this to test_cancellation and cancel the query regularly using the client -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 11 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 10 May 2019 00:50:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py@336 PS10, Line 336: assert err_msg in get_operation_status_resp.status.errorMessage > wont this fail if the error message is "Invalid query handle. The query is Thanks for catching this. This will not be put into archive log http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@549 PS10, Line 549: """Verify that an indicative error message is returned when trying to use a query > nit: Verify that Done http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@551 PS10, Line 551: is still in the query archive log, so to make sure it is not pushed out of the log by > nit: out of the log Thanks. -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 11 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 10 May 2019 00:25:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#11). Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. >From the user prospective the error is not indicative, therefore the patch added additional information to the error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/webserver/test_web_pages.py 5 files changed, 55 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/11 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 11 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3157/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 10 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 09 May 2019 22:44:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py@336 PS10, Line 336: assert err_msg in get_operation_status_resp.status.errorMessage wont this fail if the error message is "Invalid query handle. The query is already unregistered: efcdab8967452301:3031323334353637" http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@549 PS10, Line 549: """Verify an indicative error message is returned when trying to use a query handle nit: Verify that http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@551 PS10, Line 551: in the query archive log, so to make sure it is not pushed out by other queries we run nit: out of the log -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 10 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 09 May 2019 19:19:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 10: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3144/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 10 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 09 May 2019 03:29:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Alice Fan has posted comments on this change. (
http://gerrit.cloudera.org:8080/12926 )
Change subject: IMPALA-7031: Add additional info to query canceled from http
endpoint
..
Patch Set 9:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc@201
PS9, Line 201: ImpalaServer::
> nit: you can just call the method without this scope resolution.
Done
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@605
PS9, Line 605: // When a query is unregistered, its QueryStateRecord will be
added into query log
: // during the archive stage. This method is used to identify
whether the query is
: // unregistered and return a proper error message. The method
is used for unknown
: // query, which is a query lost its ClientRequestState.
> Returns the appropriate error message for an invalid query id. Also checks
Done
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@609
PS9, Line 609: getErrMsgForUnknownQuery
> nit: how about getErrMsgForInvalidQueryId
sure.
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc@2589
PS9, Line 2589: Query is already unregistered
> I think we should still say invalid query handle in this case too, because
Done
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py@335
PS9, Line 335:
: assert "Query is already unregistered" or "Invalid query
handle" \
: in get_operation_status_resp.status.errorMessage
> ("a" or "b" in str) is the wrong conditional, please see the following link
Thanks!
Removed changes here. As error msg now will include 'Invalid query handle' in
anyway.
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py@194
PS9, Line 194: ('Invalid query handle' in str(thread.fetch_results_error) or
: 'Query is already unregistered' in
str(thread.fetch_results_error)
: and not join_before_close)
> this becomes A || B & C, which is interpreted as A || (B & C)
Woops. Thanks for point this out!
I removed the "Query is already unregistered" from here, as it is an extra info
for "Invalid query handle"
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@546
PS9, Line 546:
> add @pytest.mark.execute_serially decorator to this test. You should make s
Thanks!
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@548
PS9, Line 548: """When a query is successfully canceled, client should be
notified that the
: query is already unregistered."""
> nit:how about: Make sure an indicative error message is returned when tryin
Done
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@562
PS9, Line 562: # Because the query is unregistered, client should receive
exception with
: # "Query is already unregistered" or "Invalid query
handle" notice.
: # For the query gets "Invalid query handle" is because
query log might be
: # full and then removed the oldest unregistered query
> nit: you can remove this comment after you make the change to run this seri
Done
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan
Gerrit-Reviewer: Alice Fan
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 09 May 2019 03:16:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#10). Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. >From the user prospective the error is not indicative, therefore the patch added additional information to the error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/webserver/test_web_pages.py 5 files changed, 55 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/10 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 10 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/12926 )
Change subject: IMPALA-7031: Add additional info to query canceled from http
endpoint
..
Patch Set 9:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc@201
PS9, Line 201: ImpalaServer::
nit: you can just call the method without this scope resolution.
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@605
PS9, Line 605: // When a query is unregistered, its QueryStateRecord will be
added into query log
: // during the archive stage. This method is used to identify
whether the query is
: // unregistered and return a proper error message. The method
is used for unknown
: // query, which is a query lost its ClientRequestState.
Returns the appropriate error message for an invalid query id. Also checks
whether the query was recently unregistered by checking for it in the query log
and adds the appropriate info to the error message.
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@609
PS9, Line 609: getErrMsgForUnknownQuery
nit: how about getErrMsgForInvalidQueryId
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc@2589
PS9, Line 2589: Query is already unregistered
I think we should still say invalid query handle in this case too, because it
actually is invalid, the query being unregistered is only extra info
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py@335
PS9, Line 335:
: assert "Query is already unregistered" or "Invalid query
handle" \
: in get_operation_status_resp.status.errorMessage
("a" or "b" in str) is the wrong conditional, please see the following link as
to how it will be interpreted:
https://stackoverflow.com/questions/21344842/if-a-or-b-in-l-where-l-is-a-list-python
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py@194
PS9, Line 194: ('Invalid query handle' in str(thread.fetch_results_error) or
: 'Query is already unregistered' in
str(thread.fetch_results_error)
: and not join_before_close)
this becomes A || B & C, which is interpreted as A || (B & C)
whereas what we want here is (A || B) & C.
Can you add the appropriate brackets to fix this
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@546
PS9, Line 546:
add @pytest.mark.execute_serially decorator to this test. You should make sure
that "Query is already unregistered" is always returned to know that your patch
is working, but in order to flush out any flakiness due to other queries
running and pushing out the query state from the query log we can run it
serially
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@548
PS9, Line 548: """When a query is successfully canceled, client should be
notified that the
: query is already unregistered."""
nit:how about: Make sure an indicative error message is returned when trying to
use a query handle for a recently cancelled query. That kind of message is
returned if the query is still in the query archive log, so to make sure it is
not pushed out by other queries we run this test serially.
http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@562
PS9, Line 562: # Because the query is unregistered, client should receive
exception with
: # "Query is already unregistered" or "Invalid query
handle" notice.
: # For the query gets "Invalid query handle" is because
query log might be
: # full and then removed the oldest unregistered query
nit: you can remove this comment after you make the change to run this serially
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan
Gerrit-Reviewer:
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3075/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 9 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 04 May 2019 02:12:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3074/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 8 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 04 May 2019 01:37:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#9). Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. From the client/user prospective the error is not indicative, so the patch added additional information to error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests and all tests have been changed Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/hs2/test_hs2.py M tests/query_test/test_cancellation.py M tests/shell/test_shell_commandline.py M tests/stress/concurrent_select.py M tests/webserver/test_web_pages.py 9 files changed, 75 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/9 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 9 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 7 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 04 May 2019 01:22:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/12926/8/tests/query_test/test_cancellation.py File tests/query_test/test_cancellation.py: http://gerrit.cloudera.org:8080/#/c/12926/8/tests/query_test/test_cancellation.py@194 PS8, Line 194: \ flake8: E502 the backslash is redundant between brackets http://gerrit.cloudera.org:8080/#/c/12926/8/tests/query_test/test_cancellation.py@195 PS8, Line 195: \ flake8: E502 the backslash is redundant between brackets -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 8 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 04 May 2019 01:12:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#8). Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. From the client/user prospective the error is not indicative, so the patch added additional information to error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests and all tests have been changed Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/hs2/test_hs2.py M tests/query_test/test_cancellation.py M tests/shell/test_shell_commandline.py M tests/stress/concurrent_select.py M tests/webserver/test_web_pages.py 9 files changed, 75 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/8 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 8 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4151/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 7 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 03 May 2019 20:28:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3054/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 7 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 03 May 2019 06:23:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 7: (12 comments) http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@9 PS4, Line 9: When a running query is canceled from the http endpoint, the query will > nit: queries debug page Done http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@10 PS4, Line 10: a client that has : a reference to the query handle is unaware that the query has been : unregistered via > This can result in a scenario where a client that has a reference to the qu Updated. Thanks. It is much clear. http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@12 PS4, Line 12: tp endpoint, and attempts to run an operation on : that query handle but encounters "Invalid query handle" error. : From the clie > This patch updates the cancel http endpoint to only cancel the query. Done http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@18 PS4, Line 18: sti > nit: ran http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc@179 PS6, Line 179: VLOG(1) << err_msg; : return Status::Expected(err_msg); : } : : // > this looks like common code that you can refactor out and use at all other Done http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py@543 PS1, Line 543: > flake8: E501 line too long (93 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@537 PS4, Line 537: he 'krpc_address' is the krpc address of the impalad. : assert len(backend_row['krpc_a > Verify that the cancel http endpoint does not unregister the query Done http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@540 PS4, Line 540: ddress > cancels Done http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@547 PS4, Line 547: def test_cancel_query_by_http_endpoint(self): > assert "Query cancellation successful" in responses[0].text, responses[0].t Thanks. Added responses[0].text as error message http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@548 PS4, Line 548: """When a query is successfully canceled, client should be notified that the > to verify that the query id is still registered Done http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@551 PS4, Line 551: cancels it b > nit not necessarily the case, could be anything. Just say something like "E Done http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@552 PS4, Line 552: query = "select count(*) from functional.alltypes where bool_col = sleep(1000)" > finally: Done -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 7 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 03 May 2019 05:30:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12926 to look at the new patch set (#7). Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. From the client/user prospective the error is not indicative, so the patch added additional information to error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/webserver/test_web_pages.py 5 files changed, 57 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/7 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 7 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/12926 )
Change subject: IMPALA-7031: Add additional info to query canceled from http
endpoint
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc@179
PS6, Line 179: if (ImpalaServer::isUnregistered(query_id)) {
: err_msg = Substitute("Query is already unregistered: $0",
PrintId(query_id));
: } else {
: err_msg = Substitute("Invalid query handle: $0",
PrintId(query_id));
: }
this looks like common code that you can refactor out and use at all other
places
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 02 May 2019 20:07:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3026/ : 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/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 6 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 01 May 2019 19:23:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/12926/6/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12926/6/tests/webserver/test_web_pages.py@557 PS6, Line 557: " flake8: E122 continuation line missing indentation or outdented http://gerrit.cloudera.org:8080/#/c/12926/6/tests/webserver/test_web_pages.py@558 PS6, Line 558: + flake8: E122 continuation line missing indentation or outdented -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 6 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 01 May 2019 18:39:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint
Alice Fan has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12926 ) Change subject: IMPALA-7031: Add additional info to query canceled from http endpoint .. IMPALA-7031: Add additional info to query canceled from http endpoint When a running query is canceled from the http endpoint, the query will be unregistered. This can result in a scenario where a client that has a reference to the query handle is unaware that the query has been unregistered via http endpoint, and attempts to run an operation on that query handle but encounters "Invalid query handle" error. >From the client/user prospective the error is not indicative, so the patch added additional information to error to notify that the query is already unregistered. Testing: - Added a test to test_web_pages.py - Ran all webserver tests Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 --- M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/webserver/test_web_pages.py 5 files changed, 100 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/6 -- To view, visit http://gerrit.cloudera.org:8080/12926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8 Gerrit-Change-Number: 12926 Gerrit-PatchSet: 6 Gerrit-Owner: Alice Fan Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins
