You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2017/09/05 22:26:48 UTC

[3/9] incubator-impala git commit: rpc: move ConnectionId to its own file

rpc: move ConnectionId to its own file

This class was previously implemented inside of outbound_call.{cc,h}
where it didn't quite belong. In the several years since the code was
initially written we've moved more towards a "single class per header"
model unless the classes are truly trivial or really tightly coupled.
Neither is the case here.

Change-Id: I5343c2f6ad305b5927d71457d5d19e3799052ec9
Reviewed-on: http://gerrit.cloudera.org:8080/7685
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/7895
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Sailesh Mukil <sa...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/7d41b96b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/7d41b96b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/7d41b96b

Branch: refs/heads/master
Commit: 7d41b96b23c2a084247f38dcdf068053e46e432c
Parents: e7bd0ce
Author: Todd Lipcon <to...@apache.org>
Authored: Tue Aug 15 15:06:35 2017 -0700
Committer: Sailesh Mukil <sa...@cloudera.com>
Committed: Fri Sep 1 03:11:43 2017 +0000

----------------------------------------------------------------------
 be/src/kudu/rpc/CMakeLists.txt   |  1 +
 be/src/kudu/rpc/connection_id.cc | 85 +++++++++++++++++++++++++++++++++++
 be/src/kudu/rpc/connection_id.h  | 81 +++++++++++++++++++++++++++++++++
 be/src/kudu/rpc/outbound_call.cc | 63 +-------------------------
 be/src/kudu/rpc/outbound_call.h  | 60 ++-----------------------
 be/src/kudu/rpc/proxy.h          |  2 +-
 be/src/kudu/rpc/reactor.h        |  2 +
 be/src/kudu/rpc/rpc_context.cc   |  1 -
 8 files changed, 175 insertions(+), 120 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/CMakeLists.txt b/be/src/kudu/rpc/CMakeLists.txt
