Hi Joseph,
On Thu, May 14, 2015 at 09:06:23AM -0700, Joseph Lynch wrote:
> The attached patch allows HAProxy to redispatch on retry intervals,
> e.g. every retry (1), on the last retry (-1), or every 3rd retry (3).
> The idea is to not have to wait for the final retry before
> redispatching.
>
> I moved the order of the comparisons around a little bit to ensure
> that the redispatch_after variable would be defined (namely if
> PR_O_REDISP is set then redispatch_after must have been set to -1).
I think that we'll be able to get rid of the option after that, as
a cleanup resulting from your work.
> I
> did notice that with round robin load balancing we can immediately
> redispatch due to the other branch on the || conditional in the status
> quo, is that intentional? For testing I had to use leastconn.
That reminded me something, so I looked and found this commit which
did this :
commit 33a14e515bdbb02d8193bd870f3b659243a851f9
Author: Willy Tarreau
Date: Fri Jun 13 17:49:40 2014 +0200
MEDIUM: session: redispatch earlier when possible
As discussed with Dmitry Sivachenko, is a server farm has more than one
active server, uses a guaranteed non-determinist algorithm (round robin),
and a connection was initiated from a non-persistent connection, there's
no point insisting to reconnect to the same server after a connect failure,
better redispatch upon the very first retry instead of insisting on the same
server multiple times.
So yes, it's intentional.
> Testing:
> I added a debug line inside the redispatch block, failed 127.0.0.2 and
> then launched some curls against all the frontends. From the debug
> output and the stat interface the option appears to work. If you would
> like me to do more extensive testing let me know and I'm happy to do
> it.
I think your tests are OK. However I don't see the redispatch_after value
being pre-initialized from the defaults value, so I suspect it does not
work across defaults sections.
Please just check for "conn_retries" in cfgparse.c, everywhere it is
assigned, your new variable must be set as well. Please double-check
this and after that I think it's OK for merging.
Thanks,
Willy
