Re: [squid-dev] [RFC\PREVIEW] ICAP service nodown option addition.
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. > 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 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). > + [...] 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. HTH, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [RFC\PREVIEW] ICAP service nodown option addition.
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
Re: [squid-dev] [RFC\PREVIEW] ICAP service nodown option addition.
On 01/09/2016 12:35 PM, Eliezer Croitoru wrote: > 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" ? Let's finalize the scope of the changes before deciding how to name them. My comment above was specific to the "negativity" of the name you have chosen. The negativity should be avoided regardless of the feature scope. As for the scope of the changes, I continue to think that a different solution is needed. > I was thinking about it but this feature eliminates every suspension > between each OPTIONS fetch\check. IMO, the need for "eliminating suspension between OPTIONS" should be addressed by adding an adaptation_failure directive driven by ACLs instead of adding optional hard-coded decision logic. For example, # "allowed" adaptation failures are not counted as service failures adaptation_failure allow all > I know it doesn't fit all setups but ... Being applicable to "all setups" is impossible and is not required for an optional feature to be accepted. However, while your specific use case is important, we ought to consider other use cases as well, especially when adding new configuration options. The adaptation_failure directive will address a lot more use cases, including yours. It is not difficult to implement. Thus, if we add something, we should add the more general adaptation_failure (or similar) support and not nodown (or similar) support. > the idea is that the service is essential and the proxy cannot allow any > traffic without this service Also known as the default bypass=0 setting combined with the "send all requests to this service" adaptation_access rule. This is already fully supported. What Squid does not yet support is an admin-configurable classification of adaptation failures: Which adaptation failures are specific to the adaptation transaction and which are a sign of a failing adaptation service? Your "nodown" proposal classifies all adaptation failures as non-service failures. We can do much better without a lot more work. > Do you think a wiki article will be good enough to explain the use cases > of this option instead of the documentation? I think we need a different configuration option or two. I do not think the right new options require extensive documentation on the wiki, but describing your specific use case and how you have handled it using those new options may be useful for many admins, of course. HTH, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
