From ecb40b2c388dffcc9998a92df536ec7419ed8fed Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 1 Sep 2022 19:58:58 +0200 Subject: [PATCH] MINOR: backend: always satisfy the first req reuse rule with l7 retries The "first req" rule consists in not delivering a connection's first request to a connection that's not known for being safe so that we don't deliver a broken page to a client if the server didn't intend to keep it alive. That's what's used by "http-reuse safe" particularly. But the reason this rule was created was precisely because haproxy was not able to re-emit the request to the server in case of connection breakage, which is precisely what l7 retries later brought. As such, there's no reason for enforcing this rule when l7 retries are properly enabled because such a blank page will trigger a retry and will not be delivered to the client. This patch simply checks that the l7 retries are enabled for the 3 cases that can be triggered on a dead or dying connection (failure, empty, and timeout), and if all 3 are enabled, then regular idle connections can be reused. This could almost be marked as a bug fix because a lot of users relying on l7 retries do not necessarily think about using http-reuse always due to the recommendation against it in the doc, while the protection that the safe mode offers is never used in that mode, and it forces the http client not to reuse existing persistent connections since it never sets the "not first" flag. It could also be decided that the protection is not used either when the origin is an applet, as in this case this is internal code that we can decide to let handle the retry by itself (all info are still present). But at least the httpclient will be happy with this alone. It would make sense to backport this at least to 2.6 in order to let the httpclient reuse connections, maybe to older releases if some users report low reuse counts. --- src/backend.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend.c b/src/backend.c index 672604601..dc30c7627 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1452,10 +1452,12 @@ static int connect_server(struct stream *s) const int not_first_req = s->txn && s->txn->flags & TX_NOT_FIRST; const int idle = srv->curr_idle_nb > 0; const int safe = srv->curr_safe_nb > 0; + const int retry_safe = (s->be->retry_type & (PR_RE_CONN_FAILED | PR_RE_DISCONNECTED | PR_RE_TIMEOUT)) == + (PR_RE_CONN_FAILED | PR_RE_DISCONNECTED | PR_RE_TIMEOUT); /* second column of the tables above, * search for an idle then safe conn */ - if (not_first_req) { + if (not_first_req || retry_safe) { if (idle || safe) srv_conn = conn_backend_get(s, srv, 0, hash); }