From 2b1d91758d74f53df3b9a1dbb1d8e627320b9532 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 17 Jun 2021 15:14:49 +0200 Subject: [PATCH] BUG/MINOR: backend: restore the SF_SRV_REUSED flag original purpose The SF_SRV_REUSED flag was set if a stream reused a backend connection. One of its purpose is to count the total reuse on the backend in opposition to newly instantiated connection. However, the flag was diverted from its original purpose since the following commit : e8f5f5d8b228d71333fb60229dc908505baf9222 BUG/MEDIUM: servers: Only set SF_SRV_REUSED if the connection if fully ready. With this change, the flag is not set anymore if the mux is not ready when a connection is picked for reuse. This can happen for multiplexed connections which are inserted in the available list as soon as created in http-reuse always mode. The goal of this change is to not retry immediately this request in case on an error on the same server if the reused connection is not fully ready. This change is justified for the retry timeout handling but it breaks other places which still uses the flag for its original purpose. Mainly, in this case the wrong 'connect' backend counter is incremented instead of the 'reuse' one. The flag is also used in http_return_srv_error and may have an impact if a http server error is replied for this stream. To fix this problem, the original purpose of the flag is restored by setting it unconditionaly when a connection is reused. Additionally, a new flag SF_SRV_REUSED_ANTICIPATED is created. This flag is set when the connection is reused but the mux is not ready yet. For the timeout handling on error, the request is retried immediately only if the stream reused a connection without this newly anticipated flag. This must be backported up to 2.1. --- include/haproxy/stream-t.h | 1 + src/backend.c | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/haproxy/stream-t.h b/include/haproxy/stream-t.h index 9499e94d7..e5482a2e8 100644 --- a/include/haproxy/stream-t.h +++ b/include/haproxy/stream-t.h @@ -88,6 +88,7 @@ #define SF_IGNORE_PRST 0x00080000 /* ignore persistence */ #define SF_SRV_REUSED 0x00100000 /* the server-side connection was reused */ +#define SF_SRV_REUSED_ANTICIPATED 0x00200000 /* the connection was reused but the mux is not ready yet */ /* flags for the proxy of the master CLI */ diff --git a/src/backend.c b/src/backend.c index 35cf72787..638280581 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1591,16 +1591,15 @@ skip_reuse: } } else { + s->flags |= SF_SRV_REUSED; + /* Currently there seems to be no known cases of xprt ready * without the mux installed here. */ BUG_ON(!srv_conn->mux); - /* Only consider we're doing reuse if the connection was - * ready. - */ - if (srv_conn->mux->ctl(srv_conn, MUX_STATUS, NULL) & MUX_STATUS_READY) - s->flags |= SF_SRV_REUSED; + if (!(srv_conn->mux->ctl(srv_conn, MUX_STATUS, NULL) & MUX_STATUS_READY)) + s->flags |= SF_SRV_REUSED_ANTICIPATED; } /* flag for logging source ip/port */ @@ -2273,6 +2272,8 @@ void back_handle_st_cer(struct stream *s) */ int delay = 1000; + const int reused = (s->flags & SF_SRV_REUSED) && + !(s->flags & SF_SRV_REUSED_ANTICIPATED); if (s->be->timeout.connect && s->be->timeout.connect < delay) delay = s->be->timeout.connect; @@ -2283,7 +2284,7 @@ void back_handle_st_cer(struct stream *s) /* only wait when we're retrying on the same server */ if ((si->state == SI_ST_ASS || (s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_RR || - (s->be->srv_act <= 1)) && !(s->flags & SF_SRV_REUSED)) { + (s->be->srv_act <= 1)) && !reused) { si->state = SI_ST_TAR; si->exp = tick_add(now_ms, MS_TO_TICKS(delay)); }