Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/13550 )
Change subject: WIP IMPALA-8484: Run queries on disjoint executor groups ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/admission-controller.cc@1336 PS7, Line 1336: for (auto& it: queue_node->per_group_schedules) { > Yes, per_group_schedules is a map, so the order is deterministic and this i I agree that the advantage of filling one group before starting another is that is does make it easier to choose an executor group to remove when scaling down. On the other hand does this result in situations that look weird to the user? For example, suppose we predict we need more capacity, scale up and add a new executor group. Now we have extra capacity but we don't use it until we have to, does that look weird? Another reason to pack executor groups might be to make better use of the remote read cache. Overall I think this is probably OK overall. I do think the policy should be documented more explicitly in the code. -- To view, visit http://gerrit.cloudera.org:8080/13550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a1d0900f2a82bd2fc0a906cc094e442cffa189b Gerrit-Change-Number: 13550 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 28 Jun 2019 21:24:50 +0000 Gerrit-HasComments: Yes