Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4833: Compute precise per-host reservation size ......................................................................
Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: Line 63: //const vector<const FInstanceExecParams*>& instance_params_list, > Remove? Done http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/scheduling/query-schedule.h File be/src/scheduling/query-schedule.h: PS2, Line 55: FInstanceExecParams* > Maybe make it const& since it can't be null? Can't do it without some other trickery: https://stackoverflow.com/questions/10531010/const-references-in-stdvector-elements Line 251: /// TODO: Replace uses of unique_hosts_ with per_backend_exec_params. > It looks like there are a limited number of callsites, can we just do this Done Line 251: /// TODO: Replace uses of unique_hosts_ with per_backend_exec_params. > It looks like there are a limited number of callsites, can we just do this Done http://gerrit.cloudera.org:8080/#/c/7630/2/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: Line 688: // TODO: Move to admission control, it doesn't need to be in the Scheduler. > What if admission control is disabled? We don't really need the request pool for anything else anymore. It used to live here because the Llama path used it as well, and the Llama/YARN reservation happened outside of AC. That said, if I move this I'd try not to change the behavior, i.e. even if admission control is disabled I'll still resolve the request pool since we do that today. http://gerrit.cloudera.org:8080/#/c/7630/2/tests/custom_cluster/test_mem_reservations.py File tests/custom_cluster/test_mem_reservations.py: Line 38: config_map = {'DEFAULT_SPILLABLE_BUFFER_SIZE': '33554432', > The choice of queries, params, etc needs some explanation. Done -- To view, visit http://gerrit.cloudera.org:8080/7630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idbcd1e9b1be14edc4017b4907e83f9d56059fbac Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
