Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/23763 )
Change subject: IMPALA-14605: Fix memory leak in global admissiond for cancelled queued queries ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/23763/2/be/src/scheduling/admission-control-service.cc File be/src/scheduling/admission-control-service.cc: http://gerrit.cloudera.org:8080/#/c/23763/2/be/src/scheduling/admission-control-service.cc@410 PS2, Line 410: // Release the memory against the control service's memory tracker. > Done Done http://gerrit.cloudera.org:8080/#/c/23763/2/be/src/scheduling/admission-control-service.cc@420 PS2, Line 420: curr_version = update_version; > I moved the unique_lock outside the loop as you suggested. Makes sense on keeping shutdown_ atomic. Nice technique to use swap to avoid blocking the admission_state_map_! http://gerrit.cloudera.org:8080/#/c/23763/3/be/src/scheduling/admission-control-service.cc File be/src/scheduling/admission-control-service.cc: http://gerrit.cloudera.org:8080/#/c/23763/3/be/src/scheduling/admission-control-service.cc@148 PS3, Line 148: std::lock_guard<std::mutex> l(cleanup_queue_lock_); No need to take ownership of the mutex before calling notify_all: https://en.cppreference.com/w/cpp/thread/condition_variable/notify_all.html#:~:text=The%20notifying%20thread,notified%20under%20lock. http://gerrit.cloudera.org:8080/#/c/23763/3/be/src/scheduling/admission-control-service.cc@438 PS3, Line 438: this Instead of passing "this" to a lambda, pass references to the two variables it uses. http://gerrit.cloudera.org:8080/#/c/23763/2/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/23763/2/tests/custom_cluster/test_admission_controller.py@2468 PS2, Line 2468: > Done Done http://gerrit.cloudera.org:8080/#/c/23763/2/tests/custom_cluster/test_admission_controller.py@2479 PS2, Line 2479: @SkipIfNotHdfsMinicluster.tuned_for_minicluster > Done Done -- To view, visit http://gerrit.cloudera.org:8080/23763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f16f3cf986e2038d6486d3ec687135d561e2cbf Gerrit-Change-Number: 23763 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 16 Dec 2025 20:53:52 +0000 Gerrit-HasComments: Yes
