You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by kw...@apache.org on 2019/07/09 17:42:52 UTC

[impala] 02/03: IMPALA-8748: Always pass hostname to RpcMgr::GetProxy()

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

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

commit fce190a5d96f722f549983e2deb1fd5c20a27af7
Author: Michael Ho <kw...@cloudera.com>
AuthorDate: Mon Jul 8 17:28:41 2019 -0700

    IMPALA-8748: Always pass hostname to RpcMgr::GetProxy()
    
    This change fixes some callers of RpcMgr::GetProxy() tp
    pass the hostname instead of the resolved IP adddress.
    The hostname passed to RpcMgr::GetProxy() is used to
    construct the principal name of the remote destination
    when Kerberos is enabled.
    
    Change-Id: I85b661c8c3b3b67bfc1ce9e29aecb90a862666c0
    Reviewed-on: http://gerrit.cloudera.org:8080/13818
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/rpc/rpc-mgr.h                        |  7 ++++---
 be/src/runtime/coordinator-backend-state.cc |  6 ++----
 be/src/service/client-request-state.cc      | 13 +++++++------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/be/src/rpc/rpc-mgr.h b/be/src/rpc/rpc-mgr.h
index eb45203..e88579f 100644
--- a/be/src/rpc/rpc-mgr.h
+++ b/be/src/rpc/rpc-mgr.h
@@ -143,9 +143,10 @@ class RpcMgr {
   bool Authorize(const string& service_name, kudu::rpc::RpcContext* context,
       MemTracker* mem_tracker) const;
 
-  /// Creates a new proxy of type P at location 'address' with hostname 'hostname' and
-  /// places it in 'proxy'. 'P' must descend from kudu::rpc::Proxy. Note that 'address'
-  /// must be a resolved IP address.
+  /// Creates a new proxy of type P to a host with IP address 'address' and hostname
+  /// 'hostname'. Please note that 'address' has to be a resolved IP address and
+  /// 'hostname' has to match the hostname used in the Kerberos principal of the
+  /// destination host if Kerberos is enabled. 'P' must descend from kudu::rpc::Proxy.
   template <typename P>
   Status GetProxy(const TNetworkAddress& address, const std::string& hostname,
       std::unique_ptr<P>* proxy) WARN_UNUSED_RESULT;
diff --git a/be/src/runtime/coordinator-backend-state.cc b/be/src/runtime/coordinator-backend-state.cc
index 19709f7..4b9bbe1 100644
--- a/be/src/runtime/coordinator-backend-state.cc
+++ b/be/src/runtime/coordinator-backend-state.cc
@@ -188,8 +188,7 @@ void Coordinator::BackendState::Exec(
     exec_complete_barrier->Notify();
   });
   std::unique_ptr<ControlServiceProxy> proxy;
-  Status get_proxy_status =
-      ControlService::GetProxy(krpc_host_, krpc_host_.hostname, &proxy);
+  Status get_proxy_status = ControlService::GetProxy(krpc_host_, host_.hostname, &proxy);
   if (!get_proxy_status.ok()) {
     SetExecError(get_proxy_status);
     return;
@@ -495,8 +494,7 @@ bool Coordinator::BackendState::Cancel() {
              << " backend=" << TNetworkAddressToString(krpc_host_);
 
   std::unique_ptr<ControlServiceProxy> proxy;
-  Status get_proxy_status =
-      ControlService::GetProxy(krpc_host_, krpc_host_.hostname, &proxy);
+  Status get_proxy_status = ControlService::GetProxy(krpc_host_, host_.hostname, &proxy);
   if (!get_proxy_status.ok()) {
     status_.MergeStatus(get_proxy_status);
     VLOG_QUERY << "Cancel query_id= " << PrintId(query_id()) << " could not get proxy to "
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index 452a546..afdfb36 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -644,20 +644,21 @@ Status ClientRequestState::ExecShutdownRequest() {
             << " to ip address, error: " << ip_status.GetDetail();
     return ip_status;
   }
-  TNetworkAddress addr = MakeNetworkAddress(ip_address, port);
+  TNetworkAddress krpc_addr = MakeNetworkAddress(ip_address, port);
 
   std::unique_ptr<ControlServiceProxy> proxy;
-  Status get_proxy_status = ControlService::GetProxy(addr, addr.hostname, &proxy);
+  Status get_proxy_status =
+      ControlService::GetProxy(krpc_addr, request.backend.hostname, &proxy);
   if (!get_proxy_status.ok()) {
     return Status(
         Substitute("Could not get Proxy to ControlService at $0 with error: $1.",
-            TNetworkAddressToString(addr), get_proxy_status.msg().msg()));
+            TNetworkAddressToString(krpc_addr), get_proxy_status.msg().msg()));
   }
 
   RemoteShutdownParamsPB params;
   if (request.__isset.deadline_s) params.set_deadline_s(request.deadline_s);
   RemoteShutdownResultPB resp;
-  VLOG_QUERY << "Sending Shutdown RPC to " << TNetworkAddressToString(addr);
+  VLOG_QUERY << "Sending Shutdown RPC to " << TNetworkAddressToString(krpc_addr);
 
   const int num_retries = 3;
   const int64_t timeout_ms = 10 * MILLIS_PER_SEC;
@@ -669,10 +670,10 @@ Status ClientRequestState::ExecShutdownRequest() {
   if (!rpc_status.ok()) {
     const string& msg = rpc_status.msg().msg();
     VLOG_QUERY << "RemoteShutdown query_id= " << PrintId(query_id())
-               << " failed to send RPC to " << TNetworkAddressToString(addr) << " :"
+               << " failed to send RPC to " << TNetworkAddressToString(krpc_addr) << " :"
                << msg;
     string err_string = Substitute(
-        "Rpc to $0 failed with error '$1'", TNetworkAddressToString(addr), msg);
+        "Rpc to $0 failed with error '$1'", TNetworkAddressToString(krpc_addr), msg);
     // Attempt to detect if the the failure is because of not using a KRPC port.
     if (backend_port_specified
         && msg.find("RemoteShutdown() RPC failed: Timed out: connection negotiation to")