Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21930 )

Change subject: IMPALA-12648: Add KILL QUERY statement
......................................................................


Patch Set 27:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/21930/20/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/21930/20/be/src/service/client-request-state.cc@2494
PS20, Line 2494:               NetworkAddressPBToString(krpc_addr), 
get_proxy_status.msg().msg()));
               :     }
               :     KillQueryResponsePB response;
> It seems that the two query options only affects how the client interacts w
If KILL query is not implemented like regular query, then it might not subject 
to query_timeout_s or fetch_rows_timeout_ms.
That leave question, what if KILL query hang, and user cancel the KILL query in 
impala-shell?


http://gerrit.cloudera.org:8080/#/c/21930/20/be/src/service/client-request-state.cc@2500
PS20, Line 2500: Status rpc_status = RpcMgr::
> Can you also report progress too like this?
Actually, never mind about reporting progress. Lets consider that for later.

But please consider to not fail immediately if there is 1 unreachable 
Coordinator. That also means moving on to next Coordinator if get_proxy_status 
is not OK.


http://gerrit.cloudera.org:8080/#/c/21930/24/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/21930/24/be/src/service/client-request-state.cc@2513
PS24, Line 2513:     } else if (status.code() != 
TErrorCode::INVALID_QUERY_HANDLE) {
               :       LOG(ERROR) << "KillQuery: Found the coordinator at
> Thanks! Changed.
Done


http://gerrit.cloudera.org:8080/#/c/21930/24/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/21930/24/be/src/service/impala-server.cc@1795
PS24, Line 1795:              << ", method=" << (from_kill_query_statement ? 
"kill_query" : "cancel_rpc");
> I meant passing an argument to Cancel() that just changes the logging.
Agree with Michael. ImpalaServer::CancelInternal() can remain unchanged.


http://gerrit.cloudera.org:8080/#/c/21930/24/tests/custom_cluster/test_kill_query_custom_cluster.py
File tests/custom_cluster/test_kill_query_custom_cluster.py:

http://gerrit.cloudera.org:8080/#/c/21930/24/tests/custom_cluster/test_kill_query_custom_cluster.py@32
PS24, Line 32:
> Thanks! Changed the name.
Done


http://gerrit.cloudera.org:8080/#/c/21930/17/tests/util/cancel_util.py
File tests/util/cancel_util.py:

http://gerrit.cloudera.org:8080/#/c/21930/17/tests/util/cancel_util.py@23
PS17, Line 23: from tests.common.test_result_verifier import error_msg_expected
> It seems that Python 2 does not support this because of the “unbound method
Thanks for testing it. Lets keep the cluster_config.py as it is then.



--
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: 27
Gerrit-Owner: Xuebin Su <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Xuebin Su <[email protected]>
Gerrit-Comment-Date: Tue, 07 Jan 2025 01:29:08 +0000
Gerrit-HasComments: Yes

Reply via email to