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

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

commit 9b03c9c163dbd489d858169cd7386e00359a3c73
Author: Alan M. Carroll <[email protected]>
AuthorDate: Fri Jul 20 08:37:13 2018 -0500

    Outbound server session management - Replace TSHashTable with 
IntrusiveHashMap.
---
 proxy/http/HttpServerSession.h   | 111 ++++++++++++++++++++++++++++++++++++---
 proxy/http/HttpSessionManager.cc |  74 +++++++++++++++-----------
 proxy/http/HttpSessionManager.h  |  58 ++------------------
 3 files changed, 152 insertions(+), 91 deletions(-)

diff --git a/proxy/http/HttpServerSession.h b/proxy/http/HttpServerSession.h
index 491e2ca..5e4224a 100644
--- a/proxy/http/HttpServerSession.h
+++ b/proxy/http/HttpServerSession.h
@@ -66,10 +66,13 @@ enum {
 
 class HttpServerSession : public VConnection
 {
+  using self_type  = HttpServerSession;
   using super_type = VConnection;
 
 public:
   HttpServerSession() : super_type(nullptr) {}
+  HttpServerSession(self_type const &) = delete;
+  self_type &operator=(self_type const &) = delete;
 
   void destroy();
   void new_connection(NetVConnection *new_vc);
@@ -152,12 +155,36 @@ public:
   bool private_session = false;
 
   // Copy of the owning SM's server session sharing settings
-  TSServerSessionSharingMatchType 
sharing_match{TS_SERVER_SESSION_SHARING_MATCH_BOTH};
-  TSServerSessionSharingPoolType 
sharing_pool{TS_SERVER_SESSION_SHARING_POOL_GLOBAL};
+  TSServerSessionSharingMatchType sharing_match = 
TS_SERVER_SESSION_SHARING_MATCH_BOTH;
+  TSServerSessionSharingPoolType sharing_pool   = 
TS_SERVER_SESSION_SHARING_POOL_GLOBAL;
   //  int share_session;
 
-  LINK(HttpServerSession, ip_hash_link);
-  LINK(HttpServerSession, host_hash_link);
+  /// Hash map descriptor class for IP map.
+  struct IPLinkage {
+    self_type *_next = nullptr;
+    self_type *_prev = nullptr;
+
+    static self_type *&next_ptr(self_type *);
+    static self_type *&prev_ptr(self_type *);
+    static uint32_t hash_of(sockaddr const *key);
+    static sockaddr const *key_of(self_type const *ssn);
+    static bool equal(sockaddr const *lhs, sockaddr const *rhs);
+    // Add a couple overloads for internal convenience.
+    static bool equal(sockaddr const *lhs, HttpServerSession const *rhs);
+    static bool equal(HttpServerSession const *lhs, sockaddr const *rhs);
+  } _ip_link;
+
+  /// Hash map descriptor class for FQDN map.
+  struct FQDNLinkage {
+    self_type *_next = nullptr;
+    self_type *_prev = nullptr;
+
+    static self_type *&next_ptr(self_type *);
+    static self_type *&prev_ptr(self_type *);
+    static uint64_t hash_of(CryptoHash const &key);
+    static CryptoHash const &key_of(self_type *ssn);
+    static bool equal(CryptoHash const &lhs, CryptoHash const &rhs);
+  } _fqdn_link;
 
   // Keep track of connection limiting and a pointer to the
   // singleton that keeps track of the connection counts.
@@ -187,8 +214,6 @@ public:
   }
 
 private:
-  HttpServerSession(HttpServerSession &);
-
   NetVConnection *server_vc = nullptr;
   int magic                 = HTTP_SS_MAGIC_DEAD;
 
@@ -197,6 +222,8 @@ private:
 
 extern ClassAllocator<HttpServerSession> httpServerSessionAllocator;
 
+// --- Implementation ---
+
 inline void
 HttpServerSession::attach_hostname(const char *hostname)
 {
@@ -204,3 +231,75 @@ HttpServerSession::attach_hostname(const char *hostname)
     CryptoContext().hash_immediate(hostname_hash, (unsigned char *)hostname, 
strlen(hostname));
   }
 }
