Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 )
Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups ...................................................................... Patch Set 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/runtime/exec-env.cc@161 PS9, Line 161: t : sna > nit: hand off. How about SendClusterMembershipToFrontend()? Done http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/runtime/exec-env.cc@177 PS9, Line 177: > move the helper into an anonymous namespace above the impala namespace Done http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.h@951 PS9, Line 951: const string& g > Pass by const ref, looks like the implementation doesn't need a copy Done http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.cc@1871 PS9, Line 1871: // There might be lingering queries from when this group was active. > Maybe this code could go into a helper, e.g. MaxQueriesOnExecutorGroupHosts good point, however I'll keep it for now to avoid passing around ClusterMembershipMgr::* typedefs http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.cc@1873 PS9, Line 1873: auto new_grp_it = snapshot->executor_groups.find(new_grp); > usually we name iterators 'it', but since this is the second one in this me Done http://gerrit.cloudera.org:8080/#/c/14103/9/be/src/scheduling/admission-controller.cc@1877 PS9, Line 1877: const string& host = TNetworkAddressToString(be_desc.address); > const string& Done -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 10 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@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-Comment-Date: Tue, 10 Sep 2019 20:07:26 +0000 Gerrit-HasComments: Yes
