This removes the domain from the server pconn key.

Under the squid-3.2 pconn model the IP:port specifying the destination are part of the key and can be used to strictly filter selection when locating pconn. This means the domain is no longer a necessary part of the key.

Squid using cache_peer can see a large number of wasted idle connections to their peers due to the key domain value if the peer hostname is not substituted properly. There is also a similar affect when contacting servers with virtual hosted domains.

A simpler form of this with just the functional change in key generation has been tested for several months now with only socket usage benefits seen in a few production networks.

Amos

=== modified file 'src/forward.cc'
--- src/forward.cc      2011-11-10 21:49:07 +0000
+++ src/forward.cc      2011-11-17 10:24:01 +0000
@@ -849,13 +849,7 @@
     }
 
     // Use pconn to avoid opening a new connection.
-    const char *host;
-    if (serverDestinations[0]->getPeer()) {
-        host = serverDestinations[0]->getPeer()->host;
-    } else {
-        host = request->GetHost();
-    }
-    Comm::ConnectionPointer temp = fwdPconnPool->pop(serverDestinations[0], 
host, checkRetriable());
+    Comm::ConnectionPointer temp = fwdPconnPool->pop(serverDestinations[0], 
checkRetriable());
 
     // if we found an open persistent connection to use. use it.
     if (temp != NULL && Comm::IsConnOpen(temp)) {
@@ -903,7 +897,15 @@
 
     calls.connector = commCbCall(17,3, "fwdConnectDoneWrapper", 
CommConnectCbPtrFun(fwdConnectDoneWrapper, this));
     Comm::ConnOpener *cs = new Comm::ConnOpener(serverDestinations[0], 
calls.connector, ctimeout);
+
+    const char *host;
+    if (serverDestinations[0]->getPeer()) {
+        host = serverDestinations[0]->getPeer()->host;
+    } else {
+        host = request->GetHost();
+    }
     cs->setHost(host);
+
     AsyncJob::Start(cs);
 }
 
@@ -1149,13 +1151,9 @@
  *  -  domain name of server at other end of this link (either peer or 
requested host)
  */
 void
-FwdState::pconnPush(Comm::ConnectionPointer &conn, const char *domain)
+FwdState::pconnPush(Comm::ConnectionPointer &conn)
 {
-    if (conn->getPeer()) {
-        fwdPconnPool->push(conn, conn->getPeer()->name);
-    } else {
-        fwdPconnPool->push(conn, domain);
-    }
+    fwdPconnPool->push(conn);
 }
 
 void

=== modified file 'src/forward.h'
--- src/forward.h       2011-08-20 15:57:06 +0000
+++ src/forward.h       2011-11-17 10:24:01 +0000
@@ -55,7 +55,7 @@
     bool checkRetry();
     bool checkRetriable();
     void dispatch();
-    void pconnPush(Comm::ConnectionPointer & conn, const char *domain);
+    void pconnPush(Comm::ConnectionPointer & conn);
 
     bool dontRetry() { return flags.dont_retry; }
 

=== modified file 'src/http.cc'
--- src/http.cc 2011-10-21 16:20:42 +0000
+++ src/http.cc 2011-11-17 10:24:01 +0000
@@ -1405,7 +1405,7 @@
                 request->pinnedConnection()->pinConnection(serverConnection, 
request, _peer,
                         (request->flags.connection_auth != 0));
             } else {
-                fwd->pconnPush(serverConnection, request->peer_host ? 
request->peer_host : request->GetHost());
+                fwd->pconnPush(serverConnection);
             }
 
             serverConnection = NULL;

=== modified file 'src/pconn.cc'
--- src/pconn.cc        2011-11-08 13:40:26 +0000
+++ src/pconn.cc        2011-11-17 10:24:01 +0000
@@ -324,17 +324,13 @@
 /* ========== PconnPool PRIVATE FUNCTIONS 
============================================ */
 
 const char *
-PconnPool::key(const Comm::ConnectionPointer &destLink, const char *domain)
+PconnPool::key(const Comm::ConnectionPointer &destLink)
 {
     LOCAL_ARRAY(char, buf, SQUIDHOSTNAMELEN * 3 + 10);
 
     destLink->remote.ToURL(buf, SQUIDHOSTNAMELEN * 3 + 10);
-    if (domain) {
-        const int used = strlen(buf);
-        snprintf(buf+used, SQUIDHOSTNAMELEN * 3 + 10-used, "/%s", domain);
-    }
 
-    debugs(48,6,"PconnPool::key(" << destLink << ", " << (domain?domain:"[no 
domain]") << ") is {" << buf << "}" );
+    debugs(48,6,"PconnPool::key(" << destLink << ") is {" << buf << "}" );
     return buf;
 }
 
