Re: [PATCH] : Prevent redispatcher from selecting the same server

From: Willy Tarreau <w#1wt.eu>
Date: Sun, 10 Feb 2008 13:20:46 +0100


Hi Krzysztof,

I finally found some time to get back to userland :-) I've carefully inspected your patch. Initially I thought that the FWRR change was wrong but it's OK, it's my understanding which was wrong !

However, I'm not sure that I completely get the logic behind the changes in assign_server_and_queue() :

> @@ -1060,9 +1076,8 @@ int assign_server_and_queue(struct session *s)

>
> if (s->flags & SN_ASSIGNED) {
> if (s->srv && s->srv->maxqueue > 0 && s->srv->nbpend >= s->srv->maxqueue) {
> - s->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET); > - s->srv = NULL; > - http_flush_cookie_flags(&s->txn); > + /* it's left to the dispatcher to choose a server */ > + s->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
> } else {
> /* a server does not need to be assigned, perhaps because we're in
> * direct mode, or in dispatch or transparent modes where the server
> @@ -1081,7 +1096,22 @@ int assign_server_and_queue(struct session *s)
> }
>
> /* a server needs to be assigned */
> + srv = s->srv;
> err = assign_server(s);
> + > + if (srv && srv != s->srv) { > + /* This session was previously dispatched to another server: > + * - clear SN_DIRECT > + * - flush a cookie > + * - set SN_REDISP if it was successfully redispatched > + */ > + > + s->flags &= ~(SN_DIRECT); > + http_flush_cookie_flags(&s->txn); > + if (srv) > + s->flags |= SN_REDISP; > + } > + I think that the redispatch will not work anymore if we hit srv->maxqueue for a server : since the SN_DIRECT flag is not cleared anymore, assign_server() will return exactly the same server. Am I right or am I missing something ? Then, maybe we have the same problem here, and at every other place where the redispatch code gets touched : > @@ -1409,10 +1439,8 @@ int srv_retryable_connect(struct session *t)
> }
> t->be->redispatches++;
>
> - t->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET); > - t->flags |= SN_REDISP; > - t->srv = NULL; /* it's left to the dispatcher to choose a server */ > - http_flush_cookie_flags(&t->txn); > + /* it's left to the dispatcher to choose a server */ > + t->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
> return 0;
> }

etc...

Please try to explain me if I'm wrong. I think it was Einstein who said that it requires a smarter brain to solve a problem than to create it, which implies that if you do everything using all of your capacities, then you will only have unfixable problems everywhere :-) And I think I've reached that limit now !

BTW, If you think that the LB code needs refactoring, I'm open to discuss about it.

Regards,
Willy Received on 2008/02/10 13:20

This archive was generated by hypermail 2.2.0 : 2008/02/10 13:30 CET