Implements the pconn and IdleConnList as storage lists of
Comm::Connection.
With Comm::Connection the key definition has to change. The old
keying based solely on domain would produce a Connection object whose FD
did not exactly match the remote IP:port selected by the new routing logics.
I've kept the concept that the pconn key is the details to describe
the remote end of the link used to fetch any given domain. As such its
become much simpler now. Just the remote IP:port from the TCP details
and requested domain (post re-writing) or peer hostname for peer pconn.
The change also removes the client address from relevance (enhancement
bug fix). Instead we scan the list of pconn going to our desired
remote-end for the local-end details (tcp_outgoing_addr or tproxy IPs)
to pick an exact pconn match (bug fix).
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.8
Beta testers wanted for 3.2.0.2
=== modified file 'src/pconn.cc'
--- src/pconn.cc 2010-04-17 02:29:04 +0000
+++ src/pconn.cc 2010-10-02 16:37:57 +0000
@@ -34,98 +34,104 @@
#include "squid.h"
#include "CacheManager.h"
-#include "Store.h"
#include "comm.h"
+#include "comm/Connection.h"
+#include "fde.h"
#include "pconn.h"
-#include "fde.h"
+#include "Store.h"
#define PCONN_FDS_SZ 8 /* pconn set size, increase for better memcache hit rate */
-static MemAllocator *pconn_fds_pool = NULL;
+//TODO: re-attach to MemPools. WAS: static MemAllocator *pconn_fds_pool = NULL;
PconnModule * PconnModule::instance = NULL;
CBDATA_CLASS_INIT(IdleConnList);
/* ========== IdleConnList ============================================ */
-IdleConnList::IdleConnList(const char *key, PconnPool *thePool) : parent(thePool)
+IdleConnList::IdleConnList(const char *key, PconnPool *thePool) :
+ nfds_alloc(PCONN_FDS_SZ),
+ nfds(0),
+ parent(thePool)
{
hash.key = xstrdup(key);
- nfds_alloc = PCONN_FDS_SZ;
- nfds = 0;
- fds = (int *)pconn_fds_pool->alloc();
+ theList = new Comm::ConnectionPointer[nfds_alloc];
+// TODO: re-attach to MemPools. WAS: fds = (int *)pconn_fds_pool->alloc();
}
IdleConnList::~IdleConnList()
{
-
parent->unlinkList(this);
+/* TODO: re-attach to MemPools.
if (nfds_alloc == PCONN_FDS_SZ)
- pconn_fds_pool->freeOne(fds);
+ pconn_fds_pool->freeOne(theList);
else
- xfree(fds);
+*/
+ delete[] theList;
xfree(hash.key);
}
int
-IdleConnList::findFDIndex (int fd)
+IdleConnList::findIndex(const Comm::ConnectionPointer &conn)
{
- int index;
-
- for (index = nfds - 1; index >= 0; --index) {
- if (fds[index] == fd)
+ for (int index = nfds - 1; index >= 0; --index) {
+ if (conn->fd == theList[index]->fd)
return index;
}
return -1;
}
-void
-IdleConnList::removeFD(int fd)
+bool
+IdleConnList::remove(const Comm::ConnectionPointer &conn)
{
- int index = findFDIndex(fd);
+ int index = findIndex(conn);
if (index < 0) {
- debugs(48, 2, "IdleConnList::removeFD: FD " << fd << " NOT FOUND!");
- return;
+ debugs(48, 2, HERE << conn << " NOT FOUND!");
+ return false;
}
- debugs(48, 3, "IdleConnList::removeFD: found FD " << fd << " at index " << index);
+ debugs(48, 3, HERE << "found " << conn << " at index " << index);
for (; index < nfds - 1; index++)
- fds[index] = fds[index + 1];
+ theList[index] = theList[index + 1];
if (--nfds == 0) {
debugs(48, 3, "IdleConnList::removeFD: deleting " << hashKeyStr(&hash));
delete this;
}
+ return true;
}
void
-IdleConnList::clearHandlers(int fd)
+IdleConnList::clearHandlers(const Comm::ConnectionPointer &conn)
{
- comm_read_cancel(fd, IdleConnList::read, this);
- commSetTimeout(fd, -1, NULL, NULL);
+ comm_read_cancel(conn->fd, IdleConnList::read, this);
+ commSetTimeout(conn->fd, -1, NULL, NULL);
}
void
-IdleConnList::push(int fd)
+IdleConnList::push(const Comm::ConnectionPointer &conn)
{
if (nfds == nfds_alloc) {
debugs(48, 3, "IdleConnList::push: growing FD array");
nfds_alloc <<= 1;
- int *old = fds;
- fds = (int *)xmalloc(nfds_alloc * sizeof(int));
- xmemcpy(fds, old, nfds * sizeof(int));
+ const Comm::ConnectionPointer *oldList = theList;
+ theList = new Comm::ConnectionPointer[nfds_alloc];
+ for (int index = 0; index < nfds; index++)
+ theList[index] = oldList[index];
+/* TODO: re-attach to MemPools.
if (nfds == PCONN_FDS_SZ)
- pconn_fds_pool->freeOne(old);
+ pconn_fds_pool->freeOne(oldList);
else
- xfree(old);
+*/
+ delete[] oldList;
}
- fds[nfds++] = fd;
- comm_read(fd, fakeReadBuf, sizeof(fakeReadBuf), IdleConnList::read, this);
- commSetTimeout(fd, Config.Timeout.pconn, IdleConnList::timeout, this);
+ theList[nfds++] = conn;
+ comm_read(conn, fakeReadBuf, sizeof(fakeReadBuf), IdleConnList::read, this);
+ commSetTimeout(conn->fd, Config.Timeout.pconn, IdleConnList::timeout, this);
}
/*
@@ -136,24 +142,36 @@
* of requests JUST as they timeout (say, it shuts down) we'll be wasting
* quite a bit of CPU. Just keep it in mind.
*/
-int
-IdleConnList::findUseableFD()
+Comm::ConnectionPointer
+IdleConnList::findUseable(const Comm::ConnectionPointer &key)
{
assert(nfds);
for (int i=nfds-1; i>=0; i--) {
- if (!comm_has_pending_read_callback(fds[i])) {
- return fds[i];
- }
+
+ // callback pending indicates that remote end of the conn has just closed.
+ if (comm_has_pending_read_callback(theList[i]->fd))
+ continue;
+
+ // local end port is required, but dont match.
+ if (key->local.GetPort() > 0 && key->local.GetPort() != theList[i]->local.GetPort())
+ continue;
+
+ // local address is required, but does not match.
+ if (!key->local.IsAnyAddr() && key->local.matchIPAddr(theList[i]->local) != 0)
+ continue;
+
+ // finally, a match
+ return theList[i];
}
- return -1;
+ return key;
}
void
-IdleConnList::read(int fd, char *buf, size_t len, comm_err_t flag, int xerrno, void *data)
+IdleConnList::read(const Comm::ConnectionPointer &conn, char *buf, size_t len, comm_err_t flag, int xerrno, void *data)
{
- debugs(48, 3, "IdleConnList::read: " << len << " bytes from FD " << fd);
+ debugs(48, 3, HERE << len << " bytes from " << conn);
if (flag == COMM_ERR_CLOSING) {
/* Bail out early on COMM_ERR_CLOSING - close handlers will tidy up for us */
@@ -161,8 +179,11 @@
}
IdleConnList *list = (IdleConnList *) data;
- list->removeFD(fd); /* might delete list */
- comm_close(fd);
+ /* might delete list */
+ if (list && list->remove(conn)) {
+ Comm::ConnectionPointer nonConst = conn;
+ nonConst->close();
+ }
}
void
@@ -170,35 +191,34 @@
{
debugs(48, 3, "IdleConnList::timeout: FD " << fd);
IdleConnList *list = (IdleConnList *) data;
- list->removeFD(fd); /* might delete list */
- comm_close(fd);
+ Comm::ConnectionPointer temp = new Comm::Connection; // XXX: transition. make timeouts pass conn in
+ temp->fd = fd;
+ if (list->remove(temp)) {
+ temp->close();
+ } else
+ temp->fd = -1; // XXX: transition. prevent temp erasure double-closing FD until timeout CB passess conn in.
}
/* ========== PconnPool PRIVATE FUNCTIONS ============================================ */
const char *
-PconnPool::key(const char *host, u_short port, const char *domain, Ip::Address &client_address)
+PconnPool::key(const Comm::ConnectionPointer &destLink, const char *domain)
{
LOCAL_ARRAY(char, buf, SQUIDHOSTNAMELEN * 3 + 10);
- char ntoabuf[MAX_IPSTRLEN];
-
- if (domain && !client_address.IsAnyAddr())
- snprintf(buf, SQUIDHOSTNAMELEN * 3 + 10, "%s:%d-%s/%s", host, (int) port, client_address.NtoA(ntoabuf,MAX_IPSTRLEN), domain);
- else if (domain && client_address.IsAnyAddr())
- snprintf(buf, SQUIDHOSTNAMELEN * 3 + 10, "%s:%d/%s", host, (int) port, domain);
- else if ((!domain) && !client_address.IsAnyAddr())
- snprintf(buf, SQUIDHOSTNAMELEN * 3 + 10, "%s:%d-%s", host, (int) port, client_address.NtoA(ntoabuf,MAX_IPSTRLEN));
- else
- snprintf(buf, SQUIDHOSTNAMELEN * 3 + 10, "%s:%d", host, (int) port);
-
- debugs(48,6,"PconnPool::key(" << (host?host:"(no host!)") << "," << port << "," << (domain?domain:"(no domain)") << "," << client_address << "is {" << buf << "}" );
+
+ destLink->remote.ToURL(buf, SQUIDHOSTNAMELEN * 3 + 10);
+ if (domain) {
+ 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 << "}" );
return buf;
}
void
-PconnPool::dumpHist(StoreEntry * e)
+PconnPool::dumpHist(StoreEntry * e) const
{
- int i;
storeAppendPrintf(e,
"%s persistent connection counts:\n"
"\n"
@@ -207,7 +227,7 @@
"\t---- ---------\n",
descr);
- for (i = 0; i < PCONN_HIST_SZ; i++) {
+ for (int i = 0; i < PCONN_HIST_SZ; i++) {
if (hist[i] == 0)
continue;
@@ -216,14 +236,13 @@
}
void
-PconnPool::dumpHash(StoreEntry *e)
+PconnPool::dumpHash(StoreEntry *e) const
{
- int i;
- hash_link *walker = NULL;
hash_table *hid = table;
hash_first(hid);
- for (i = 0, walker = hid->next; walker; walker = hash_next(hid)) {
+ int i = 0;
+ for (hash_link *walker = hid->next; walker; walker = hash_next(hid)) {
storeAppendPrintf(e, "\t item %5d: %s\n", i++, (char *)(walker->key));
}
}
@@ -232,10 +251,9 @@
PconnPool::PconnPool(const char *aDescr) : table(NULL), descr(aDescr)
{
- int i;
table = hash_create((HASHCMP *) strcmp, 229, hash_string);
- for (i = 0; i < PCONN_HIST_SZ; i++)
+ for (int i = 0; i < PCONN_HIST_SZ; i++)
hist[i] = 0;
PconnModule::GetInstance()->add(this);
@@ -248,25 +266,22 @@
}
void
-PconnPool::push(int fd, const char *host, u_short port, const char *domain, Ip::Address &client_address)
+PconnPool::push(const Comm::ConnectionPointer &conn, const char *domain)
{
- IdleConnList *list;
- const char *aKey;
- LOCAL_ARRAY(char, desc, FD_DESC_SZ);
-
if (fdUsageHigh()) {
debugs(48, 3, "PconnPool::push: Not many unused FDs");
- comm_close(fd);
+ Comm::ConnectionPointer nonConst = conn;
+ nonConst->close();
return;
} else if (shutting_down) {
- comm_close(fd);
+ Comm::ConnectionPointer nonConst = conn;
+ nonConst->close();
debugs(48, 3, "PconnPool::push: Squid is shutting down. Refusing to do anything");
return;
}
- aKey = key(host, port, domain, client_address);
-
- list = (IdleConnList *) hash_lookup(table, aKey);
+ const char *aKey = key(conn, domain);
+ IdleConnList *list = (IdleConnList *) hash_lookup(table, aKey);
if (list == NULL) {
list = new IdleConnList(aKey, this);
@@ -276,48 +291,41 @@
debugs(48, 3, "PconnPool::push: found IdleConnList for {" << hashKeyStr(&list->hash) << "}" );
}
- list->push(fd);
+ list->push(conn);
+ assert(!comm_has_incomplete_write(conn->fd));
- assert(!comm_has_incomplete_write(fd));
- snprintf(desc, FD_DESC_SZ, "%s idle connection", host);
- fd_note(fd, desc);
- debugs(48, 3, "PconnPool::push: pushed FD " << fd << " for " << aKey);
+ LOCAL_ARRAY(char, desc, FD_DESC_SZ);
+ snprintf(desc, FD_DESC_SZ, "Idle: %s", aKey);
+ fd_note(conn->fd, desc);
+ debugs(48, 3, HERE << "pushed " << conn << " for " << aKey);
}
-/**
- * Return a pconn fd for host:port if available and retriable.
- * Otherwise, return -1.
- *
- * We close available persistent connection if the caller transaction is not
- * retriable to avoid having a growing number of open connections when many
- * transactions create persistent connections but are not retriable.
- */
-int
-PconnPool::pop(const char *host, u_short port, const char *domain, Ip::Address &client_address, bool isRetriable)
+bool
+PconnPool::pop(Comm::ConnectionPointer &destLink, const char *domain, bool isRetriable)
{
- const char * aKey = key(host, port, domain, client_address);
+ const char * aKey = key(destLink, domain);
IdleConnList *list = (IdleConnList *)hash_lookup(table, aKey);
if (list == NULL) {
debugs(48, 3, "PconnPool::pop: lookup for key {" << aKey << "} failed.");
- return -1;
+ return false;
} else {
debugs(48, 3, "PconnPool::pop: found " << hashKeyStr(&list->hash) << (isRetriable?"(to use)":"(to kill)") );
}
- int fd = list->findUseableFD(); // search from the end. skip pending reads.
-
- if (fd >= 0) {
- list->clearHandlers(fd);
- list->removeFD(fd); /* might delete list */
-
- if (!isRetriable) {
- comm_close(fd);
- return -1;
- }
+ Comm::ConnectionPointer temp = list->findUseable(destLink);
+
+ if (Comm::IsConnOpen(temp)) {
+ list->clearHandlers(temp);
+
+ /* might delete list */
+ if (list->remove(temp) && !isRetriable)
+ temp->close();
+ else
+ destLink = temp;
}
- return fd;
+ return true;
}
void
@@ -344,7 +352,7 @@
PconnModule::PconnModule() : pools(NULL), poolCount(0)
{
pools = (PconnPool **) xcalloc(MAX_NUM_PCONN_POOLS, sizeof(*pools));
- pconn_fds_pool = memPoolCreate("pconn_fds", PCONN_FDS_SZ * sizeof(int));
+//TODO: re-link to MemPools. WAS: pconn_fds_pool = memPoolCreate("pconn_fds", PCONN_FDS_SZ * sizeof(int));
debugs(48, 0, "persistent connection module initialized");
registerWithCacheManager();
}
@@ -369,8 +377,7 @@
void
-PconnModule::add
-(PconnPool *aPool)
+PconnModule::add(PconnPool *aPool)
{
assert(poolCount < MAX_NUM_PCONN_POOLS);
*(pools+poolCount) = aPool;
@@ -380,9 +387,7 @@
void
PconnModule::dump(StoreEntry *e)
{
- int i;
-
- for (i = 0; i < poolCount; i++) {
+ for (int i = 0; i < poolCount; i++) {
storeAppendPrintf(e, "\n Pool %d Stats\n", i);
(*(pools+i))->dumpHist(e);
storeAppendPrintf(e, "\n Pool %d Hash Table\n",i);
=== modified file 'src/pconn.h'
--- src/pconn.h 2010-05-02 18:52:45 +0000
+++ src/pconn.h 2010-10-02 16:33:56 +0000
@@ -26,7 +26,10 @@
/// \ingroup PConnAPI
#define PCONN_HIST_SZ (1<<16)
-/// \ingroup PConnAPI
+/** \ingroup PConnAPI
+ * A list of connections currently open to a particular destination end-point.
+ * We currently define the end-point by the FQDN it is serving.
+ */
class IdleConnList
{
public:
@@ -34,11 +37,26 @@
~IdleConnList();
int numIdle() { return nfds; }
- int findFDIndex(int fd); ///< search from the end of array
- void removeFD(int fd);
- void push(int fd);
- int findUseableFD(); ///< find first from the end not pending read fd.
- void clearHandlers(int fd);
+ int findIndex(const Comm::ConnectionPointer &conn); ///< search from the end of array
+
+ /**
+ * Search the list for an existing connection. Matches by FD.
+ *
+ * \retval false The connection does not currently exist in the list.
+ * We seem to have hit and lost a race condition.
+ * Nevermind, but MUST NOT do anything with the raw FD.
+ */
+ bool remove(const Comm::ConnectionPointer &conn);
+
+ void push(const Comm::ConnectionPointer &conn);
+
+ /** Search the list for a connection which matches the 'key' details.
+ * The list is created based on remote IP:port hash. This further filters
+ * the choices based on specific local-end details requested.
+ * If nothing usable is found the key is returned unchanged.
+ */
+ Comm::ConnectionPointer findUseable(const Comm::ConnectionPointer &key);
+ void clearHandlers(const Comm::ConnectionPointer &conn);
private:
static IOCB read;
@@ -48,11 +66,11 @@
hash_link hash; /** must be first */
private:
- int *fds;
+ Comm::ConnectionPointer *theList;
int nfds_alloc;
int nfds;
PconnPool *parent;
- char fakeReadBuf[4096];
+ char fakeReadBuf[4096]; // TODO: kill magic number.
CBDATA_CLASS2(IdleConnList);
};
@@ -65,7 +83,10 @@
/* for hash_table */
#include "hash.h"
-/// \ingroup PConnAPI
+/** \ingroup PConnAPI
+ * A pool of persistent connections for a particular service type.
+ * HTTP servers being one such pool type, ICAP services another etc.
+ */
class PconnPool
{
@@ -74,28 +95,38 @@
~PconnPool();
void moduleInit();
- void push(int fd, const char *host, u_short port, const char *domain, Ip::Address &client_address);
- int pop(const char *host, u_short port, const char *domain, Ip::Address &client_address, bool retriable);
+ void push(const Comm::ConnectionPointer &serverConn, const char *domain);
+
+ /**
+ * Updates destLink to point at an existing open connection if available and retriable.
+ * Otherwise, return false.
+ *
+ * We close available persistent connection if the caller transaction is not
+ * retriable to avoid having a growing number of open connections when many
+ * transactions create persistent connections but are not retriable.
+ */
+ bool pop(Comm::ConnectionPointer &serverConn, const char *domain, bool retriable);
void count(int uses);
- void dumpHist(StoreEntry *e);
- void dumpHash(StoreEntry *e);
+ void dumpHist(StoreEntry *e) const;
+ void dumpHash(StoreEntry *e) const;
void unlinkList(IdleConnList *list) const;
private:
- static const char *key(const char *host, u_short port, const char *domain, Ip::Address &client_address);
+ static const char *key(const Comm::ConnectionPointer &destLink, const char *domain);
int hist[PCONN_HIST_SZ];
hash_table *table;
const char *descr;
-
};
class StoreEntry;
class PconnPool;
-/// \ingroup PConnAPI
+/** \ingroup PConnAPI
+ * The global registry of persistent connection pools.
+ */
class PconnModule
{
