You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2021/10/08 00:54:45 UTC

[impala] 02/02: IMPALA-10942: Fix memory leak in admission controller

This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to branch branch-4.0.1
in repository https://gitbox.apache.org/repos/asf/impala.git

commit edca349527657a9847659d7b0400bf62aa6938ce
Author: Bikramjeet Vig <bi...@gmail.com>
AuthorDate: Thu Sep 30 19:03:15 2021 -0700

    IMPALA-10942: Fix memory leak in admission controller
    
    This patch fixes a memory leak in the admission controller where
    objects tracking state required for queuing were being retained
    despite the query being admitted or rejected immediately.
    
    Change-Id: I1df0de0e658f6dcb5a044d108d0943aaa3dea0a1
    Reviewed-on: http://gerrit.cloudera.org:8080/17893
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/scheduling/admission-control-service.cc      | 2 +-
 be/src/scheduling/admission-controller.cc           | 6 +++---
 be/src/scheduling/admission-controller.h            | 2 +-
 be/src/scheduling/local-admission-control-client.cc | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/be/src/scheduling/admission-control-service.cc b/be/src/scheduling/admission-control-service.cc
index f00f335..748107f 100644
--- a/be/src/scheduling/admission-control-service.cc
+++ b/be/src/scheduling/admission-control-service.cc
@@ -323,7 +323,7 @@ void AdmissionControlService::AdmitFromThreadPool(UniqueIdPB query_id) {
         admission_state->blacklisted_executor_addresses};
     admission_state->admit_status =
         AdmissiondEnv::GetInstance()->admission_controller()->SubmitForAdmission(request,
-            &admission_state->admit_outcome, &admission_state->schedule, &queued,
+            &admission_state->admit_outcome, &admission_state->schedule, queued,
             &admission_state->request_pool);
     admission_state->submitted = true;
     if (!queued) {
diff --git a/be/src/scheduling/admission-controller.cc b/be/src/scheduling/admission-controller.cc
index 1d1f544..9aebfad 100644
--- a/be/src/scheduling/admission-controller.cc
+++ b/be/src/scheduling/admission-controller.cc
@@ -1133,9 +1133,9 @@ void AdmissionController::PoolStats::UpdateConfigMetrics(const TPoolConfig& pool
 
 Status AdmissionController::SubmitForAdmission(const AdmissionRequest& request,
     Promise<AdmissionOutcome, PromiseMode::MULTIPLE_PRODUCER>* admit_outcome,
-    unique_ptr<QuerySchedulePB>* schedule_result, bool* queued,
+    unique_ptr<QuerySchedulePB>* schedule_result, bool& queued,
     std::string* request_pool) {
-  *queued = false;
+  queued = false;
   DebugActionNoFail(request.query_options, "AC_BEFORE_ADMISSION");
   DCHECK(schedule_result->get() == nullptr);
 
@@ -1249,7 +1249,7 @@ Status AdmissionController::SubmitForAdmission(const AdmissionRequest& request,
       PROFILE_INFO_KEY_INITIAL_QUEUE_REASON, queue_node->initial_queue_reason);
 
   queue_node->wait_start_ms = MonotonicMillis();
-  *queued = true;
+  queued = true;
   return Status::OK();
 }
 
diff --git a/be/src/scheduling/admission-controller.h b/be/src/scheduling/admission-controller.h
index fb8bca2..635ceac 100644
--- a/be/src/scheduling/admission-controller.h
+++ b/be/src/scheduling/admission-controller.h
@@ -369,7 +369,7 @@ class AdmissionController {
   /// cancelled to ensure that the pool statistics are updated.
   Status SubmitForAdmission(const AdmissionRequest& request,
       Promise<AdmissionOutcome, PromiseMode::MULTIPLE_PRODUCER>* admit_outcome,
-      std::unique_ptr<QuerySchedulePB>* schedule_result, bool* queued,
+      std::unique_ptr<QuerySchedulePB>* schedule_result, bool& queued,
       std::string* request_pool = nullptr);
 
   /// After SubmitForAdmission(), if the query was queued this must be called. If
diff --git a/be/src/scheduling/local-admission-control-client.cc b/be/src/scheduling/local-admission-control-client.cc
index faf47a5..e85f34b 100644
--- a/be/src/scheduling/local-admission-control-client.cc
+++ b/be/src/scheduling/local-admission-control-client.cc
@@ -38,7 +38,7 @@ Status LocalAdmissionControlClient::SubmitForAdmission(
   query_events->MarkEvent(QUERY_EVENT_SUBMIT_FOR_ADMISSION);
   bool queued;
   Status status = ExecEnv::GetInstance()->admission_controller()->SubmitForAdmission(
-      request, &admit_outcome_, schedule_result, &queued);
+      request, &admit_outcome_, schedule_result, queued);
   if (queued) {
     query_events->MarkEvent(QUERY_EVENT_QUEUED);
     DCHECK(status.ok());