Re: Changes in the check timeouts

From: Krzysztof Oledzki <ole#ans.pl>
Date: Fri, 15 Feb 2008 21:10:15 +0100 (CET)

On Fri, 15 Feb 2008, Willy Tarreau wrote:

> On Fri, Feb 15, 2008 at 06:13:14PM +0100, Krzysztof Oledzki wrote:

>>
>> What we need is something like this:
>>
>> --- a/src/cfgparse.c
>> +++ b/src/cfgparse.c
>> @@ -516,6 +516,7 @@ static void init_default_instance()
>>         defproxy.maxconn = cfg_maxpconn;
>>         defproxy.conn_retries = CONN_RETRIES;
>>         defproxy.logfac1 = defproxy.logfac2 = -1; /* log disabled */
>> +       tv_eternity(&defproxy.timeout.check);
>>         tv_eternity(&defproxy.timeout.client);
>>         tv_eternity(&defproxy.timeout.connect);
>>         tv_eternity(&defproxy.timeout.server);
>>
>> So simply, so stupid. :( Without this fix
>> tv_isset(&s->proxy->timeout.check) is not going to return what we expect. :(
>>
>> But like I said: I *really* like to be sure that this is all.
>

> No, that's not all. This one, I've fixed it this morning.

Sorry about that. :(

> The second problem is the way the check connect timeout is computed. 1)
> it is complex and hard to understand from a user standpoint. 2) it's
> bogus since if some timeouts are not set, they are still added to <now>.
> That's what my patch addressed, which is more a functional issue than a
> technical issue :

>

> original code does :

>


>   if (timeout.check)

>       timeout = min(inter, timeout.connect)

>   else

>       timeout = inter


And timeout += timeout.check *if* connection was successfully established. 
So, we still allow for a server to process a request (it may take some 
time) but detect dead servers sooner.

> It's hard to understand why timeout.check has an impact here and how it's
> used.

Because timeout.check is and additional timeout available after a connections was established.

> The simpler method (and IMHO more logical) is the following one :

>


>   if (timeout.connect)

>      timeout = timeout.connect

>   else

>      timeout = inter

>


>   if (timeout.check)

>      bound (timeount, timeout.check)


Hm... timeout.check is was not supposed to bound timeount but rather to 
give a possibility to have short timeout for connect and additional 
timeout for response. As you have suggested, some people have large 
timeout.connect for queue/tarpit, others have large inter. Maybe it is not 
very easy to understand, but imho this is the only clean solution and is 

documented. Alternatively we may simply use timeout.connect alone and force people to use "timeout queue" and "timeout tarpit" instead of large timeout.connect but it looks too invasive.

Best regards,

                         Krzysztof Olêdzki Received on 2008/02/15 21:10