Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10060 )

Change subject: IMPALA-5216: Make admission control queuing async
......................................................................


Patch Set 12:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10060/12/be/src/service/client-request-state.cc@975
PS12, Line 975:   // RPCs and can block for a long time.
Maybe we could add something short explaining this condition. E.g. "Ensure the 
parent query is cancelled if execution has started (if the query was not 
started, cancellation is handled by the async thread)."


http://gerrit.cloudera.org:8080/#/c/10060/12/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/10060/12/be/src/service/impala-http-handler.cc@776
PS12, Line 776:  request_state->operation_state() != 
TOperationState::ERROR_STATE
> I guess another way to as it is what does this error mean "invalid query id
Yeah the logic of this function is a bit weird. It looks like we actually 
return success with an empty summary, etc for archived DDL statements (and 
other things that wouldn't have had a coordinator when running).

So it would be more consistent to do the same for a running query and would 
allow deleting some of this code.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 12
Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 May 2018 05:09:34 +0000
Gerrit-HasComments: Yes

Reply via email to