+
+inline HttpServerSession *&
+HttpServerSession::IPLinkage::next_ptr(self_type *ssn)
+{
+  return ssn->_ip_link._next;
+}
+
+inline HttpServerSession *&
+HttpServerSession::IPLinkage::prev_ptr(self_type *ssn)
+{
+  return ssn->_ip_link._prev;
+}
+
+inline uint32_t
+HttpServerSession::IPLinkage::hash_of(sockaddr const *key)
+{
+  return ats_ip_hash(key);
+}
+
+inline sockaddr const *
+HttpServerSession::IPLinkage::key_of(self_type const *ssn)
+{
+  return &ssn->get_server_ip().sa;
+}
+
+inline bool
+HttpServerSession::IPLinkage::equal(sockaddr const *lhs, sockaddr const *rhs)
+{
+  return ats_ip_addr_port_eq(lhs, rhs);
+}
+
+inline bool
+HttpServerSession::IPLinkage::equal(sockaddr const *lhs, HttpServerSession 
const *rhs)
+{
+  return ats_ip_addr_port_eq(lhs, key_of(rhs));
+}
+
+inline bool
+HttpServerSession::IPLinkage::equal(HttpServerSession const *lhs, sockaddr 
const *rhs)
+{
+  return ats_ip_addr_port_eq(key_of(lhs), rhs);
+}
+
+inline HttpServerSession *&
+HttpServerSession::FQDNLinkage::next_ptr(self_type *ssn)
+{
+  return ssn->_fqdn_link._next;
+}
+
+inline HttpServerSession *&
+HttpServerSession::FQDNLinkage::prev_ptr(self_type *ssn)
+{
+  return ssn->_fqdn_link._prev;
+}
+
+inline uint64_t
+HttpServerSession::FQDNLinkage::hash_of(CryptoHash const &key)
+{
+  return key.fold();
+}
+
+inline CryptoHash const &
+HttpServerSession::FQDNLinkage::key_of(self_type *ssn)
+{
+  return ssn->hostname_hash;
+}
+
+inline bool
+HttpServerSession::FQDNLinkage::equal(CryptoHash const &lhs, CryptoHash const 
&rhs)
+{
+  return lhs == rhs;
+}
diff --git a/proxy/http/HttpSessionManager.cc b/proxy/http/HttpSessionManager.cc
index 07f7c41..8831a45 100644
--- a/proxy/http/HttpSessionManager.cc
+++ b/proxy/http/HttpSessionManager.cc
@@ -45,11 +45,11 @@ initialize_thread_for_http_sessions(EThread *thread)
 
 HttpSessionManager httpSessionManager;
 
-ServerSessionPool::ServerSessionPool() : Continuation(new_ProxyMutex()), 
m_ip_pool(1023), m_host_pool(1023)
+ServerSessionPool::ServerSessionPool() : Continuation(new_ProxyMutex()), 
m_ip_pool(1023), m_fqdn_pool(1023)
 {
   SET_HANDLER(&ServerSessionPool::eventHandler);
-  m_ip_pool.setExpansionPolicy(IPHashTable::MANUAL);
-  m_host_pool.setExpansionPolicy(HostHashTable::MANUAL);
+  m_ip_pool.set_expansion_policy(IPTable::MANUAL);
+  m_fqdn_pool.set_expansion_policy(FQDNTable::MANUAL);
 }
 
 void
@@ -57,10 +57,9 @@ ServerSessionPool::purge()
 {
   // @c do_io_close can free the instance which clears the intrusive links and 
breaks the iterator.
   // Therefore @c do_io_close is called on a post-incremented iterator.
-  for (IPHashTable::iterator last = m_ip_pool.end(), spot = m_ip_pool.begin(); 
spot != last; spot++->do_io_close()) {
-  }
+  m_ip_pool.apply([](HttpServerSession *ssn) -> void { ssn->do_io_close(); });
   m_ip_pool.clear();
-  m_host_pool.clear();
+  m_fqdn_pool.clear();
 }
 
 bool
