ICAP services use a "service" model of pconn different from the "TCP
destination" model which PconnPool objects are designed for.
This patch alters Adaptation::Icap::ServiceRep to use the simpler
IdleConnList object for pconn storage. IdleConnList stores a "set of
idle connections" more compatible with the ICAP model.
In order to implement ICAP max-connections feature the closeN()
operation is added to IdleConnList.
The result is removal of the complex hash and management operations on
push/pop of the idle conn set. The only expected behaviour change is
more frequent re-use of idle connections on services with multiple IP
addresses. Speed gains are minimal, but positive.
Amos
=== modified file 'src/adaptation/icap/ServiceRep.cc'
--- src/adaptation/icap/ServiceRep.cc 2011-05-13 10:38:28 +0000
+++ src/adaptation/icap/ServiceRep.cc 2011-06-15 01:36:17 +0000
@@ -24,7 +24,7 @@
theBusyConns(0),
theAllWaiters(0),
connOverloadReported(false),
- theIdleConns("ICAP Service"),
+ theIdleConns("ICAP Service",NULL),
isSuspended(0), notifying(false),
updateScheduled(false),
wasAnnouncedUp(true), // do not announce an "up" service at startup
@@ -85,8 +85,7 @@
{
Ip::Address client_addr;
- int connection = theIdleConns.pop(cfg().host.termedBuf(), cfg().port, NULL, client_addr,
- retriableXact);
+ int connection = theIdleConns.findUseableFD();
reused = connection >= 0; // reused a persistent connection
@@ -116,7 +115,7 @@
debugs(93, 3, HERE << "pushing pconn" << comment);
commSetTimeout(fd, -1, NULL, NULL);
Ip::Address anyAddr;
- theIdleConns.push(fd, cfg().host.termedBuf(), cfg().port, NULL, anyAddr);
+ theIdleConns.push(fd);
} else {
debugs(93, 3, HERE << "closing pconn" << comment);
// comm_close will clear timeout
@@ -133,7 +132,7 @@
void Adaptation::Icap::ServiceRep::noteConnectionUse(int fd)
{
Must(fd >= 0);
- fd_table[fd].noteUse(&theIdleConns);
+ fd_table[fd].noteUse(NULL); // pconn re-use but not via PconnPool API
}
void Adaptation::Icap::ServiceRep::setMaxConnections()
@@ -554,8 +553,7 @@
if (excess && theIdleConns.count() > 0) {
const int n = min(excess, theIdleConns.count());
debugs(93,5, HERE << "closing " << n << " pconns to relief debt");
- Ip::Address anyAddr;
- theIdleConns.closeN(n, cfg().host.termedBuf(), cfg().port, NULL, anyAddr);
+ theIdleConns.closeN(n);
}
scheduleNotification();
=== modified file 'src/adaptation/icap/ServiceRep.h'
--- src/adaptation/icap/ServiceRep.h 2011-05-13 10:38:28 +0000
+++ src/adaptation/icap/ServiceRep.h 2011-06-14 22:55:02 +0000
@@ -160,7 +160,7 @@
int theMaxConnections; ///< the maximum allowed connections to the service
// TODO: use a better type like the FadingCounter for connOverloadReported
mutable bool connOverloadReported; ///< whether we reported exceeding theMaxConnections
- PconnPool theIdleConns; ///< idle persistent connection pool
+ IdleConnList theIdleConns; ///< idle persistent connection pool
FadingCounter theSessionFailures;
const char *isSuspended; // also stores suspension reason for debugging
=== modified file 'src/pconn.cc'
--- src/pconn.cc 2011-05-13 10:38:28 +0000
+++ src/pconn.cc 2011-06-15 02:26:58 +0000
@@ -57,8 +57,8 @@
IdleConnList::~IdleConnList()
{
-
- parent->unlinkList(this);
+ if (parent)
+ parent->unlinkList(this);
if (nfds_alloc == PCONN_FDS_SZ)
pconn_fds_pool->freeOne(fds);
@@ -97,7 +97,53 @@
if (parent)
parent->noteConnectionRemoved();
- if (--nfds == 0) {
+ if (parent && --nfds == 0) {
+ debugs(48, 3, "IdleConnList::removeFD: deleting " << hashKeyStr(&hash));
+ delete this;
+ }
+}
+
+// almost a duplicate of removeFD. But drops multiple entries.
+void
+IdleConnList::closeN(size_t n)
+{
+ if (n < 1) {
+ debugs(48, 2, HERE << "Nothing to do.");
+ return;
+ } else if (n < (size_t)count()) {
+ debugs(48, 2, HERE << "Closing all entries.");
+ while (nfds >= 0) {
+ int fd = fds[--nfds];
+ fds[nfds] = -1;
+ clearHandlers(fd);
+ comm_close(fd);
+ if (parent)
+ parent->noteConnectionRemoved();
+ }
+ } else {
+ debugs(48, 2, HERE << "Closing " << n << " of " << nfds << " entries.");
+
+ size_t index = 0;
+ // ensure the first N entries are closed
+ while (index < n) {
+ int fd = fds[--nfds];
+ fds[nfds] = -1;
+ clearHandlers(fd);
+ comm_close(fd);
+ if (parent)
+ parent->noteConnectionRemoved();
+ }
+ // shuffle the list N down.
+ for (;index < (size_t)nfds; index++) {
+ fds[index - n] = fds[index];
+ }
+ // ensure the last N entries are unset
+ while (index < ((size_t)nfds) + n) {
+ fds[index] = -1;
+ }
+ }
+
+ if (parent && nfds == 0) {
debugs(48, 3, "IdleConnList::removeFD: deleting " << hashKeyStr(&hash));
delete this;
}
=== modified file 'src/pconn.h'
--- src/pconn.h 2011-05-13 10:38:28 +0000
+++ src/pconn.h 2011-06-15 00:07:46 +0000
@@ -35,6 +35,7 @@
int findFDIndex(int fd); ///< search from the end of array
void removeFD(int fd);
+ void closeN(size_t count);
void push(int fd);
int findUseableFD(); ///< find first from the end not pending read fd.
void clearHandlers(int fd);
