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();