Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12926 )
Change subject: IMPALA-7031: Cancel action of debug page should not unregister query ...................................................................... Patch Set 4: (10 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 cancelled from the Cancel button of debug nit: queries debug page http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@10 PS4, Line 10: But the client : is unaware of the query is unregistered, it will get error/exception : on other attempts 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 the debug page, and attempts to run an operation on that query handle but encounters and error instead. http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@12 PS4, Line 12: The patch updated the ImpalaHttpHandler to make : "Cancel" button will only cancel the query instead of : unregister it This patch updates the cancel http endpoint to only cancel the query. http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@18 PS4, Line 18: Run nit: ran 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: Test if unregister query when press "Cancel" button on : the debug page (impalad WebUI) Verify that the cancel http endpoint does not unregister the query http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@540 PS4, Line 540: cancel cancels http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@547 PS4, Line 547: assert "Query cancellation successful" in responses[0].text assert "Query cancellation successful" in responses[0].text, responses[0].text http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@548 PS4, Line 548: # After the query is successfully canceled, the client runs a get_state operation to verify that the query id is still registered http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@551 PS4, Line 551: unregistered nit not necessarily the case, could be anything. Just say something like "Encountered unexpected exception. Query might be unregisted already. Exception: " http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@552 PS4, Line 552: finally: self.client.close(query_handle) -- 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: 4 Gerrit-Owner: 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: Tue, 09 Apr 2019 17:35:07 +0000 Gerrit-HasComments: Yes
