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: (3 comments) a few early comments/questions 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@496 PS7, Line 496: const int64_t max_requests = pool_cfg.max_requests; This code looks weird in that most other code now uses GetMaxRequestsForPool() rather then looking at pool config fields. It also seems inconsistent in that we are ignoring max_running_queries_multiple here, but are still using GetMaxMemForPool() in HasAvailableMemResources(). 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) { Are we still considering groups in a deterministic order? Is it deliberate that we would then tend to fill some groups first? Or do we want to balance work across groups? http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/executor-group.h File be/src/scheduling/executor-group.h: http://gerrit.cloudera.org:8080/#/c/13550/7/be/src/scheduling/executor-group.h@93 PS7, Line 93: /// executors running on the same host (e.g. during tests) are counted individually. Maybe no need to mention backend descriptors here -- 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 00:05:04 +0000 Gerrit-HasComments: Yes