@@ -97,38 +96,48 @@ ServerSessionPool::acquireSession(sockaddr const *addr, 
CryptoHash const &hostna
                                   TSServerSessionSharingMatchType match_style, 
HttpSM *sm, HttpServerSession *&to_return)
 {
   HSMresult_t zret = HSM_NOT_FOUND;
+  to_return        = nullptr;
+
   if (TS_SERVER_SESSION_SHARING_MATCH_HOST == match_style) {
-    // This is broken out because only in this case do we check the host hash 
first.
-    HostHashTable::Location loc = m_host_pool.find(hostname_hash);
-    in_port_t port              = ats_ip_port_cast(addr);
-    while (loc) { // scan for matching port.
-      if (port == ats_ip_port_cast(loc->get_server_ip()) && validate_sni(sm, 
loc->get_netvc())) {
+    // This is broken out because only in this case do we check the host hash 
first. The range must be checked
+    // to verify an upstream that matches port and SNI name is selected. Walk 
backwards to select oldest.
+    in_port_t port = ats_ip_port_cast(addr);
+    FQDNTable::iterator first, last;
+    // FreeBSD/clang++ bug workaround: explicit cast to super type to make 
overload work. Not needed on Fedora27 nor gcc.
+    // Not fixed on FreeBSD as of llvm 6.0.1.
+    std::tie(first, last) = static_cast<const 
decltype(m_fqdn_pool)::range::super_type 
&>(m_fqdn_pool.equal_range(hostname_hash));
+    while (last != first) {
+      --last;
+      if (port == ats_ip_port_cast(last->get_server_ip()) && validate_sni(sm, 
last->get_netvc())) {
+        zret = HSM_DONE;
         break;
       }
-      ++loc;
     }
-    if (loc) {
-      to_return = loc;
-      m_host_pool.remove(loc);
-      m_ip_pool.remove(m_ip_pool.find(loc));
+    if (zret == HSM_DONE) {
+      to_return = last;
+      m_fqdn_pool.erase(last);
+      m_ip_pool.erase(to_return);
     }
   } else if (TS_SERVER_SESSION_SHARING_MATCH_NONE != match_style) { // 
matching is not disabled.
-    IPHashTable::Location loc = m_ip_pool.find(addr);
-    // If we're matching on the IP address we're done, this one is good enough.
-    // Otherwise we need to scan further matches to match the host name as 
well.
-    // Note we don't have to check the port because it's checked as part of 
the IP address key.
+    IPTable::iterator first, last;
+    // FreeBSD/clang++ bug workaround: explicit cast to super type to make 
overload work. Not needed on Fedora27 nor gcc.
+    // Not fixed on FreeBSD as of llvm 6.0.1.
+    std::tie(first, last) = static_cast<const 
decltype(m_ip_pool)::range::super_type &>(m_ip_pool.equal_range(addr));
+    // The range is all that is needed in the match IP case, otherwise need to 
scan for matching fqdn.
+    // Note the port is matched as part of the address key so it doesn't need 
to be checked again.
     if (TS_SERVER_SESSION_SHARING_MATCH_IP != match_style) {
-      while (loc) {
-        if (loc->hostname_hash == hostname_hash && validate_sni(sm, 
loc->get_netvc())) {
+      while (last != first) {
+        --last;
+        if (last->hostname_hash == hostname_hash && validate_sni(sm, 
last->get_netvc())) {
+          zret = HSM_DONE;
           break;
         }
-        ++loc;
       }
     }
-    if (loc) {
-      to_return = loc;
-      m_ip_pool.remove(loc);
-      m_host_pool.remove(m_host_pool.find(loc));
+    if (zret == HSM_DONE) {
+      to_return = last;
+      m_ip_pool.erase(last);
+      m_fqdn_pool.erase(to_return);
     }
   }
   return zret;
@@ -152,7 +161,7 @@ ServerSessionPool::releaseSession(HttpServerSession *ss)
   ss->get_netvc()->set_active_timeout(ss->get_netvc()->get_active_timeout());
   // put it in the pools.
   m_ip_pool.insert(ss);
-  m_host_pool.insert(ss);
+  m_fqdn_pool.insert(ss);
 
   Debug("http_ss",
         "[%" PRId64 "] [release session] "
@@ -189,9 +198,10 @@ ServerSessionPool::eventHandler(int event, void *data)
   sockaddr const *addr                 = net_vc->get_remote_addr();
   HttpConfigParams *http_config_params = HttpConfig::acquire();
   bool found                           = false;
+  auto spot                            = m_ip_pool.find(addr);
 
-  for (ServerSessionPool::IPHashTable::Location lh = m_ip_pool.find(addr); lh; 
++lh) {
-    if ((s = lh)->get_netvc() == net_vc) {
+  while (spot != m_ip_pool.end() && spot->_ip_link.equal(addr, spot)) {
+    if ((s = spot)->get_netvc() == net_vc) {
       // if there was a timeout of some kind on a keep alive connection, and
       // keeping the connection alive will not keep us above the # of max 
connections
       // to the origin and we are below the min number of keep alive 
connections to this
@@ -218,8 +228,8 @@ ServerSessionPool::eventHandler(int event, void *data)
             HttpDebugNames::get_event_name(event));
       ink_assert(s->state == HSS_KA_SHARED);
       // Out of the pool! Now!
-      m_ip_pool.remove(lh);
-      m_host_pool.remove(m_host_pool.find(s));
+      m_ip_pool.erase(spot);
+      m_fqdn_pool.erase(s);
       // Drop connection on this end.
       s->do_io_close();
       found = true;
diff --git a/proxy/http/HttpSessionManager.h b/proxy/http/HttpSessionManager.h
index 66998a7..0c0f297 100644
--- a/proxy/http/HttpSessionManager.h
+++ b/proxy/http/HttpSessionManager.h
@@ -34,7 +34,7 @@
 
 #include "P_EventSystem.h"
 #include "HttpServerSession.h"
-#include <ts/Map.h>
+#include <ts/IntrusiveHashMap.h>
 
 class ProxyClientTransaction;
 class HttpSM;
@@ -67,56 +67,8 @@ public:
   static bool validate_sni(HttpSM *sm, NetVConnection *netvc);
 
 protected:
-  /// Interface class for IP map.
-  struct IPHashing {
-    typedef uint32_t ID;
-    typedef sockaddr const *Key;
-    typedef HttpServerSession Value;
-    typedef DList(HttpServerSession, ip_hash_link) ListHead;
-
-    static ID
-    hash(Key key)
-    {
-      return ats_ip_hash(key);
-    }
-    static Key
-    key(Value const *value)
-    {
-      return &value->get_server_ip().sa;
-    }
-    static bool
-    equal(Key lhs, Key rhs)
-    {
-      return ats_ip_addr_port_eq(lhs, rhs);
-    }
-  };
-
-  /// Interface class for FQDN map.
-  struct HostHashing {
-    typedef uint64_t ID;
-    typedef CryptoHash const &Key;
-    typedef HttpServerSession Value;
-    typedef DList(HttpServerSession, host_hash_link) ListHead;
-
-    static ID
-    hash(Key key)
-    {
-      return key.fold();
-    }
-    static Key
-    key(Value const *value)
-    {
-      return value->hostname_hash;
-    }
-    static bool
-    equal(Key lhs, Key rhs)
-    {
-      return lhs == rhs;
-    }
-  };
-
-  typedef TSHashTable<IPHashing> IPHashTable;     ///< Sessions by IP address.
-  typedef TSHashTable<HostHashing> HostHashTable; ///< Sessions by host name.
+  using IPTable   = IntrusiveHashMap<HttpServerSession::IPLinkage>;
+  using FQDNTable = IntrusiveHashMap<HttpServerSession::FQDNLinkage>;
 
 public:
   /** Check if a session matches address and host name.
@@ -142,8 +94,8 @@ public:
 
   // Pools of server sessions.
   // Note that each server session is stored in both pools.
-  IPHashTable m_ip_pool;
-  HostHashTable m_host_pool;
+  IPTable m_ip_pool;
+  FQDNTable m_fqdn_pool;
 };
 
 class HttpSessionManager

Reply via email to