Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/22453 )
Change subject: IMPALA-13736: Fix Use-After-Free in ExecutorGroup.RemoveExecutor ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/22453/1/be/src/scheduling/executor-group.cc File be/src/scheduling/executor-group.cc: http://gerrit.cloudera.org:8080/#/c/22453/1/be/src/scheduling/executor-group.cc@159 PS1, Line 159: const int64_t be_admin_mem > This can be moved to the very last before function return. I tried moving this statement around but ran into a couple different problems: the per executor mem limit for admission was incorrectly calculated, the executor was not properly removed from the executor group, or the test JVM crashed. Unfortunately it seems as though the order of these statements is very specific and cannot be modified. http://gerrit.cloudera.org:8080/#/c/22453/1/be/src/scheduling/executor-group.cc@164 PS1, Line 164: lculatePerExecut > If erasing at the very end, this can change to be_descs.size() == 1 see previous comment http://gerrit.cloudera.org:8080/#/c/22453/1/be/src/scheduling/executor-group.cc@167 PS1, Line 167: executor_map_.erase(be_descs_it); > Please add comment in executor-group.h for executor_ip_hash_ring_ that it m Done http://gerrit.cloudera.org:8080/#/c/22453/2/be/src/scheduling/executor-group.cc File be/src/scheduling/executor-group.cc: http://gerrit.cloudera.org:8080/#/c/22453/2/be/src/scheduling/executor-group.cc@155 PS2, Line 155: // Copy the data necessary to update the internal state variables since the erase call > Please include a comment here about why we're making copies, so it doesn't Done -- To view, visit http://gerrit.cloudera.org:8080/22453 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If14a3c89ee631ebb05efc9a47745f7e63ab98690 Gerrit-Change-Number: 22453 Gerrit-PatchSet: 3 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Thu, 06 Feb 2025 18:27:52 +0000 Gerrit-HasComments: Yes
