From: Willy Tarreau <w@1wt.eu>
Date: Mon, 3 Oct 2022 12:56:34 +0000 (+0200)
Subject: BUG/MINOR: backend: only enforce turn-around state when not redispatching
X-Git-Tag: v2.7-dev7~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=406efb96d135efe1d5a85bf58c589f7b6dbd8c70;p=thirdparty%2Fhaproxy.git

BUG/MINOR: backend: only enforce turn-around state when not redispatching

In github issue #1878, Bart Butler reported observing turn-around states
(1 second pause) after connection retries going to different servers,
while this ought not happen.

In fact it does happen because back_handle_st_cer() enforces the TAR
state for any algo that's not round-robin. This means that even leastconn
has it, as well as hashes after the number of servers changed.

Prior to doing that, the call to stream_choose_redispatch() has already
had a chance to perform the correct choice and to check the algo and
the number of retries left. So instead we should just let that function
deal with the algo when needed (and focus on deterministic ones), and
let the former just obey. Bart confirmed that the fixed version works
as expected (no more delays during retries).

This may be backported to older releases, though it doesn't seem very
important. At least Bart would like to have it in 2.4 so let's go there
for now after it has cooked a few weeks in 2.6.
---

diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h
index f72a923ce4..06716aeeb1 100644
--- a/include/haproxy/stream.h
+++ b/include/haproxy/stream.h
@@ -334,7 +334,7 @@ static inline void stream_choose_redispatch(struct stream *s)
 	      ((s->be->redispatch_after < 0) &&
 	       (s->conn_retries % (s->be->conn_retries + 1 + s->be->redispatch_after) == 0))) ||
 	     (!(s->flags & SF_DIRECT) && s->be->srv_act > 1 &&
-	      ((s->be->lbprm.algo & BE_LB_KIND) == BE_LB_KIND_RR)))) {
+	      ((s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_HI)))) {
 		sess_change_server(s, NULL);
 		if (may_dequeue_tasks(objt_server(s->target), s->be))
 			process_srv_queue(objt_server(s->target));
diff --git a/src/backend.c b/src/backend.c
index 45e9eab412..4b1ce5ce3d 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -2412,7 +2412,6 @@ void back_handle_st_cer(struct stream *s)
 
 		/* only wait when we're retrying on the same server */
 		if ((sc->state == SC_ST_ASS ||
-		     (s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_RR ||
 		     (s->be->srv_act <= 1)) && !reused) {
 			sc->state = SC_ST_TAR;
 			s->conn_exp = tick_add(now_ms, MS_TO_TICKS(delay));