@@ -390,7 +386,7 @@
 }
 
 void
-PconnPool::push(const Comm::ConnectionPointer &conn, const char *domain)
+PconnPool::push(const Comm::ConnectionPointer &conn)
 {
     if (fdUsageHigh()) {
         debugs(48, 3, HERE << "Not many unused FDs");
@@ -402,7 +398,7 @@
         return;
     }
 
-    const char *aKey = key(conn, domain);
+    const char *aKey = key(conn);
     IdleConnList *list = (IdleConnList *) hash_lookup(table, aKey);
 
     if (list == NULL) {
@@ -423,9 +419,9 @@
 }
 
 Comm::ConnectionPointer
-PconnPool::pop(const Comm::ConnectionPointer &destLink, const char *domain, 
bool isRetriable)
+PconnPool::pop(const Comm::ConnectionPointer &destLink, bool isRetriable)
 {
-    const char * aKey = key(destLink, domain);
+    const char * aKey = key(destLink);
 
     IdleConnList *list = (IdleConnList *)hash_lookup(table, aKey);
     if (list == NULL) {
@@ -444,11 +440,11 @@
 }
 
 void
-PconnPool::closeN(int n, const Comm::ConnectionPointer &destLink, const char 
*domain)
+PconnPool::closeN(int n, const Comm::ConnectionPointer &destLink)
 {
     // TODO: optimize: we can probably do hash_lookup just once
     for (int i = 0; i < n; ++i)
-        pop(destLink, domain, false); // may fail!
+        pop(destLink, false); // may fail!
 }
 
 void

=== modified file 'src/pconn.h'
--- src/pconn.h 2011-09-06 08:24:09 +0000
+++ src/pconn.h 2011-11-17 10:24:01 +0000
@@ -114,7 +114,7 @@
     ~PconnPool();
 
     void moduleInit();
-    void push(const Comm::ConnectionPointer &serverConn, const char *domain);
+    void push(const Comm::ConnectionPointer &serverConn);
 
     /**
      * Updates destLink to point at an existing open connection if available 
and retriable.
@@ -124,20 +124,20 @@
      * retriable to avoid having a growing number of open connections when many
      * transactions create persistent connections but are not retriable.
      */
-    Comm::ConnectionPointer pop(const Comm::ConnectionPointer &destLink, const 
char *domain, bool retriable);
+    Comm::ConnectionPointer pop(const Comm::ConnectionPointer &destLink, bool 
retriable);
     void count(int uses);
     void dumpHist(StoreEntry *e) const;
     void dumpHash(StoreEntry *e) const;
     void unlinkList(IdleConnList *list);
     void noteUses(int uses);
-    void closeN(int n, const Comm::ConnectionPointer &destLink, const char 
*domain);
+    void closeN(int n, const Comm::ConnectionPointer &destLink);
     int count() const { return theCount; }
     void noteConnectionAdded() { ++theCount; }
     void noteConnectionRemoved() { assert(theCount > 0); --theCount; }
 
 private:
 
-    static const char *key(const Comm::ConnectionPointer &destLink, const char 
*domain);
+    static const char *key(const Comm::ConnectionPointer &destLink);
 
     int hist[PCONN_HIST_SZ];
     hash_table *table;

=== modified file 'src/tests/stub_pconn.cc'
--- src/tests/stub_pconn.cc     2011-06-04 12:48:45 +0000
+++ src/tests/stub_pconn.cc     2011-11-17 10:24:02 +0000
@@ -52,13 +52,13 @@
 }
 
 void
-PconnPool::push(const Comm::ConnectionPointer &serverConn, const char *domain)
+PconnPool::push(const Comm::ConnectionPointer &serverConn)
 {
     fatal("pconn.cc required");
 }
 
 Comm::ConnectionPointer
-PconnPool::pop(const Comm::ConnectionPointer &destLink, const char *domain, 
bool retriable)
+PconnPool::pop(const Comm::ConnectionPointer &destLink, bool retriable)
 {
     fatal("pconn.cc required");
     return Comm::ConnectionPointer();

Reply via email to