]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Bug 3753: Removes the domain from the cache_peer server pconn key
authorAmos Jeffries <squid3@treenet.co.nz>
Fri, 1 Mar 2013 10:58:07 +0000 (03:58 -0700)
committerAmos Jeffries <squid3@treenet.co.nz>
Fri, 1 Mar 2013 10:58:07 +0000 (03:58 -0700)
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.

Also bug 3753 was located with peer host and name= values being used
inconsistently as the domain marker. Resulting in failed pop()
operations and extra FD usage.

This has been tested for several months now with only socket usage
benefits seen in several production networks.

NOTE: previous experience some years back with pconn has demonstrated
several broken web servers which assume all requests on a persistent
connection are for the same virtual host. For now this change avoids
altering the behaviour on DIRECT traffic for this reason.

This was sponsored by Treehouse Networks Ltd.

src/forward.cc
src/http.cc

index 91a172bc9ae628236b335c6394c9ba8041ea5c01..6c1e9975eba12a9091e53390c7fd7664529a84ab 100644 (file)
@@ -919,12 +919,9 @@ FwdState::connectStart()
     }
 
     // Use pconn to avoid opening a new connection.
-    const char *host;
-    if (serverDestinations[0]->getPeer()) {
-        host = serverDestinations[0]->getPeer()->host;
-    } else {
+    const char *host = NULL;
+    if (!serverDestinations[0]->getPeer())
         host = request->GetHost();
-    }
 
     Comm::ConnectionPointer temp;
     // Avoid pconns after races so that the same client does not suffer twice.
@@ -989,7 +986,8 @@ FwdState::connectStart()
 
     calls.connector = commCbCall(17,3, "fwdConnectDoneWrapper", CommConnectCbPtrFun(fwdConnectDoneWrapper, this));
     Comm::ConnOpener *cs = new Comm::ConnOpener(serverDestinations[0], calls.connector, ctimeout);
-    cs->setHost(host);
+    if (host)
+        cs->setHost(host);
     AsyncJob::Start(cs);
 }
 
@@ -1229,7 +1227,7 @@ void
 FwdState::pconnPush(Comm::ConnectionPointer &conn, const char *domain)
 {
     if (conn->getPeer()) {
-        fwdPconnPool->push(conn, conn->getPeer()->name);
+        fwdPconnPool->push(conn, NULL);
     } else {
         fwdPconnPool->push(conn, domain);
     }
index 6e72b1248a419bbf69a44aced429cedf19726ad6..ae2348735661ae0f22c36132f4773f776fffbaf3 100644 (file)
@@ -1449,7 +1449,7 @@ HttpStateData::processReplyBody()
                 request->clientConnectionManager->pinConnection(serverConnection, request, _peer,
                         (request->flags.connection_auth != 0));
             } else {
-                fwd->pconnPush(serverConnection, request->peer_host ? request->peer_host : request->GetHost());
+                fwd->pconnPush(serverConnection, request->GetHost());
             }
 
             serverConnection = NULL;