Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21930 )
Change subject: IMPALA-12648: Add KILL QUERY statement ...................................................................... Patch Set 45: (7 comments) Someone else with sql-parser.cup expertise should chime in. In the mean time, I have some test comments. http://gerrit.cloudera.org:8080/#/c/21930/34//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21930/34//COMMIT_MSG@19 PS34, Line 19: dded as "unreserved keywords" > Thanks! Removed the new flag in the new Patch Set. In the new Patch Set we Done http://gerrit.cloudera.org:8080/#/c/21930/34/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/21930/34/be/src/service/client-request-state.cc@2507 PS34, Line 2507: Status rpc_status = RpcMgr::DoRpcWithRetry(proxy, &ControlServiceProxy::KillQuery, : request, &response, query_ctx_, "KillQuery() RPC failed", num_retries, timeout_ms, : backoff_time_ms, "CRS_KILL_QUERY_RPC"); > Thanks! Added. Done http://gerrit.cloudera.org:8080/#/c/21930/34/fe/src/main/jflex/sql-scanner.flex File fe/src/main/jflex/sql-scanner.flex: http://gerrit.cloudera.org:8080/#/c/21930/34/fe/src/main/jflex/sql-scanner.flex@371 PS34, Line 371: reservedWords.addAll(Arrays.asList(new String[] { : "abs", "acos", "allocate", "any", "are", "array_agg", "array > KW_KILL and KW_QUERY should be added like this: Disregard. http://gerrit.cloudera.org:8080/#/c/21930/45/tests/custom_cluster/test_kill_query.py File tests/custom_cluster/test_kill_query.py: http://gerrit.cloudera.org:8080/#/c/21930/45/tests/custom_cluster/test_kill_query.py@33 PS45, Line 33: @pytest.mark.execute_serially : def test_coordinator_unreachable(self): : """ : The coordinator of the query to kill is unreachable. : : It is required that each impalad in the cluster is a coordinator. : """ : protocol = 'hs2' : with self.create_client_for_nth_impalad(0, protocol) as client, \ : QueryToKill( : self, : protocol, : check_on_exit=False, : nth_impalad=2) as query_id_to_kill: : coordinator_to_kill = self.cluster.impalads[2] : coordinator_to_kill.kill() : assert_kill_error( : client, : "KillQuery() RPC failed: Network error:", : query_id=query_id_to_kill, : ) : : @pytest.mark.execute_serially : def test_another_coordinator_unreachable(self): : """ : A coordinator other than the one of the query to kill is unreachable. : : It is required that each impalad in the cluster is a coordinator. : """ : protocol = 'hs2' : with self.create_client_for_nth_impalad(0, protocol) as client, \ : QueryToKill(self, protocol, nth_impalad=2) as query_id_to_kill: : coordinator_to_kill = self.cluster.impalads[1] # impalad 1 is between 0 and 2. : coordinator_to_kill.kill() : assert_kill_ok(client, query_id_to_kill) This tests does not have specific cluster flag to exercise. Can they turned into regular end-to-end test under tests/query_test/? http://gerrit.cloudera.org:8080/#/c/21930/45/tests/custom_cluster/test_kill_query.py@70 PS45, Line 70: @cluster_config.single_coordinator : def test_single_coordinator(self): Why is it important to tests against cluster with only 1 Impalad? Can this turned into regular end-to-end tests instead? http://gerrit.cloudera.org:8080/#/c/21930/45/tests/custom_cluster/test_kill_query.py@97 PS45, Line 97: Make sure KILL QUERY statement can be executed when no query will be admitted. I think this test can merge with test_admit_one_query_at_a_time. Scenario is like this: 1. Run a long running query A. Verify that it is running. 2. Run a long running query B. Verify that it is failed. 2. Kill query B and assert error. 3. Kill query A and assert that kill is success. You can use this query to test: https://gerrit.cloudera.org/c/22351/3/tests/custom_cluster/test_admission_controller.py#68 http://gerrit.cloudera.org:8080/#/c/21930/45/tests/util/cancel_util.py File tests/util/cancel_util.py: http://gerrit.cloudera.org:8080/#/c/21930/45/tests/util/cancel_util.py@127 PS45, Line 127: client Can you use different client to run the KILL query? Also validate that test query handle and kill query handle is closed at the end of test. -- To view, visit http://gerrit.cloudera.org:8080/21930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If12d6e47b256b034ec444f17c7890aa3b40481c0 Gerrit-Change-Number: 21930 Gerrit-PatchSet: 45 Gerrit-Owner: Xuebin Su <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Xuebin Su <[email protected]> Gerrit-Comment-Date: Thu, 16 Jan 2025 20:14:54 +0000 Gerrit-HasComments: Yes
