Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10060 )
Change subject: IMPALA-5216: Make admission control queuing async ...................................................................... Patch Set 14: (2 comments) > Do we have sufficient test coverage for the various webserver/RPC > handlers that can happen concurrently with admission control > (previously, less cases were possible since the query handle wasn't > returned until after admission control)? AFAIK we currently dont have any test coverage for webserver calls apart from those in test_redaction. We should probably add that coverage in a separate patch. However, I have added tests for the getState and getExecSummary client RPCs. Also added a session expiry test to check if queries in queue get cancelled. 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: } > Maybe we could add something short explaining this condition. E.g. "Ensure Done 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 > @Tim You are right, anything not used/set is just returned as empty which i Done -- 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: 14 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 24 May 2018 22:37:30 +0000 Gerrit-HasComments: Yes
