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 <[email protected]> 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
