You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bi...@apache.org on 2021/10/07 01:39:14 UTC

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

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

bikram pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit cee2b42781658f42dc08a41cf88bb2ed2803aaf1
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 de034d0..374da30 100644
--- a/be/src/scheduling/admission-control-service.cc
+++ b/be/src/scheduling/admission-control-service.cc
@@ -328,7 +328,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 da79c07..432f319 100644
--- a/be/src/scheduling/admission-controller.cc
+++ b/be/src/scheduling/admission-controller.cc
@@ -1165,9 +1165,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);
 
@@ -1281,7 +1281,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 e52a769..fec28d5 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());