Re: [PATCH] Service Overload Handling (ICAP Max-Connections)

 On Mon, 09 May 2011 12:41:42 +0300, Tsantilas Christos wrote:
> A fixed patch.
>
>
> On 05/08/2011 06:46 AM, Amos Jeffries wrote:
>> On 27/04/11 23:40, Tsantilas Christos wrote:
>>> This patch implements the "Service Overload Handling" feature as
>>> described in the squid wiki page:
>>> http://wiki.squid-cache.org/Features/ServiceOverload
>>>
>>> This feature also exist as bug #2055 in the squid bugzilla:
>>> http://bugs.squid-cache.org/show_bug.cgi?id=2055
>>>
>>> More informations included in the patch preamble.
>>>
>>> Regards,
>>> Christos
>>
>> Some relatively minor tweaks in the parse handling:
>>
>> Adaptation::ServiceConfig::grokLong(
>> Adaptation::ServiceConfig::grokOnOverload(
>> Adaptation::ServiceConfig::grokBool(
>>
>> - Please use DBG_CRITICAL instead of ", 0,". Or if it is not
>> critical
>> bump to an appropriate alternative (parsing errors are usually
>> DBG_IMPORTANT).
>>
>> - use "ERROR:", "FATAL:","WARNING:" as appropriate to the
>> seriousness of
>> the config problem.
>>
>> * For parsing problem debugs please just use "cfg_filename" instead
>> of
>> "HERE << cfg_filename".
>
> I fixed a little the ServiceConfig::grokLong and
> ServiceConfig::grokOnOverload methods.
> I will fix the ::grokBool method with a separate patch after
> max-connections patch commited.
>
>
>
>>
>>
>> One *Major* problem:
>>
>> Adaptation::Icap::ServiceRep::getConnection() is missing IP
>> split-stack
>> support removed from Adaptation::Icap::Xaction::openConnection().
>
> Agrr
> I missed this one....
>
>>
>> On systems where IPv6 sockets can be opened but the ICAP server is
>> IPv4-only (MacOSX, OpenBSD, Windows, manually disabled kernels) this
>> will fail to connect() the socket.
>>
>> You will need to preserve the ipv6=on|off option behaviour. Like so:
>>
>> Ip::Address anyAddr; // default: IP6_ANY_ADDR
>> ...
>> // need a new connection
>> if (!reuse) {
>> if (!Ip::EnableIpv6)
>> anyAddr.SetIPv4();
>> else if (Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && !cfg().ipv6)
>> /* split-stack for now requires default IPv4-only socket */
>> anyAddr.SetIPv4();
>
> We are assuming that Address::SetIPv4 will always return true for
> ANY_ADDR...

 For ANY_ADDR it will always succeed, SetIPv4() is effectively a
 bit-toggle. It only fails when there is no possible mapping, ie an IPv6
 address (excluding localhost).

>
> Maybe we can use
> if (!Ip::EnableIpv6 || Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK &&
> !cfg().ipv6))

 No need. There are four stacks:
  IPv4-only
  Split (Classical Dual)
  Hybrid (Extended Dual)
  IPv6-only

 When !Ip::EnableIpv6 the default anyAddr is made IPv4-only. For the
 others we know IPv6 has at least a working stack and default to v6.
 Outside of Ip::Address we only need to care about the Split case where
 sockets are IP family sensitive.

 Amos
Received on Tue May 10 2011 - 00:44:58 MDT