Re: [PATCH] : Count retries and redispatches also for servers + extend logs + %d->%u cleanup

From: Krzysztof Oledzki <ole#ans.pl>
Date: Wed, 26 Dec 2007 01:17:10 +0100 (CET)

On Wed, 26 Dec 2007, Krzysztof Oledzki wrote:

>
>
> On Tue, 25 Dec 2007, Krzysztof Oledzki wrote:
>
>>
>>
>> On Tue, 25 Dec 2007, Krzysztof Oledzki wrote:
>>
>>> From c71a482f5c303392d06c228d868a4898efde07df Mon Sep 17 00:00:00 2001
>>> From: Krzysztof Piotr Oledzki <ole#ans.pl>
>>> Date: Tue, 25 Dec 2007 04:01:58 +0100
>>> Subject: [MINOR]: Count retries and redispatches also for servers + extend
>>> logs + %d->%u cleanup
>>>
>>> This patch extends a little previously added functionality to also
>>> count retries and redispatches for servers. Now it is possible to know
>>> which server causes redispatches as it is not always the same that takes
>>> most retries.
>>>
>>> I also extended logs to show how many retries were done and if
>>> redispatching
>>> was necessary ('+'). Currently I'm using "s->conn_retries>0" logic to
>>> match
>>> redispatched connections but in the future it may be required to add for
>>> example
>>> additional SN_REDISP flag.
>>
>> Blah, this is wrong. :( Even without redispatching s->conn_retries falls
>> below 0 when last retry fails. So, we need this SN_REDISP anyway. Please
>> ignore c71a482f5c303392d06c228d868a4898efde07df and
>> 227206d2f11feb3b52909f86917f0a560d343871 for now.
>>
>> I'll send updated patch.
>
> OK, done. However, I found one problem I would like to get fixed. When
> haproxy decides that session needs to be redispatched it chose a server but
> there is no guarantee that for it to be a different one. So, it often happens
> that selected server is exactly the same that it was previously so a client
> ends up with a 503 error anyway, especially when one sever has much bigger
> weight than others.
>
> So, there should be a possibility to tell redispatcher that we want to omit a
> specific server.
>
> I'm looking for a smart solution for this, any ideas?

Hm... How about something like this:

• don't set "t->srv = NULL" when redispatched, but only clear SN_ASSIGNED
• extend fwrr_get_next_server to get and use info about a server we need to exclude:

(...)
-static struct server *fwrr_get_next_server(struct proxy *p) +static struct server *fwrr_get_next_server(struct proxy *p, struct server *esrv) { (...)

 		grp->curr_pos++;
+		if (srv != esrv &&
 		 	(!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv))
 				break;

-		/* the server is saturated, let's chain it for later reinsertion */
+		/* the server is saturated or it was asked to be excluded, 
+		 * let's chain it for later reinsertion */

(...)
}

• pass current server in assign_server(), it should may be NULL: int assign_server(struct session *s) (...) case BE_LB_ALGO_RR: s->srv = fwrr_get_next_server(s->be); + s->srv = fwrr_get_next_server(s->be, s->srv); (...)

TODO: what about other methods (SH/UH/PH)?

Above code not tested, waiting for comments before start coding. ;)

Best regards,

                                 Krzysztof Olêdzki Received on 2007/12/26 01:17