Yida Wu 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//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23763/2//COMMIT_MSG@12
PS2, Line 12: .
> nit: missing space
Done


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.
> Better to use .notify_all() in case the is a future need to have multiple w
Done


http://gerrit.cloudera.org:8080/#/c/23763/2/be/src/scheduling/admission-control-service.cc@420
PS2, Line 420:     curr_version = update_version;
> If the ability to define a predicate on .wait() is used, then shutdown_ no
I moved the unique_lock outside the loop as you suggested.
For shutdown_, I prefer to keep it atomic. It represents the whole service 
state, not just this queue. Atomic makes it safer for other threads to check it 
without needing this specific lock.
Also, I used a local queue with swap, this allows removing the map entries 
without blocking the queue.


http://gerrit.cloudera.org:8080/#/c/23763/2/be/src/scheduling/admission-control-service.cc@427
PS2, Line 427:   {
> In change https://gerrit.cloudera.org/c/23763/, I added comments about avoi
Use another way to implement


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:
> Nit:  queries
Done


http://gerrit.cloudera.org:8080/#/c/23763/2/tests/custom_cluster/test_admission_controller.py@2479
PS2, Line 2479:   @SkipIfNotHdfsMinicluster.tuned_for_minicluster
> Is there a way to wait for a certain metric value or a certain log message
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: Yida Wu <[email protected]>
Gerrit-Comment-Date: Mon, 15 Dec 2025 08:16:10 +0000
Gerrit-HasComments: Yes

Reply via email to