Any comment on this patch?
Can I commit it to trunk?
On 05/09/2011 12:41 PM, 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...
Maybe we can use
if (!Ip::EnableIpv6 || Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK &&
!cfg().ipv6))
But I let it as you suggested just for the "split-stact for now..." comment
connection = comm_open(SOCK_STREAM, 0, anyAddr, COMM_NONBLOCKING,
cfg().uri.termedBuf());
}
...
NP: the failover logics in comm_openex() are for handling sockets with
tcp_outgoing_addr pre-known. *_ANY_ADDR will never fail based on family
and so will never failover.
Amos
