Re: redispatch optimization

[Krzysztof Oledzki]

On Mon, 31 Aug 2009, Dmitry Sivachenko wrote:


Hello!

Hello,


If we are running with 'option redispatch' and
'retries' parameter set to some positive value,
the behaviur is as follows:

###
In order to avoid immediate reconnections to a server which is restarting,
 a turn-around timer of 1 second is applied before a retry occurs.

When option redispatch is set, the last retry may be performed on another
server even if a cookie references a different server.
###

While is makes sence to wait for some time (1 second)
before attempting another connection to the same
server, there is no reason to wait 1 second before
attempting the last connection to another server
(with option redispatch).  It is just a waste of one
second.

Please consider the following patch to attempt last try
immediately. (If our main server does not respond, there is
no reason to assume another one cant answer now).


Very clever idea, however it is no certain that we would not end up on the 
same sever anyway, so this may not always work.



PS: another important suggestion is to make that delay tunable
parameter (like timeout.connect, etc), rather than hardcode
1000ms in code.


Why would you like to change the value? I found 1s very well chosen.
Quoting Willy:

Initially, I wanted to reuse the contimeout as the turn-around timeout, 
but there are people using very large contimeouts because it was the 
same value which dictated the tarpit and queue timeouts in previous 
versions, so this would result in reconnection attempts being delayed 
too long. So finally I arbitrarily selected a one second delay to 
attempt a reconnection after a failure.


If this value is considered too low by some users and too high by 
others, then we'll introduce timeout reconnect  :-)




--- work/haproxy-1.4-dev2/src/session.c 2009-08-10 00:57:09.0 +0400
+++ /tmp/session.c  2009-08-31 14:28:26.0 +0400
@@ -306,7 +306,11 @@ int sess_update_st_cer(struct session *s
   si-err_type = SI_ET_CONN_ERR;

   si-state = SI_ST_TAR;
+   if (s-srv  s-conn_retries == 0  s-be-options  PR_O_REDISP) {
+   si-exp = tick_add(now_ms, MS_TO_TICKS(0));
+   } else {
   si-exp = tick_add(now_ms, MS_TO_TICKS(1000));
+   }
   return 0;
   }
   return 0;



There is no value in adding 0ms, also SI_ST_TAR should be moved inside the 
condition I think, not sure if it is enough.


Best regards,

Krzysztof Olędzki

Re: redispatch optimization

[Dmitry Sivachenko]

On Mon, Aug 31, 2009 at 03:39:35PM +0200, Krzysztof Oledzki wrote:
  PS: another important suggestion is to make that delay tunable
  parameter (like timeout.connect, etc), rather than hardcode
  1000ms in code.
 
 Why would you like to change the value? I found 1s very well chosen.

In our environment we have some program asking balancer and expecting results
to be returned very fast (say, in 0.5 second maximum).

So I want to ask one server in the backend, and, if it is not responding, 
re-ask another one immediately (or even the same once again, assuming
that just first TCP SETUP packet was lost and server is
running normally).  So I use low connect.timeout (say, 30ms) and if
connection fails i retry the same one once more.

After all, we can use 1 second default and allow to customize that
value when needed.


 
 
 
  --- work/haproxy-1.4-dev2/src/session.c 2009-08-10 00:57:09.0 +0400
  +++ /tmp/session.c  2009-08-31 14:28:26.0 +0400
  @@ -306,7 +306,11 @@ int sess_update_st_cer(struct session *s
 si-err_type = SI_ET_CONN_ERR;
 
 si-state = SI_ST_TAR;
  +   if (s-srv  s-conn_retries == 0  s-be-options  PR_O_REDISP) 
  {
  +   si-exp = tick_add(now_ms, MS_TO_TICKS(0));
  +   } else {
 si-exp = tick_add(now_ms, MS_TO_TICKS(1000));
  +   }
 return 0;
 }
 return 0;
 
 
 There is no value in adding 0ms, also SI_ST_TAR should be moved inside the 
 condition I think, not sure if it is enough.
 

Okay probably it is ugly implementation (though it works), because I 
still dont completely understand the code.
Feel free to re-implement it in better way, just grab the idea.

Thanks.