index 643b854..f15e017 100644
--- a/be/src/kudu/rpc/CMakeLists.txt
+++ b/be/src/kudu/rpc/CMakeLists.txt
@@ -45,6 +45,7 @@ set(KRPC_SRCS
     blocking_ops.cc
     client_negotiation.cc
     connection.cc
+    connection_id.cc
     constants.cc
     inbound_call.cc
     messenger.cc

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/connection_id.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/connection_id.cc b/be/src/kudu/rpc/connection_id.cc
new file mode 100644
index 0000000..e4a4dba
--- /dev/null
+++ b/be/src/kudu/rpc/connection_id.cc
@@ -0,0 +1,85 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/rpc/connection_id.h"
+
+#include <boost/functional/hash.hpp>
+
+#include "kudu/gutil/stringprintf.h"
+
+using std::string;
+
+namespace kudu {
+namespace rpc {
+
+ConnectionId::ConnectionId() {}
+
+ConnectionId::ConnectionId(const ConnectionId& other) {
+  DoCopyFrom(other);
+}
+
+ConnectionId::ConnectionId(const Sockaddr& remote, UserCredentials user_credentials) {
+  remote_ = remote;
+  user_credentials_ = std::move(user_credentials);
+}
+
+void ConnectionId::set_remote(const Sockaddr& remote) {
+  remote_ = remote;
+}
+
+void ConnectionId::set_user_credentials(UserCredentials user_credentials) {
+  user_credentials_ = std::move(user_credentials);
+}
+
+void ConnectionId::CopyFrom(const ConnectionId& other) {
+  DoCopyFrom(other);
+}
+
+string ConnectionId::ToString() const {
+  // Does not print the password.
+  return StringPrintf("{remote=%s, user_credentials=%s}",
+      remote_.ToString().c_str(),
+      user_credentials_.ToString().c_str());
+}
+
+void ConnectionId::DoCopyFrom(const ConnectionId& other) {
+  remote_ = other.remote_;
+  user_credentials_ = other.user_credentials_;
+}
+
+size_t ConnectionId::HashCode() const {
+  size_t seed = 0;
+  boost::hash_combine(seed, remote_.HashCode());
+  boost::hash_combine(seed, user_credentials_.HashCode());
+  return seed;
+}
+
+bool ConnectionId::Equals(const ConnectionId& other) const {
+  return (remote() == other.remote()
+       && user_credentials().Equals(other.user_credentials()));
+}
+
+size_t ConnectionIdHash::operator() (const ConnectionId& conn_id) const {
+  return conn_id.HashCode();
+}
+
+bool ConnectionIdEqual::operator() (const ConnectionId& cid1, const ConnectionId& cid2) const {
+  return cid1.Equals(cid2);
+}
+
+} // namespace rpc
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/connection_id.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/connection_id.h b/be/src/kudu/rpc/connection_id.h
new file mode 100644
index 0000000..ae34b29
--- /dev/null
+++ b/be/src/kudu/rpc/connection_id.h
@@ -0,0 +1,81 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <string>
+
+#include "kudu/rpc/user_credentials.h"
+#include "kudu/util/net/sockaddr.h"
+
+namespace kudu {
+namespace rpc {
+
+// Used to key on Connection information.
+// For use as a key in an unordered STL collection, use ConnectionIdHash and ConnectionIdEqual.
+// This class is copyable for STL compatibility, but not assignable (use CopyFrom() for that).
+class ConnectionId {
+ public:
+  ConnectionId();
+
+  // Copy constructor required for use with STL unordered_map.
+  ConnectionId(const ConnectionId& other);
+
+  // Convenience constructor.
+  ConnectionId(const Sockaddr& remote, UserCredentials user_credentials);
+
+  // The remote address.
+  void set_remote(const Sockaddr& remote);
+  const Sockaddr& remote() const { return remote_; }
+
+  // The credentials of the user associated with this connection, if any.
+  void set_user_credentials(UserCredentials user_credentials);
+  const UserCredentials& user_credentials() const { return user_credentials_; }
+  UserCredentials* mutable_user_credentials() { return &user_credentials_; }
+
+  // Copy state from another object to this one.
+  void CopyFrom(const ConnectionId& other);
+
+  // Returns a string representation of the object, not including the password field.
+  std::string ToString() const;
+
+  size_t HashCode() const;
+  bool Equals(const ConnectionId& other) const;
+
+ private:
+  // Remember to update HashCode() and Equals() when new fields are added.
+  Sockaddr remote_;
+  UserCredentials user_credentials_;
+
+  // Implementation of CopyFrom that can be shared with copy constructor.
+  void DoCopyFrom(const ConnectionId& other);
+
+  // Disable assignment operator.
+  void operator=(const ConnectionId&);
+};
+
+class ConnectionIdHash {
+ public:
+  std::size_t operator() (const ConnectionId& conn_id) const;
+};
+
+class ConnectionIdEqual {
+ public:
+  bool operator() (const ConnectionId& cid1, const ConnectionId& cid2) const;
+};
+
+} // namespace rpc
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/outbound_call.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/outbound_call.cc b/be/src/kudu/rpc/outbound_call.cc
index bcc39c3..8e248d2 100644
--- a/be/src/kudu/rpc/outbound_call.cc
+++ b/be/src/kudu/rpc/outbound_call.cc
@@ -16,7 +16,6 @@
 // under the License.
 
 #include <algorithm>
-#include <boost/functional/hash.hpp>
 #include <gflags/gflags.h>
 #include <memory>
 #include <mutex>
@@ -24,10 +23,11 @@
 #include <unordered_set>
 #include <vector>
 
+#include "kudu/gutil/stringprintf.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/walltime.h"
-#include "kudu/rpc/outbound_call.h"
 #include "kudu/rpc/constants.h"
+#include "kudu/rpc/outbound_call.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/rpc/rpc_introspection.pb.h"
 #include "kudu/rpc/rpc_sidecar.h"
@@ -481,65 +481,6 @@ void OutboundCall::DumpPB(const DumpRunningRpcsRequestPB& req,
 }
 
 ///
-/// ConnectionId
-///
-
-ConnectionId::ConnectionId() {}
-
-ConnectionId::ConnectionId(const ConnectionId& other) {
-  DoCopyFrom(other);
-}
-
-ConnectionId::ConnectionId(const Sockaddr& remote, UserCredentials user_credentials) {
-  remote_ = remote;
-  user_credentials_ = std::move(user_credentials);
-}
-
-void ConnectionId::set_remote(const Sockaddr& remote) {
-  remote_ = remote;
-}
-
-void ConnectionId::set_user_credentials(UserCredentials user_credentials) {
-  user_credentials_ = std::move(user_credentials);
-}
-
-void ConnectionId::CopyFrom(const ConnectionId& other) {
-  DoCopyFrom(other);
-}
-
-string ConnectionId::ToString() const {
-  // Does not print the password.
-  return StringPrintf("{remote=%s, user_credentials=%s}",
-      remote_.ToString().c_str(),
-      user_credentials_.ToString().c_str());
-}
-
-void ConnectionId::DoCopyFrom(const ConnectionId& other) {
-  remote_ = other.remote_;
-  user_credentials_ = other.user_credentials_;
-}
-
-size_t ConnectionId::HashCode() const {
-  size_t seed = 0;
-  boost::hash_combine(seed, remote_.HashCode());
-  boost::hash_combine(seed, user_credentials_.HashCode());
-  return seed;
-}
-
-bool ConnectionId::Equals(const ConnectionId& other) const {
-  return (remote() == other.remote()
-       && user_credentials().Equals(other.user_credentials()));
-}
-
-size_t ConnectionIdHash::operator() (const ConnectionId& conn_id) const {
-  return conn_id.HashCode();
-}
-
-bool ConnectionIdEqual::operator() (const ConnectionId& cid1, const ConnectionId& cid2) const {
-  return cid1.Equals(cid2);
-}
-
-///
 /// CallResponse
 ///
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/outbound_call.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/outbound_call.h b/be/src/kudu/rpc/outbound_call.h
index 221c368..0ab1553 100644
--- a/be/src/kudu/rpc/outbound_call.h
+++ b/be/src/kudu/rpc/outbound_call.h
@@ -25,16 +25,15 @@
 
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/rpc/connection_id.h"
 #include "kudu/rpc/constants.h"
-#include "kudu/rpc/rpc_header.pb.h"
-#include "kudu/rpc/rpc_sidecar.h"
 #include "kudu/rpc/remote_method.h"
 #include "kudu/rpc/response_callback.h"
+#include "kudu/rpc/rpc_header.pb.h"
+#include "kudu/rpc/rpc_sidecar.h"
 #include "kudu/rpc/transfer.h"
-#include "kudu/rpc/user_credentials.h"
 #include "kudu/util/locks.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/net/sockaddr.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 
@@ -57,59 +56,6 @@ class RpcCallInProgressPB;
 class RpcController;
 class RpcSidecar;
 
-// Used to key on Connection information.
-// For use as a key in an unordered STL collection, use ConnectionIdHash and ConnectionIdEqual.
-// This class is copyable for STL compatibility, but not assignable (use CopyFrom() for that).
-class ConnectionId {
- public:
-  ConnectionId();
-
-  // Copy constructor required for use with STL unordered_map.
-  ConnectionId(const ConnectionId& other);
-
-  // Convenience constructor.
-  ConnectionId(const Sockaddr& remote, UserCredentials user_credentials);
-
-  // The remote address.
-  void set_remote(const Sockaddr& remote);
-  const Sockaddr& remote() const { return remote_; }
-
-  // The credentials of the user associated with this connection, if any.
-  void set_user_credentials(UserCredentials user_credentials);
-  const UserCredentials& user_credentials() const { return user_credentials_; }
-  UserCredentials* mutable_user_credentials() { return &user_credentials_; }
-
-  // Copy state from another object to this one.
-  void CopyFrom(const ConnectionId& other);
-
-  // Returns a string representation of the object, not including the password field.
-  std::string ToString() const;
-
-  size_t HashCode() const;
-  bool Equals(const ConnectionId& other) const;
-
- private:
-  // Remember to update HashCode() and Equals() when new fields are added.
-  Sockaddr remote_;
-  UserCredentials user_credentials_;
-
-  // Implementation of CopyFrom that can be shared with copy constructor.
-  void DoCopyFrom(const ConnectionId& other);
-
-  // Disable assignment operator.
-  void operator=(const ConnectionId&);
-};
-
-class ConnectionIdHash {
- public:
-  std::size_t operator() (const ConnectionId& conn_id) const;
-};
-
-class ConnectionIdEqual {
- public:
-  bool operator() (const ConnectionId& cid1, const ConnectionId& cid2) const;
-};
-
 // Tracks the status of a call on the client side.
 //
 // This is an internal-facing class -- clients interact with the

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/proxy.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/proxy.h b/be/src/kudu/rpc/proxy.h
index ddbbe60..6e43e93 100644
--- a/be/src/kudu/rpc/proxy.h
+++ b/be/src/kudu/rpc/proxy.h
@@ -22,7 +22,7 @@
 #include <string>
 
 #include "kudu/gutil/atomicops.h"
-#include "kudu/rpc/outbound_call.h"
+#include "kudu/rpc/connection_id.h"
 #include "kudu/rpc/response_callback.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/rpc/rpc_header.pb.h"

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/reactor.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/reactor.h b/be/src/kudu/rpc/reactor.h
index dcdc8a2..2c1bfc8 100644
--- a/be/src/kudu/rpc/reactor.h
+++ b/be/src/kudu/rpc/reactor.h
@@ -32,6 +32,8 @@
 
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/rpc/connection.h"
+#include "kudu/rpc/connection_id.h"
+#include "kudu/rpc/messenger.h"
 #include "kudu/rpc/transfer.h"
 #include "kudu/util/thread.h"
 #include "kudu/util/locks.h"

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/7d41b96b/be/src/kudu/rpc/rpc_context.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/rpc_context.cc b/be/src/kudu/rpc/rpc_context.cc
index 06fd8c5..d6d872e 100644
--- a/be/src/kudu/rpc/rpc_context.cc
+++ b/be/src/kudu/rpc/rpc_context.cc
@@ -21,7 +21,6 @@
 #include <ostream>
 #include <sstream>
 
-#include "kudu/rpc/outbound_call.h"
 #include "kudu/rpc/inbound_call.h"
 #include "kudu/rpc/remote_user.h"
 #include "kudu/rpc/result_tracker.h"