Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23257 )

Change subject: IMPALA-14276: Fix memory leak by removing AdmissionState on 
rejection
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23257/3/be/src/scheduling/admission-control-service.cc
File be/src/scheduling/admission-control-service.cc:

http://gerrit.cloudera.org:8080/#/c/23257/3/be/src/scheduling/admission-control-service.cc@215
PS3, Line 215:   if (admission_state->admission_done && 
!admission_state->admit_status.ok()) {
             :     LOG(INFO) << "Query " << req->query_id()
             :               << " was rejected. Removing admission state to 
free resources.";
             :     // If this RPC fails and the admission state is already 
removed,
             :
> This could be a future improvement since addressing the memory growth seems
I think I will file another jira for it. Added comments.


http://gerrit.cloudera.org:8080/#/c/23257/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/23257/4/tests/custom_cluster/test_admission_controller.py@2374
PS4, Line 2374: number_of_iterations = 500
> How long is takes to run 1000 queries? Is lower count sufficient to reveal
It is around 140s, now is increasing from around 100MB to 140MB with 1000 
queries if not fixing this issue. Changed to 500, seems good to detect this 
case.


http://gerrit.cloudera.org:8080/#/c/23257/4/tests/custom_cluster/test_admission_controller.py@2389
PS4, Line 2389:     # Check if the admission state map size stays 1 all the 
time, which is
> The remaining 1 is the long_query?
Yeah, the remaining count of 1 is from a long running query. In this test, we 
examine the log showing 1000 times, and we can’t cancel the long query during 
testing. Having 1 remaining is acceptable, since we’re checking for leaks, as 
long as the count doesn’t keep increasing, it should be good enough. Added some 
comments.



--
To view, visit http://gerrit.cloudera.org:8080/23257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9fba4f176c648ed7811225f7f94c91342a724d10
Gerrit-Change-Number: 23257
Gerrit-PatchSet: 5
Gerrit-Owner: Yida Wu <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Wed, 06 Aug 2025 21:28:01 +0000
Gerrit-HasComments: Yes

Reply via email to