Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16412 )
Change subject: IMPALA-9930 (part 2): Introduce new admission control rpc service ...................................................................... Patch Set 7: (2 comments) Read through the headers, the tests and the protobuf, so pushing out a batch of comments. Need to read through implementation next. http://gerrit.cloudera.org:8080/#/c/16412/7/common/protobuf/admission_control_service.proto File common/protobuf/admission_control_service.proto: http://gerrit.cloudera.org:8080/#/c/16412/7/common/protobuf/admission_control_service.proto@233 PS7, Line 233: rpc GetQueryStatus(GetQueryStatusRequestPB) returns (GetQueryStatusResponsePB); Does this return immediately and require the client to throttle polling, or is it more of a long-polling thing where it can block for a bit? Would be useful to document the expected usage pattern briefly. If a coordinator fails, how do we clean up admission requests? Is GetQueryStatus() effectively a keepalive? http://gerrit.cloudera.org:8080/#/c/16412/7/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/16412/7/tests/custom_cluster/test_admission_controller.py@1303 PS7, Line 1303: class TestAdmissionControllerWithACService(TestAdmissionController): How much time does this add to exhaustive tests? -- To view, visit http://gerrit.cloudera.org:8080/16412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I594fc593a27b24b6952e381a9bc1a9a5c6b757ae Gerrit-Change-Number: 16412 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 28 Oct 2020 16:49:17 +0000 Gerrit-HasComments: Yes
