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

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

commit f0e602dbb9e8bada186b4b4a214c636e2bc65e7d
Author: Abhishek Rawat <[email protected]>
AuthorDate: Wed Jan 4 22:35:19 2023 -0800

    IMPALA-11827: do not cache admission control service's IP address in impalad
    
    The Impalad's ExecEnv caches resolved IP address for admission control
    service. The Impala server runs a heart beat thread for admission
    control service and it relies on the cached resolved address in ExecEnv.
    This breaks when IP address for admission control services changes.
    
    This patch removes the 'admission_service_address_' member and its
    getter in the ExecEnv class and adds a GetAdmissionServiceAddress()
    interface to dynamically get the resolved address, if global admission
    control service is enabled ('admission_service_host' flag is set).
    
    Testing: Manually tested by restarting global admission controller
    service in a Kubernetes setup where it gets new IP address on startup.
    Coordinator's admission heart beat thread was able to connect to the
    admission control service using the new IP address. Also made sure that
    the coordinator and admissiond communication was working as expected
    upon admissiond restarts.
    
    Change-Id: I09b4c52644f9e1b3c1f0e68c9900464d722517af
    Reviewed-on: http://gerrit.cloudera.org:8080/19403
    Reviewed-by: Impala Public Jenkins <[email protected]>
    Tested-by: Impala Public Jenkins <[email protected]>
---
 be/src/runtime/exec-env.cc                     | 22 +++++++++++++---------
 be/src/runtime/exec-env.h                      | 10 +++-------
 be/src/scheduling/admission-control-service.cc |  5 ++++-
 be/src/service/impala-server.cc                |  2 +-
 4 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index 3c57abdd3..c796c1337 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -273,15 +273,6 @@ ExecEnv::ExecEnv(int krpc_port, int subscriber_port, int 
webserver_port,
         FLAGS_metrics_webserver_port, metrics_.get(), 
Webserver::AuthMode::NONE));
   }
 
-  if (AdmissionServiceEnabled()) {
-    IpAddr ip;
-    ABORT_IF_ERROR(HostnameToIpAddr(FLAGS_admission_service_host, &ip));
-    // TODO: get BackendId of admissiond in global admission control mode.
-    // Use admissiond's IP address as unique ID for UDS now.
-    admission_service_address_ = MakeNetworkAddressPB(
-        ip, FLAGS_admission_service_port, UdsAddressUniqueIdPB::IP_ADDRESS);
-  }
-
   exec_env_ = this;
 }
 
@@ -636,4 +627,17 @@ bool ExecEnv::AdmissionServiceEnabled() const {
   return !FLAGS_admission_service_host.empty();
 }
 
+Status ExecEnv::GetAdmissionServiceAddress(
+    NetworkAddressPB& admission_service_address) const {
+  if (AdmissionServiceEnabled()) {
+    IpAddr ip;
+    RETURN_IF_ERROR(HostnameToIpAddr(FLAGS_admission_service_host, &ip));
+    // TODO: get BackendId of admissiond in global admission control mode.
+    // Use admissiond's IP address as unique ID for UDS now.
+    admission_service_address = MakeNetworkAddressPB(
+        ip, FLAGS_admission_service_port, UdsAddressUniqueIdPB::IP_ADDRESS);
+  }
+  return Status::OK();
+}
+
 } // namespace impala
diff --git a/be/src/runtime/exec-env.h b/be/src/runtime/exec-env.h
index bf710a12e..f15e674b3 100644
--- a/be/src/runtime/exec-env.h
+++ b/be/src/runtime/exec-env.h
@@ -185,9 +185,9 @@ class ExecEnv {
   int64_t admit_mem_limit() const { return admit_mem_limit_; }
   int64_t admission_slots() const { return admission_slots_; }
 
-  const NetworkAddressPB& admission_service_address() const {
-    return admission_service_address_;
-  }
+  /// Gets the resolved IP address and port where the admission control 
service is
+  /// running, if enabled.
+  Status GetAdmissionServiceAddress(NetworkAddressPB& address) const;
 
   /// Returns true if the admission control service is enabled.
   bool AdmissionServiceEnabled() const;
@@ -296,10 +296,6 @@ class ExecEnv {
   /// this backend. Queries take up multiple slots only when mt_dop > 1.
   int64_t admission_slots_;
 
-  /// If the admission control service is enabled, the resolved IP address and 
port where
-  /// the service is running.
-  NetworkAddressPB admission_service_address_;
-
   /// Initialize ExecEnv based on Hadoop config from frontend.
   Status InitHadoopConfig();
 
diff --git a/be/src/scheduling/admission-control-service.cc 
b/be/src/scheduling/admission-control-service.cc
index 374da302a..3cf4682ab 100644
--- a/be/src/scheduling/admission-control-service.cc
+++ b/be/src/scheduling/admission-control-service.cc
@@ -116,9 +116,12 @@ void AdmissionControlService::Join() {
 
 Status AdmissionControlService::GetProxy(
     unique_ptr<AdmissionControlServiceProxy>* proxy) {
+  NetworkAddressPB admission_service_address;
+  RETURN_IF_ERROR(ExecEnv::GetInstance()->GetAdmissionServiceAddress(
+      admission_service_address));
   // Create a AdmissionControlService proxy to the destination.
   RETURN_IF_ERROR(ExecEnv::GetInstance()->rpc_mgr()->GetProxy(
-      ExecEnv::GetInstance()->admission_service_address(), 
FLAGS_admission_service_host,
+      admission_service_address, FLAGS_admission_service_host,
       proxy));
   return Status::OK();
 }
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 5d7b77dbc..95fd16362 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -2826,7 +2826,7 @@ void ImpalaServer::UnregisterSessionTimeout(int32_t 
session_timeout) {
     if (!get_proxy_status.ok()) {
       LOG(ERROR) << "Admission heartbeat thread was unabe to get an "
                     "AdmissionControlService proxy:"
-                 << get_proxy_status;
+                 << get_proxy_status.GetDetail();
       continue;
     }
 

Reply via email to