]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: streams: Don't redispatch with L7 retries if redispatch isn't set.
authorOlivier Houchard <ohouchard@haproxy.com>
Fri, 12 Jul 2019 14:16:59 +0000 (16:16 +0200)
committerOlivier Houchard <cognet@ci0.org>
Fri, 12 Jul 2019 14:17:50 +0000 (16:17 +0200)
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
src/proto_htx.c
src/stream.c

index 60338c9974dec1fc6c020f04dfd6481b651e85b0..e94e32bd4ddb9a8ca534a8f5d6d553cb60994e30 100644 (file)
@@ -28,6 +28,8 @@
 #include <types/stream.h>
 #include <proto/fd.h>
 #include <proto/freq_ctr.h>
+#include <proto/obj_type.h>
+#include <proto/queue.h>
 #include <proto/stick_table.h>
 #include <proto/task.h>
 
@@ -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);
 
index cbe5c84e0c4612fa4d5983ddf80b8faeaeec3a1c..59b0ff975ac503a4373ba3ebebe911d4c0f79b6a 100644 (file)
@@ -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 */
index 69befbdd08635a73b4c9ee7653470d7ab47f6015..a5c5f45c77ab5f8176896cf6809a558ea1607725 100644 (file)
@@ -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