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 15:

> 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.

Done. Filed IMPALA-7081

 > 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.

Please see comment against each point

- GetOperationStatus() while queued should show PENDING/OK
  => covered in test_cancellation added to test_admission_controller.py where 
query state is checked using a beeswax client instead

- GetOperationStatus() after rejection should show reason.
  => GetOperationStatus contains the operation state and the status message. 
Unfortunately for a CANCELLED status the message is empty. Instead we can take 
care of this coverage after IMPALA-1262 where we can check for operation state 
to verify this scenario.

- CloseOperation() while queued.
  => covered in test_cancellation added to test_admission_controller.py

- FetchResults() while queued (our current semantics are to allow this and 
block until the query reaches FINISHED, I believe).
  => covered in a number of other tests. eg. test_fetch.py where fetch is 
called right after an execute RPC returns.

- GetRuntimeProfile() while queued.
  => added this coverage to test_cancellation in test_admission_controller.py


--
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 <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Sat, 26 May 2018 02:28:31 +0000
Gerrit-HasComments: No

Reply via email to