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 <fan...@gmail.com> Gerrit-Reviewer: Alice Fan <fan...@gmail.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Fri, 03 May 2019 05:30:49 +0000 Gerrit-HasComments: Yes
