Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21896 )
Change subject: IMPALA-12146: Fix incorrect host memory reserved when the executor quits abnormally ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller-test.cc File be/src/scheduling/admission-controller-test.cc: http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller-test.cc@549 PS3, Line 549: /// Test that removing hosts from AdmissionController, and check if the memory reserved in > Add a brief descriptive comment Done http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller.cc@1768 PS3, Line 1768: pool_stats_removed_nodes.insert(topic_backend_id); > The method comment says "Any remote host stats removed during the process w Thanks Andrew. The UpdateRemoteStats() would remove the remote stats from the pool stats. But it seems in UpdateClusterAggregates() line 1861, it only uses the pool stats for aggregated memory reserved calculation. It doesn't use remote_per_host_stats_, so I remove the line 1786, and handle the case when a host is removed from the pool only. Retest it, and seems good. http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller.cc@1880 PS3, Line 1880: // update by the set pool_stats_removed_nodes. If a host was removed and no longer > Nit: I know this line is describing what we are doing but it does not descr Done http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller.cc@1881 PS3, Line 1881: sts i > Nit: maybe "know" is better, we already did thr check Done http://gerrit.cloudera.org:8080/#/c/21896/3/be/src/scheduling/admission-controller.cc@1892 PS3, Line 1892: (old > Maybe "mem_reserved" or similar would be clearer than just "Stats"? Done -- To view, visit http://gerrit.cloudera.org:8080/21896 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6f6edd28c55904d63d0c494230ee2bf7a0f6cce Gerrit-Change-Number: 21896 Gerrit-PatchSet: 4 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 08 Oct 2024 22:17:11 +0000 Gerrit-HasComments: Yes
