[prev in list] [next in list] [prev in thread] [next in thread]
List: squid-cvs
Subject: cvs commit: squid3/src/ICAP ICAPClientVector.cc ICAPClientVector.h
From: Duane Wessels <wessels () squid-cache ! org>
Date: 2006-10-31 23:30:59
Message-ID: 200610312330.k9VNUxMU098431 () squid-cache ! org
[Download RAW message or body]
wessels 2006/10/31 16:30:59 MST
Modified files:
src/ICAP ChunkedCodingParser.cc
ICAPClientReqmodPrecache.cc
ICAPClientReqmodPrecache.h
ICAPClientRespmodPrecache.cc
ICAPClientRespmodPrecache.h ICAPConfig.cc
ICAPConfig.h ICAPModXact.cc ICAPModXact.h
ICAPOptXact.cc ICAPOptXact.h
ICAPOptions.cc ICAPOptions.h
ICAPServiceRep.cc ICAPServiceRep.h
ICAPXaction.cc ICAPXaction.h MsgPipe.cc
MsgPipeData.h TextException.h
Added files:
src/ICAP ICAPClientVector.cc ICAPClientVector.h
Log:
- Many ICAP fixes from Alex Rousskov accumulated on the
sourceforge squid3-icap branch since 2006/10, including:
- Polished ICAP service selection code and implemented bypass of
optional services. The code implements icap_class
configuration directive which is currently used as a "set of
interchangeable ICAP services". Squid2 and current squid.conf
may imply otherwise.
- Support Transfer-* ICAP OPTIONS response header. If Squid
knows that a service does not want the URL, Squid will not use
the service, even if it is an essential service with
bypass=0. Note that we may make this decision before we know
what the service wants. Eventually, ACLs should initiate and
wait for the OPTIONS transaction for yet-unprobed services.
- When ICAP transactions fail to connect to the service many
times, the service is suspended until the next OPTIONS
update. The limit is currently hard-coded to 10. Suspended
service is a down service and will be skipped by the ACL
service selection algorithm.
- Rewrote the code updating ICAP service options. We no longer
mark the service being updated as "down". Only presence of
valid and fresh options is important. We also try to update
the options before they expire to avoid any service downtime
or use of stale options.
- Report interesting changes in the ICAP service state, some
with debugging level one to alert the cache administrator.
- When cloning a request during an ICAP 204 "No Content" REQMOD
response, preserve the client address so that the rest of the
code has access to it. This change appears to fix Squid Bug
#1712.
- After ICAP 100 Continue, expect new ICAP headers instead of
HTTP headers. Reset ICAP message object to be ready to parse
ICAP headers again. (Tsantilas Christos
<chtsanti@users.sourceforge.net>)
- The ieof HTTP chunk-extension was written after chunk-data
instead of being written after the chunk-size. (Tsantilas
Christos <chtsanti@users.sourceforge.net>)
- Merged common code from the ICAPClientReqmodPrecache and
ICAPClientReqmodPrecache classes into the newly added
ICAPClientVector class. The specific vectors do not have a
common owner (yet?) because ServerStateData and
ClientHttpRequest do not have a common base class. Thus,
ICAPClientVector has to rely on its kids to communicate with
their owners. However, at least 50% of the logic was common
and has been moved. Eventually, we may want to create a
simple ICAPOwner API that ServerStateData and
ClientHttpRequest can implement and ICAPClientVector can rely
on. This will make the code simpler and more efficient. The
big merge was motivated by a couple of bugs that were found
in one vector class but that did not exist or behaved
differently in the other vector, mostly likely due to natural
diversion of used-to-be identical code.
- Rewrote communication between a server-side ICAPClient*mod*
vector and its owner. When a server-side ICAPClient*mod*
vector was notifying its owner of more adapted data, the
owner could delete the vector (by calling icap->ownerAbort)
if the store entry was not willing to accept the data. The
same deletion could happen when a vector was notifying the
owner of a successful termination. In all those cases, the
vector did not expect to be deleted and could continue to do
something, causing segmentation faults. Now, when more data
is available, the vector calls its owner and checks the
return value of the call. If it is false, the vector knows it
has been deleted and quits. When vector terminates, it calls
its owner and trusts the owner to always delete the vector.
The "check return value and quit" design is not perfect, but
we are paying the price for isolating the vectors from their
owners while using direct calls between them (instead of
MsgPipe or a similar less efficient indirect approach we use
elsewhere).
- Renamed doIcap to startIcap and moved more common code there.
Changed its return type to bool. We now handle three cases
when ICAP ACLs call back: 1) No service was selected
(because there was no applicable service or because all
applicable services were broken and optional). We proceed as
if ICAP was not configured. 2) The selected essential
service is broken. This is a fatal transaction error and we
return an "ICAP protocol error" HTTP error response. We could
proceed with the ICAP stuff, but it saves a lot of cycles to
abort early. 3) The selected service is not broken. We
proceed with the ICAP stuff. The old code did not detect
case #2, even though there was code to handle that case (with
dangerous XXX suggestions that are now gone). The code
should probably be polished further to move common ftp/http
logic from icapAclCheckDone()s into ServerStateData.
- Make sure there is an accept callback when we are accepting.
If there is no callback and we accept, we will silently leak
the accepted FD. When we are running out of FDs, there is
often no accept callback. The old code, when running out of
FDs, would create lots of "orphaned" or "forgotten" FDs that
will eventually get into a CLOSED_WAIT state and remain there
until Squid quits. The new code does not call accept() if
there is no accept callback and does not register the accept
FD for reading if the AcceptLimiter is deferring, because
when the AcceptLimiter kicks in, it will register the accept
FD for reading. There are most likely other places/cases
where accept FD should not be registered for reading.
- When an exception is caught, mark the ICAP connection as
non-reusable so that it is not recycled while a write is
pending but simply closed instead. Our write callback will
still be called, unfortunately, because there is no way to
clear the callback without invalidating its data (i.e., the
transaction pointer). This change prevents pconn.cc:253:
"!comm_has_incomplete_write(fd)" assertion from firing when
things go wrong (e.g., the ICAP server cannot be contacted to
retrieve OPTIONS). Not all exceptions caught by the ICAP
xaction should lead to the ICAP connection termination, but
it is very difficult if not impossible to reliably detect
exceptional conditions when it is safe to reuse the ICAP
connection, and it is probably not worth it anyway.
- Added Tsantilas Christos <chtsanti@users.sourceforge.net>
to CONTRIBUTORS for fixing ICAP bugs.
- Polished debugging.
Revision Changes Path
1.2 +2 -2 squid3/src/ICAP/ChunkedCodingParser.cc
1.9 +28 -148 squid3/src/ICAP/ICAPClientReqmodPrecache.cc
1.4 +20 -34 squid3/src/ICAP/ICAPClientReqmodPrecache.h
1.8 +30 -163 squid3/src/ICAP/ICAPClientRespmodPrecache.cc
1.4 +17 -35 squid3/src/ICAP/ICAPClientRespmodPrecache.h
1.12 +69 -34 squid3/src/ICAP/ICAPConfig.cc
1.10 +2 -1 squid3/src/ICAP/ICAPConfig.h
1.25 +113 -76 squid3/src/ICAP/ICAPModXact.cc
1.6 +11 -8 squid3/src/ICAP/ICAPModXact.h
1.3 +14 -3 squid3/src/ICAP/ICAPOptXact.cc
1.4 +2 -1 squid3/src/ICAP/ICAPOptXact.h
1.9 +110 -97 squid3/src/ICAP/ICAPOptions.cc
1.8 +31 -26 squid3/src/ICAP/ICAPOptions.h
1.8 +152 -77 squid3/src/ICAP/ICAPServiceRep.cc
1.5 +48 -18 squid3/src/ICAP/ICAPServiceRep.h
1.9 +77 -25 squid3/src/ICAP/ICAPXaction.cc
1.9 +14 -9 squid3/src/ICAP/ICAPXaction.h
1.4 +4 -4 squid3/src/ICAP/MsgPipe.cc
1.8 +11 -7 squid3/src/ICAP/MsgPipeData.h
1.2 +5 -5 squid3/src/ICAP/TextException.h
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic