BUG/MEDIUM: stconn: Don't report rcv/snd expiration date if SC cannot epxire

When receive or send expiration date of a stream-connector is retrieved, we
now automatically check if it may expire. If not, TICK_ETERNITY is returned.

The expiration dates of the frontend and backend stream-connectors are used
to compute the stream expiration date. This operation is performed at 2
places: at the end of process_stream() and in sc_notify() if the stream is
not woken up.

With this patch, there is no special changes for process_stream() because it
was already handled. It make thing a little simpler. However, it fixes
sc_notify() by avoiding to erroneously compute an expiration date in
past. This highly reduce the stream wakeups when there is contention on the
consumer side.

The bug was introduced with the commit 8073094bf ("NUG/MEDIUM: stconn:
Always update stream's expiration date after I/O"). It was an error to
unconditionnaly set the stream expiration data, without testing blocking
conditions on both SC.

This patch must be backported to 2.8.

diff --git a/include/haproxy/sc_strm.h b/include/haproxy/sc_strm.h
index 8810d616a4a4af582b3b59221ef9cf83338a7e5c..98b803dd16baffaac43671e0fcda22fcbba91770 100644 (file)
--- a/include/haproxy/sc_strm.h
+++ b/include/haproxy/sc_strm.h
@@ -379,11 +379,25 @@ static inline int sc_snd_may_expire(const struct stconn *sc)
        return 1;
 }
 
+static forceinline int sc_ep_rcv_ex(const struct stconn *sc)
+{
+       return ((tick_isset(sc->sedesc->lra) && sc_rcv_may_expire(sc))
+               ? tick_add_ifset(sc->sedesc->lra, sc->ioto)
+               : TICK_ETERNITY);
+}
+
+static forceinline int sc_ep_snd_ex(const struct stconn *sc)
+{
+       return ((tick_isset(sc->sedesc->fsb) && sc_snd_may_expire(sc))
+               ? tick_add_ifset(sc->sedesc->fsb, sc->ioto)
+               : TICK_ETERNITY);
+}
+
 static inline void sc_check_timeouts(const struct stconn *sc)
 {
-       if (likely(sc_rcv_may_expire(sc)) && unlikely(tick_is_expired(sc_ep_rcv_ex(sc), now_ms)))
+       if (unlikely(tick_is_expired(sc_ep_rcv_ex(sc), now_ms)))
                sc_ic(sc)->flags |= CF_READ_TIMEOUT;
-       if (likely(sc_snd_may_expire(sc)) && unlikely(tick_is_expired(sc_ep_snd_ex(sc), now_ms)))
+       if (unlikely(tick_is_expired(sc_ep_snd_ex(sc), now_ms)))
                sc_oc(sc)->flags |= CF_WRITE_TIMEOUT;
 }
 

diff --git a/include/haproxy/stconn.h b/include/haproxy/stconn.h
index 4ba6aca20513a596bf046ce9705f18290e752721..3b24b217e5379106e6f94f2eeec04e6760ce3df3 100644 (file)
--- a/include/haproxy/stconn.h
+++ b/include/haproxy/stconn.h
@@ -210,20 +210,6 @@ static forceinline size_t sc_ep_ff_data(struct stconn *sc)
        return se_ff_data(sc->sedesc);
 }
 
-static forceinline int sc_ep_rcv_ex(const struct stconn *sc)
-{
-       return (tick_isset(sc->sedesc->lra)
-               ? tick_add_ifset(sc->sedesc->lra, sc->ioto)
-               : TICK_ETERNITY);
-}
-
-static forceinline int sc_ep_snd_ex(const struct stconn *sc)
-{
-       return (tick_isset(sc->sedesc->fsb)
-               ? tick_add_ifset(sc->sedesc->fsb, sc->ioto)
-               : TICK_ETERNITY);
-}
-
 /* Returns the stream endpoint from an connector, without any control */
 static inline void *__sc_endp(const struct stconn *sc)
 {

diff --git a/src/stream.c b/src/stream.c
index ef8d46625ea7970ed92643cbaa701dc0ab3a7f83..6a6166eea7b918bff5a027cad5093e88975ee09b 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -2518,17 +2518,6 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 
        update_exp_and_leave:
                /* Note: please ensure that if you branch here you disable SC_FL_DONT_WAKE */
-               t->expire = (tick_is_expired(t->expire, now_ms) ? 0 : t->expire);
-
-               if (likely(sc_rcv_may_expire(scf)))
-                       t->expire = tick_first(t->expire, sc_ep_rcv_ex(scf));
-               if (likely(sc_snd_may_expire(scf)))
-                       t->expire = tick_first(t->expire, sc_ep_snd_ex(scf));
-               if (likely(sc_rcv_may_expire(scb)))
-                       t->expire = tick_first(t->expire, sc_ep_rcv_ex(scb));
-               if (likely(sc_snd_may_expire(scb)))
-                       t->expire = tick_first(t->expire, sc_ep_snd_ex(scb));
-
                if (!req->analysers)
                        req->analyse_exp = TICK_ETERNITY;
                if (!res->analysers)
@@ -2538,10 +2527,13 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                          (!tick_isset(req->analyse_exp) || tick_is_expired(req->analyse_exp, now_ms)))
                        req->analyse_exp = tick_add(now_ms, 5000);
 
+               t->expire = (tick_is_expired(t->expire, now_ms) ? 0 : t->expire);
+               t->expire = tick_first(t->expire, sc_ep_rcv_ex(scf));
+               t->expire = tick_first(t->expire, sc_ep_snd_ex(scf));
+               t->expire = tick_first(t->expire, sc_ep_rcv_ex(scb));
+               t->expire = tick_first(t->expire, sc_ep_snd_ex(scb));
                t->expire = tick_first(t->expire, req->analyse_exp);
-
                t->expire = tick_first(t->expire, res->analyse_exp);
-
                t->expire = tick_first(t->expire, s->conn_exp);
 
                if (unlikely(tick_is_expired(t->expire, now_ms))) {