Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21803 )

Change subject: IMPALA-915: Support cancel queries in frontend
......................................................................


Patch Set 43:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/21803/41/be/src/service/client-request-state.cc@256
PS41, Line 256:   profile_->AddChild(server_profile_);
> Those both seem reasonable. Moving MarkActive later might delay execution i
Done


http://gerrit.cloudera.org:8080/#/c/21803/41/fe/src/main/java/org/apache/impala/service/Canceller.java
File fe/src/main/java/org/apache/impala/service/Canceller.java:

http://gerrit.cloudera.org:8080/#/c/21803/41/fe/src/main/java/org/apache/impala/service/Canceller.java@41
PS41, Line 41: public final class Canceller {
> This class should enforce its design pattern by marking the class final and
Done


http://gerrit.cloudera.org:8080/#/c/21803/41/fe/src/main/java/org/apache/impala/service/Canceller.java@94
PS41, Line 94:    * Cancels thread associated with queryId.
> I think:
Done


http://gerrit.cloudera.org:8080/#/c/21803/41/fe/src/main/java/org/apache/impala/service/Canceller.java@132
PS41, Line 132:
> Probably not, I'll see about moving it to the following patch.
Done


http://gerrit.cloudera.org:8080/#/c/21803/41/fe/src/main/java/org/apache/impala/service/Canceller.java@146
PS41, Line 146:
> Probably not, I'll see about moving it to the following patch.
Done


http://gerrit.cloudera.org:8080/#/c/21803/41/tests/util/web_pages_util.py
File tests/util/web_pages_util.py:

http://gerrit.cloudera.org:8080/#/c/21803/41/tests/util/web_pages_util.py@61
PS41, Line 61: def cancel(impalad, query_id):
> Some of these functions are similar to existing functions in impala_service
wait_for_query_state doesn't work because we don't have a handle in most cases 
for these tests. Handles are only returned after the query has started running. 
Cleaned this up a bit though.



--
To view, visit http://gerrit.cloudera.org:8080/21803
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d25d4c7fb0b8dcc7dad9510db1e8dca220eeb86
Gerrit-Change-Number: 21803
Gerrit-PatchSet: 43
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[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: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Thu, 29 May 2025 23:31:56 +0000
Gerrit-HasComments: Yes

Reply via email to