Thanks Alex,
Inline comments..
On 08/01/2016 17:27, Alex Rousskov wrote:
On 01/07/2016 07:03 PM, Eliezer Croitoru wrote:
I added a configurable option to the ICAP services named "nodown" but
maybe another name would be better fit.
Please do not use negative option names. If this feature is committed
despite my objections, please consider using "always-up" or another
positive name.
Both nodown and always-up do not describe the exact meaning of the flag.
Do you(..and others) think that a longer name would be better? something
like "options-only-suspened" or "suspend-only-by-options-fail" ?
The idea of the patch is to allow the admin to count on the periodic
OPTIONS request only to identify the current state of the ICAP service.
If an ICAP OPTIONS request fails, will the service be marked as down? If
yes, the option name is misleading. If not, your description above is
misleading. If this feature is committed, please adjust the
documentation to clearly define whether Squid may mark a nodown=1
service as down for any reason.
I suspected that my words might not be descriptive enough and I will try
to clear it out in the next comments.
I was thinking about using the icap_service_failure_limit with a very
high limit but it is an ICAP global configuration and not a service
specific configuration.
IMO, you should add a service-specific failure-limit option instead of
adding a brand new option with an overlapping but very narrow scope.
Squid already implements the failure-limit logic on a per-service basis.
You just need to add a per-service option (that will default to the
global one).
I was thinking about it but this feature eliminates every suspension
between each OPTIONS fetch\check.
I know it doesn't fit all setups but I want to eliminate any other test
then the OPTIONS fetch and the reason is that only using a very high
failure limit will answer to this specific attack I have seen. A simple
JS just sent about 1k requests pretty fast and with all of them failing
pretty fast the ICAP service is just being suspended so just eliminating
the issues solved my problem.
(I cannot do a thing about a webui programmer that doesn't like to do
his job properly??)
+ [...] It gives the
+ service admin to implement his own heart-beat script which
+ will work as a replacment to the default internal requests
+ success\failure probes while the ICAP service state is UP.
+ An external heart-beat script can run under external_acl
+ and can be checked in http_access and couple other acls
+ vectoring points.
IMO, this text should not be added to the new option because it talks
about controlling access to an ICAP service rather than about [not]
changing the service state which the new option controls. Referring to
adaptation_access from the new option documentation and adding a similar
hint to the adaptation_access documentation instead may be a good idea.
Not really but I do understand what you are writing about.
The intention is very specific and not related to the adaptation_access,
the idea is that the service is essential and the proxy cannot allow any
traffic without this service due to some health facilities
confidentiality policy. So for the case that the service is indeed down
the admin needs to deny access to the whole service since it's useless
without the adaptation service. The external_acl is per squid port and
will deny access to the whole service based on the real status of the
ICAP service.
If I will use adaptation_access it will lead to a situation which the
clients traffic will not pass inspection which is worst then having ICAP
failures per specific wrongly formed requests.
However I did not test what is the situation with adaptation services
set and if their "chained" use is based only on full suspension or it
will also pass the request to the next host per request failure.
I will conduct couple tests this week to make sure how things works with
sets.
Do you think a wiki article will be good enough to explain the use cases
of this option instead of the documentation?
I was thinking about writing one.
HTH,
Alex.
Again Thanks!
Eliezer
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
