[PATCH] cleanup-comm

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Sun, 05 Jun 2011 02:05:22 +1200

Updated to apply on current 3.HEAD with a few minor fixes spoken about
last audit submission.

Changes since last submission:
  * made Comm::Connection a pooled class. This resolves the issues
mentioned about pconn becoming un-pooled. I've run this now for several
hours personal traffic with no problems appearing.

  * changed ServerStateData::dataDescriptor() to ...::dataConnection()
as requested.

  * re-constructed the Comm::Connection handling in ICAP after
max-connections patch. There is a pconn key problem now.

  The new model of pconn pool requires remote IP:port for the key. ICAP
does not have any IP visibly available in closeN() where 3.HEAD
pre-emptively purges idle pconn to a smaller max. So this patch breaks
that optimization. Existing connections will still be closed without
re-pooling as requests finish with them.
   I don't see this as a major issue to block the patch merge. But would
like to fix it ASAP if anyone can point out a simple change to do so.

In general overview:

  The premise underlying this whole patch is that instead of copying and
re-copying and re-lookups for the FD related data we can take the
ConnectionDetail class which is generated to store a few bits of IP
information about newly accept()'d connections and make it persist
across the whole of Squid.

  It has been renamed from ConnectionDetails to Comm::Connection and has
absorbed a few FD data fields from other classes long the code paths.
Its scope is to hold an FD (or potential FD) plus meta data.

Comm::Connection are valid before, during and after the period when
their particular FD is open. The meta data may be used beforehand to
setup the FD (in the case of peer selection or other TcpAcceptor usage),
and it may remain in use after FD closure until logging or all linked
job and call objects have detected the closure and terminated. A global
function Comm::IsConnOpen() may be used on the pointer objects to detect
whether they point at an active connection.

  Most of the patch is simple parameter changes to functions and methods
to pass a "cont Comm::ConnectionPointer &" instead of an "int FD". Along
with class FD fields being converted to these object pointers.

In order to support this alteration there have been behavioral changes to:

  the socket accept() Job
   Comm::TcpAcceptor altered to spawn Comm::Connection objects and to
operate with one controlling their active/closed state.

  FTP data channel handling Calls.
   efficiency improvements making use of Comm::Connection as a feedback
channel between TcpAcceptor and FtpStateData to cancel the listening
Job. Most of the underlying logic change is already in trunk to use the
Subscription API. This just streamlines and fixes some race bugs.

  Peer selection
   updated to spawn a set of Comm::Connection objects. To do this it is
updated to determine *all* peers including DIRECT ones. Doing the DNS
lookup instead of leaving it to comm_connect() on the other side of
FwdState. It also absorbs the outgoing address selection from FwdState
and can now specify details of local+remote ends of an outgoing TCP link.

  Forwarding
   updated to handle the new outputs from peer selection and to open
sequentially.

  pconn handling
    updated to use destination IP/port and hold a Comm::Connection
instead of domain name indexing an FD. This allows us to maintain idle
pools and re-use FD more efficiently with virtual-hosted servers. Along
with maintaining certainty that the pconn selected actually goes to the
exact destination IP:port needed by forwarding.

  comm layer outgoing connections
   now have a control job Comm::ConnOpener to do this. Due to the peer
selection and forwarding changes this is a much simpler operation.

  http / tunnel / gopher / whois / ftp
   updated to receive a server and client Comm::Connection object from
forwarding. To operate on those until they close or are finished with.

  SNMP / ICP / HTCP / DNS port listeners
   updated to work with Comm::Connection holding their listening socket
meta data. This is a side-effect of the ICP and Comm read/write/timeout
changes.

This patch closes the following bugs:
   http://bugs.squid-cache.org/show_bug.cgi?id=2561
   http://bugs.squid-cache.org/show_bug.cgi?id=3050
   http://bugs.squid-cache.org/show_bug.cgi?id=3177

By passing FD meta data as Comm::Connection it forms the groundwork to
also close these bugs quite easily:
   http://bugs.squid-cache.org/show_bug.cgi?id=2279
   http://bugs.squid-cache.org/show_bug.cgi?id=692
   http://bugs.squid-cache.org/show_bug.cgi?id=2558

I believe these are also fixed or nearly so, but have not tested. They
fall into one of the two categories above:
  http://bugs.squid-cache.org/show_bug.cgi?id=3111
  http://bugs.squid-cache.org/show_bug.cgi?id=3108
  http://bugs.squid-cache.org/show_bug.cgi?id=3045

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.12
   Beta testers wanted for 3.2.0.8 and 3.1.12.2

Received on Sat Jun 04 2011 - 14:05:44 MDT

This archive was generated by hypermail 2.2.0 : Fri Jun 17 2011 - 12:00:04 MDT