Henry Robinson has posted comments on this change. Change subject: IMPALA-4041: Limit catalog and admission control updates to coordinators ......................................................................
Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/6344/3/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: PS3, Line 180: if (FLAGS_disable_admission_control) LOG(INFO) << "Admission control is disabled."; : if (!FLAGS_disable_admission_control) { if (!FLAGS_disable_admission_control) { } else { LOG(INFO) << ... } http://gerrit.cloudera.org:8080/#/c/6344/3/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: Line 200: /// Registers the request queue topic. ... with the statestore. http://gerrit.cloudera.org:8080/#/c/6344/3/be/src/scheduling/scheduler-test-util.cc File be/src/scheduling/scheduler-test-util.cc: Line 457: return scheduler_->ComputeScanRangeAssignment(*scheduler_->GetBackendConfig(), 0, nullptr, long line http://gerrit.cloudera.org:8080/#/c/6344/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: PS3, Line 1489: Otherwise, this operation : // is performed by the Scheduler. Can you just ensure that this is always done by this callback, and remove the logic to do from the scheduler? http://gerrit.cloudera.org:8080/#/c/6344/3/be/src/testutil/in-process-servers.h File be/src/testutil/in-process-servers.h: PS3, Line 95: Not owned by this class; : /// instead it's owned via shared_ptrs Ownership is shared now. -- To view, visit http://gerrit.cloudera.org:8080/6344 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f2c74abdbcd60ac050efa323616bd41182ceff3 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-HasComments: Yes