]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: mux-h2: do not apply timer on idle backend connection
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 15 Apr 2025 16:06:54 +0000 (18:06 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 17 Apr 2025 12:49:36 +0000 (14:49 +0200)
Since the following commit, MUX H2 timeout function has been slightly
exetended.

  d38d8c6ccb189e7bc813b3693fec3093c9be55f1
  BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout

A side-effect of this patch is that now backend idle connection expire
timer is not reset if already defined. This means that if a timer was
registered prior to the connection transition to idle, the connection
would be destroyed on its timeout. If this happens for enough
connection, this may have an impact on the reuse rate.

In practice, this case should be rare, as h2c timer is set to
TICK_ETERNITY while there is active streams. The timer is not refreshed
most of the time before going the transition to idle, so the connection
won't be deleted on expiration.

The only case where it could occur is if there is still pending data
blocked on emission on stream detach. Here, timeout server is applied on
the connection. When the emission completes, the connection goes to
idle, but the timer will still armed, and thus will be triggered on the
idle connection.

To prevent this, explicitely reset h2c timer to TICK_ETERNITY for idle
backend connection via h2c_update_timeout().

This patch is explicitely not scheduled for backport for now, as it is
difficult to estimate the real impact of the previous code state.

src/mux_h2.c

index 1de8cb2637ccedec35a1fc7d614c7044ab0ab4f3..6fb76aec79cd94d491349c1af5d777b0c4db845a 100644 (file)
@@ -863,6 +863,10 @@ static void h2c_update_timeout(struct h2c *h2c)
                                int exp = tick_add_ifset(now_ms, h2c->shut_timeout);
 
                                h2c->task->expire = tick_first(h2c->task->expire, exp);
+                               /* if a timeout above was not set, fall back to the default one */
+                               if (!tick_isset(h2c->task->expire))
+                                       h2c->task->expire = tick_add_ifset(now_ms, h2c->timeout);
+
                                is_idle_conn = 1;
                        }
                        else if (!(h2c->flags & H2_CF_IS_BACK)) {
@@ -880,12 +884,16 @@ static void h2c_update_timeout(struct h2c *h2c)
                                }
 
                                h2c->task->expire = tick_add_ifset(h2c->idle_start, to);
+                               /* if a timeout above was not set, fall back to the default one */
+                               if (!tick_isset(h2c->task->expire))
+                                       h2c->task->expire = tick_add_ifset(now_ms, h2c->timeout);
+
                                is_idle_conn = 1;
                        }
-
-                       /* if a timeout above was not set, fall back to the default one */
-                       if (!tick_isset(h2c->task->expire))
-                               h2c->task->expire = tick_add_ifset(now_ms, h2c->timeout);
+                       else {
+                               /* No timeout on backend idle conn. */
+                               h2c->task->expire = TICK_ETERNITY;
+                       }
                }
 
                if ((h2c->proxy->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) &&