Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10060 )
Change subject: IMPALA-5216: Make admission control queuing async ...................................................................... Patch Set 15: > (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. Could you file a JIRA for that? We probably have some incidental coverage since we probably scrape some of the webpages to determine state, but it does seem good to have explicit tests. > 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. Thanks for adding those. How about these HS2 cases (we might already have coverage but just want to be sure): - GetOperationStatus() while queued should show PENDING/OK - GetOperationStatus() after rejection should show reason. - CloseOperation() while queued. - FetchResults() while queued (our current semantics are to allow this and block until the query reaches FINISHED, I believe). - GetRuntimeProfile() while queued. I think it would be okay to add some of these in a follow up change if we don't already have the coverage as long as we're sure we can do that soonish. -- 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: 15 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: Fri, 25 May 2018 17:00:06 +0000 Gerrit-HasComments: No
