Alex Rousskov wrote:
Hello,
Please consider the following changes for Squid3 trunk inclusion.
They have been been tested in the lab and will be put in production. The
ICAP chains feature (i.e., a "pipeline" of ICAP services) has been on
many wish lists. ICAP service sets allow for backup ICAP servers which
are typical in busy/critical adaptation environments.
I compressed the 90KB patch (am I being too paranoid?), but the attached
text file shows squid.conf documentation describing the features listed
below. I have also quoted relevant commit messages that may help with
code understanding. The patch includes adaptation notes.dox file with
developer-centric documentation.
Not too paranoid, this one got through to me :)
If approved, I will try to do a "bzr merge" instead of a raw patch to
preserve commit messages, using a Robert-provided trick (thanks, Robert!).
This work depends on the enhanced logging features submitted previously.
Thank you,
Alex.
bb:approve
--------------------------------
Support adaptation sets and chains, including dynamic ICAP chains.
- Support adaptation service sets and chains
(adaptation_service_set and adaptation_service_chain)
- Dynamically form chains based on ICAP X-Next-Services header
(icap_service routing=on)
------------------------------------
Support adaptation service sets and chains.
An adaptation service set contains similar, interchangeable services. No more
than one service is successfully applied. If one service is down or fails,
Squid can use another service. Think "hot standby" or "spare" ICAP servers.
Sets may seem similar to the existing "service bypass" feature, but they allow
the failed adaptation to be retried and succeed if a replacement service is
available. The services in a set may be all optional or all essential,
depending on whether ignoring the entire set is acceptable. The mixture of
optional and essential services in a set is supported, but yields results that
may be difficult for a human to anticipate or interpret. Squid warns when it
detects such a mixture.
When performing adaptations with a set, failures at a service (optional or
essential, does not matter) are retried with a different service if possible.
If there are no more replacement services left to try, the failure is treated
depending on whether the last service tried was optional or essential: Squid
either tries to ignore the failure and proceed or terminates the master
transaction.
An adaptation chain is a list of different services applied one after another,
forming an adaptation pipeline. Services in a chain may be optional or
essential. When performing adaptations, failures at an optional service are
ignored as if the service did not exist in the chain.
Request satisfaction terminates the adaptation chain.
When forming a set or chain for a given transaction, optional down services
are ignored as if they did not exist.
ICAP and eCAP services can be mixed and matched in an adaptation set or chain.
* Implementation notes
The notes below focus on _changes_. Adaptation terminology and current layers
are now being documented in src/adaptation/notes.dox
Service sets and chains are implemented as ServiceGroup class kids. They are
very similar in most code aspects. The primary external difference is that
ServiceSet can "replace" a service and ServiceChain can find the "next"
service. The internal search code is implemented in ServiceGroup parent and
is parametrized by the kids.
Before the adaptation starts, Squid calculates the adaptation "plan", which is
just an iterator into the ServiceGroup. The client- and server-side adaptation
initiators used to deal with Service pointers. They now deal with ServiceGroup
pointers. The only interesting difference is that a ServiceGroup does not have
a notion of being optional or essential. Thus, if adaptation start fails, we
do not know whether the failure can be bypassed. Fortunately, starting an
adaptation does not require anything that depends on the adaptation services,
so we now simply assert that the start succeeds.
If the entire adaptation fails, the callers are notified as before. They are
told whether they can ignore the failure as before. No changes there.
A new Adaptation::Iterator class has been added to execute the adaptation
plan. That class is responsible for iterating the services in a service group
until the plan is exhausted or cannot progress due to a final failure.
Dynamically form adaptation chains based on the ICAP X-Next-Services header.
If an ICAP service with the routing=1 option in squid.conf returns an ICAP
X-Next-Services response header during a successful REQMOD or RESPMOD
transaction, Squid abandons the original adaptation plan and forms a new
adaptation chain consisting of services identified in the X-Next-Services
header value (using a comma-separated list of adaptation service names from
squid.conf). The dynamically created chain is destroyed once the new plan is
completed or replaced.
This feature is useful when a custom adaptation service knows which other
services are applicable to the message being adapted.
Limit adaptation iterations to adaptation_service_iteration_limit to protect
Squid from infinite adaptation loops caused by ICAP services constantly
including themselves in the dynamic adaptation chain they request. When the
limit is exceeded, the master transaction fails. The default limit of 16
should be large enough to not require an explicit configuration in most
environments yet may be small enough to limit side-effects of loops.
I see your comment at src/adaptation/AccessCheck.cc: ~156
TODO: should we shift and checkCandidates if and only if (!g) ?!
If the TODO is about the case of self-reference you mention above then I
would say yes we must. To drop such loops into error condition as
quickly as possible. I understand the limit needs to be kept anyway for
multi-stage loops. But losing the loop early saves a lot of resources.
Also, should we prevent circular loops by way of preventing a filter
being run twice over the same request? There have to be few cases where
a single adaptation step needs to be repeated exactly.
IMO we are finding more and more reasons to add an ERR_*_CONFIG page :)
TODO: Add metadata support to eCAP API and honor X-Next-Services there as
well. Currently, only ICAP services can form dynamic chains but the formed
chains may contain eCAP services.
Other improvements:
Polished adaptation service configuration in squid.conf. Old format with an
anonymous bypass option is deprecated but still supported. Quit with a fatal
message if an adaptation service is misconfigured (debugging level-0 messages
do not seem to work at that stage, but that is probably another, general bug).
Aye general bug. Pre-configure startup hits it as well :(
Polished HttpRequest::adaptHistory() interface so that the code that knows the
history is needed can force history creation without complex
configuration-time preparations and state. Currently, all adaptation history
users but the logging-related ones know runtime whether the history must be
created (e.g., when a certain ICAP header is received).
Fixed "canonical" Request URL maintenance when ICAP clones requests.
TODO: The urlCanonical() must become HttpRequest::canonical(), hiding the
often out-of-sync canonical data member.
IMO: TODO: url cleanup bug needs closing to handle canonical and all
other URL display cases properly behind a single URL object. Requires
decent string code though or memory duplicates will blow out of
proportion to usefulness.
Fixed ICAP request parsing (for ICAP logging). We used to parse Request-Line
as if it were the first header. TODO: optimize by parsing only when needed.
I took a look at the patch, and decided to take your word that it works :)
Please reference and close
http://www.squid-cache.org/bugs/show_bug.cgi?id=2087
in the commit if/when this goes in.
Some stuff I found in the code:
in src/HttpRequest.cc please add brackets to make the code more readable:
"loggingNeedsHistory = LogfileStatus == LOG_ENABLE &&
alLogformatHasAdaptToken;"
in src/adaptation/AccessCheck.cc you create a ServiceFilter via:
"new AccessCheck( ServiceFilter(method, vp, req, rep), cb, cbdata);"
which takes a copy by & reference. The ServiceFilter locks the given
request and reply but I can't see where the ServiceFilter is deleted (&
references cannot be and the object is not RefCounted.)
- I expect this to lead to memory leaks. Possibly big ones.
in adaptation/icap/ModXact.cc when testing for HDR_X_NEXT_SERVICES I
think it would be worth warning if a non-routable service attempted to
hand back a routing header. How to do it efficiently may be a problem
though.
This TODO needs to be enacted rather soon. We are aiming for performance
in 3.2...
// TODO: use HttpMsg::clone()!
On the side; this has brought to my attention the adaptation/forward.h.
Can we not call that adaptation/Adaptation.h instead? the forward.h
breaks CamelCase and will clash with src/forward.h on those recently
vocal compilers.
notes.dox ***
\section label takes two parameters: an ID then the text title.
mixing \b and the <b></b> equivalents on alternate lines looks a bit
mucky. Can we at least do it consistently within one document?
Thats it from me. Just ... no more huge project for a few decades, yes? ;)
Amos
--
Please be using
Current Stable Squid 2.7.STABLE6 or 3.0.STABLE16
Current Beta Squid 3.1.0.9
