From 731c8e6cf9a89a17d58a626dd409cb3991c1f40c Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Tue, 29 Mar 2022 16:08:44 +0200 Subject: [PATCH] MINOR: stream: Simplify retries counter calculation The conn_retries counter was set to the max value and decremented at each connection retry. Thus the counter reflected the number of retries left and not the real number of retries. All calculations of redispatch or reporting of number of retries experienced were made using subtracts from the configured retries, which was complicated and didn't bring any benefit. Now, this counter is set to 0 and incremented at each retry. We know we've reached the maximum allowed connection retries by comparing it to the configured value. In all other cases, we directly use the counter. This patch should address the feature request #1608. --- include/haproxy/stream-t.h | 2 +- include/haproxy/stream.h | 6 ++---- src/backend.c | 10 +++++----- src/http_ana.c | 4 ++-- src/log.c | 5 +---- src/stream.c | 2 +- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/include/haproxy/stream-t.h b/include/haproxy/stream-t.h index 0fb5e7a8b9..2b65ea0aff 100644 --- a/include/haproxy/stream-t.h +++ b/include/haproxy/stream-t.h @@ -139,7 +139,7 @@ struct stream { int16_t priority_class; /* priority class of the stream for the pending queue */ int32_t priority_offset; /* priority offset of the stream for the pending queue */ - int conn_retries; /* number of connect retries left */ + int conn_retries; /* number of connect retries performed */ struct list list; /* position in the thread's streams list */ struct mt_list by_srv; /* position in server stream list */ diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h index 93093f74e3..1983230146 100644 --- a/include/haproxy/stream.h +++ b/include/haproxy/stream.h @@ -331,11 +331,9 @@ static inline void stream_choose_redispatch(struct stream *s) (s->be->options & PR_O_REDISP) && !(s->flags & SF_FORCE_PRST) && ((__objt_server(s->target)->cur_state < SRV_ST_RUNNING) || (((s->be->redispatch_after > 0) && - ((s->be->conn_retries - s->conn_retries) % - s->be->redispatch_after == 0)) || + (s->conn_retries % s->be->redispatch_after == 0)) || ((s->be->redispatch_after < 0) && - ((s->be->conn_retries - s->conn_retries) % - (s->be->conn_retries + 1 + 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)))) { sess_change_server(s, NULL); diff --git a/src/backend.c b/src/backend.c index d2d04d986f..c73b23e785 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1720,8 +1720,7 @@ static int connect_server(struct stream *s) * it's our first try */ ((cli_conn->flags & CO_FL_EARLY_DATA) || - ((s->be->retry_type & PR_RE_EARLY_ERROR) && - s->conn_retries == s->be->conn_retries)) && + ((s->be->retry_type & PR_RE_EARLY_ERROR) && !s->conn_retries)) && !channel_is_empty(cs_oc(s->csb)) && srv_conn->flags & CO_FL_SSL_WAIT_HS) srv_conn->flags &= ~(CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN); @@ -2222,6 +2221,8 @@ void back_handle_st_cer(struct stream *s) cs->si->exp = TICK_ETERNITY; cs->si->flags &= ~SI_FL_EXP; + s->conn_retries++; + /* we probably have to release last stream from the server */ if (objt_server(s->target)) { struct connection *conn = cs_conn(cs); @@ -2251,14 +2252,13 @@ void back_handle_st_cer(struct stream *s) * provided by the client and we don't want to let the * client provoke retries. */ - s->conn_retries = 0; + s->conn_retries = s->be->conn_retries; DBG_TRACE_DEVEL("Bad SSL cert, disable connection retries", STRM_EV_STRM_PROC|STRM_EV_SI_ST|STRM_EV_STRM_ERR, s); } } /* ensure that we have enough retries left */ - s->conn_retries--; - if (s->conn_retries < 0 || !(s->be->retry_type & PR_RE_CONN_FAILED)) { + if (s->conn_retries >= s->be->conn_retries || !(s->be->retry_type & PR_RE_CONN_FAILED)) { if (!cs->si->err_type) { cs->si->err_type = SI_ET_CONN_ERR; } diff --git a/src/http_ana.c b/src/http_ana.c index 7a72a06773..2c93edbd20 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -1226,8 +1226,8 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) struct channel *req, *res; int co_data; - s->conn_retries--; - if (s->conn_retries < 0) + s->conn_retries++; + if (s->conn_retries >= s->be->conn_retries) return -1; if (objt_server(s->target)) { diff --git a/src/log.c b/src/log.c index c3b7d92baf..edffcbdb75 100644 --- a/src/log.c +++ b/src/log.c @@ -2616,10 +2616,7 @@ int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t case LOG_FMT_RETRIES: // %rq if (s_flags & SF_REDISP) LOGCHAR('+'); - ret = ltoa_o(((s && s->conn_retries > 0) - ? (be->conn_retries - s->conn_retries) - : ((s && cs_si(s->csb)->state != SI_ST_INI) ? be->conn_retries : 0)), - tmplog, dst + maxsize - tmplog); + ret = ltoa_o((s ? s->conn_retries : 0), tmplog, dst + maxsize - tmplog); if (ret == NULL) goto out; tmplog = ret; diff --git a/src/stream.c b/src/stream.c index 71460fe87a..200f69efd7 100644 --- a/src/stream.c +++ b/src/stream.c @@ -2161,7 +2161,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) * perform a connection request. */ si_b->state = SI_ST_REQ; /* new connection requested */ - s->conn_retries = s->be->conn_retries; + s->conn_retries = 0; if ((s->be->retry_type &~ PR_RE_CONN_FAILED) && (s->be->mode == PR_MODE_HTTP) && !(s->txn->flags & TX_D_L7_RETRY))