From 4bd58676275d557a654979948f587aaae8e15c0d Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Fri, 12 Jul 2019 16:16:59 +0200 Subject: [PATCH] BUG/MEDIUM: streams: Don't redispatch with L7 retries if redispatch isn't set. Move the logic to decide if we redispatch to a new server from sess_update_st_cer() to a new inline function, stream_choose_redispatch(), and use it in do_l7_retry() instead of just setting the state to SI_ST_REQ. That way, when using L7 retries, we won't redispatch the request to another server except if "option redispatch" is used. This should be backported to 2.0. --- include/proto/stream.h | 44 ++++++++++++++++++++++++++++++++++++++++++ src/proto_htx.c | 4 ++-- src/stream.c | 36 +--------------------------------- 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/include/proto/stream.h b/include/proto/stream.h index 60338c997..e94e32bd4 100644 --- a/include/proto/stream.h +++ b/include/proto/stream.h @@ -28,6 +28,8 @@ #include #include #include +#include +#include #include #include @@ -351,6 +353,48 @@ static inline void stream_init_srv_conn(struct stream *sess) LIST_INIT(&sess->by_srv); } +static inline void stream_choose_redispatch(struct stream *s) +{ + struct stream_interface *si = &s->si[1]; + + /* If the "redispatch" option is set on the backend, we are allowed to + * retry on another server. By default this redispatch occurs on the + * last retry, but if configured we allow redispatches to occur on + * configurable intervals, e.g. on every retry. In order to achieve this, + * we must mark the stream unassigned, and eventually clear the DIRECT + * bit to ignore any persistence cookie. We won't count a retry nor a + * redispatch yet, because this will depend on what server is selected. + * If the connection is not persistent, the balancing algorithm is not + * determinist (round robin) and there is more than one active server, + * we accept to perform an immediate redispatch without waiting since + * we don't care about this particular server. + */ + if (objt_server(s->target) && + (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 - si->conn_retries) % + s->be->redispatch_after == 0)) || + ((s->be->redispatch_after < 0) && + ((s->be->conn_retries - si->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); + if (may_dequeue_tasks(objt_server(s->target), s->be)) + process_srv_queue(objt_server(s->target)); + + s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET); + si->state = SI_ST_REQ; + } else { + if (objt_server(s->target)) + _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.retries, 1); + _HA_ATOMIC_ADD(&s->be->be_counters.retries, 1); + si->state = SI_ST_ASS; + } + +} + void service_keywords_register(struct action_kw_list *kw_list); void list_services(FILE *out); diff --git a/src/proto_htx.c b/src/proto_htx.c index cbe5c84e0..59b0ff975 100644 --- a/src/proto_htx.c +++ b/src/proto_htx.c @@ -1406,13 +1406,13 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) res->flags &= ~(CF_READ_ERROR | CF_READ_TIMEOUT | CF_SHUTR | CF_EOI | CF_READ_NULL | CF_SHUTR_NOW); res->analysers = 0; si->flags &= ~(SI_FL_ERR | SI_FL_EXP | SI_FL_RXBLK_SHUT); - si->state = SI_ST_REQ; + stream_choose_redispatch(s); si->exp = TICK_ETERNITY; res->rex = TICK_ETERNITY; res->to_forward = 0; res->analyse_exp = TICK_ETERNITY; res->total = 0; - s->flags &= ~(SF_ASSIGNED | SF_ADDR_SET | SF_ERR_SRVTO | SF_ERR_SRVCL); + s->flags &= ~(SF_ERR_SRVTO | SF_ERR_SRVCL); si_release_endpoint(&s->si[1]); b_free(&req->buf); /* Swap the L7 buffer with the channel buffer */ diff --git a/src/stream.c b/src/stream.c index 69befbdd0..a5c5f45c7 100644 --- a/src/stream.c +++ b/src/stream.c @@ -770,41 +770,7 @@ static void sess_update_st_cer(struct stream *s) return; } - /* If the "redispatch" option is set on the backend, we are allowed to - * retry on another server. By default this redispatch occurs on the - * last retry, but if configured we allow redispatches to occur on - * configurable intervals, e.g. on every retry. In order to achieve this, - * we must mark the stream unassigned, and eventually clear the DIRECT - * bit to ignore any persistence cookie. We won't count a retry nor a - * redispatch yet, because this will depend on what server is selected. - * If the connection is not persistent, the balancing algorithm is not - * determinist (round robin) and there is more than one active server, - * we accept to perform an immediate redispatch without waiting since - * we don't care about this particular server. - */ - if (objt_server(s->target) && - (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 - si->conn_retries) % - s->be->redispatch_after == 0)) || - ((s->be->redispatch_after < 0) && - ((s->be->conn_retries - si->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); - if (may_dequeue_tasks(objt_server(s->target), s->be)) - process_srv_queue(objt_server(s->target)); - - s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET); - si->state = SI_ST_REQ; - } else { - if (objt_server(s->target)) - _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.retries, 1); - _HA_ATOMIC_ADD(&s->be->be_counters.retries, 1); - si->state = SI_ST_ASS; - } + stream_choose_redispatch(s); if (si->flags & SI_FL_ERR) { /* The error was an asynchronous connection error, and we will