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