On Tue, 2026-11-03 at 16:23 +0100, Amaury Denoyelle wrote:
> Thanks for your report. It's still good to see some interest in
reverse
> HTTP. At first glance, I am mostly okay with your patch. You
mentionned
> the GOAWAY emission as a possibility, but I think there is no real
> interest from an HTTP/2 client point of view, so closing the
connection
> using srv_cleanup_connections() seems the correct approach.
Thanks for looking at it.
Closing is enough for an idle connection, but it cannot reach a busy
one. The 16 concurrent requests case is that one: every connection
carries a stream, so nothing is idle at the reload, and the new process
gets no connection from that peer until the old streams end. With
long-running requests that is the common case for us.
A GOAWAY on those busy connections would tell the dialer at the reload
that the connection is going away, while its streams still finish.
What the dialer does with it is up to the dialer, but today it never
gets the chance: the passive side never sends one. Some dialers would
use it as is. The oktalz/reverse-http library already dials a
replacement as soon as it receives a graceful GOAWAY and drains the old
connection, and we intend to do the same in ours.
HAProxy's own active side would need a small change to benefit as
well, as far as I can read it. h2c_handle_goaway() records last_sid and
wakes the streams, and the drain itself works, since a reversed
frontend connection has reached its last stream with last_sid 0. But
the preconnect slot is only released in listener_release() when the
connection ends, so rhttp_accept_conn() opens the replacement only
after the drain. Releasing the nbconn slot when a GOAWAY is received on
a reversed connection would let it preconnect a replacement while the
old connection drains.
Sending it is the part I got wrong in my first look. Putting reversed
connections back on the stopping list would not help: the soft-stop
GOAWAY in h2_process() is skipped for H2_CF_IS_BACK connections, and
mux_stopping_process() runs once per thread when it first sees
"stopping", which with "grace" is before do_soft_stop_now(). So it
would need that guard to admit reversed backend connections, and a
wake of the busy ones when their backend is actually stopped. I do not
see a clean way to find the busy reversed connections of a server per
thread at that point. Do you have a preferred approach, or would you
rather handle it differently? I am happy to work on it if you think it
is worth having.
> In fact, I wonder if your patch should be generalized to all
backends,
> regardless of reverse HTTP usage. Idle connections are already
cleaned
> up when a server is put in maintenance mode, and
srv_add_to_idle_list()
> already has a condition to reject idle connections in this case. A
> disabled backend is quite similar to a server in maintenance. Of
course,
> the main benefit would still be for reverse HTTP case, but even in
> normal case it could be useful to release connections faster on
legacy
> worker processes. I have to perform some tests and I'll let you know
my
> observations.
Agreed, and done in v2, sent as a new thread: every server's idle
connections are released in do_soft_stop_now(), and any stopped
backend refuses connections back into its idle list, next to the
maintenance check.
In case it helps your testing, here is what I ran on master at
a2175623a, with v2:
reg-tests (default,bug,devel), master and v2 alike:
327 passed, 42 skipped, 1 failed (ssl/wrong_ctx_storage.vtc, which
fails the same way on master in my container; its shell step exits
127, so I take it for a missing tool there)
reverse HTTP peer (oktalz/reverse-http, 4 connections), reload while
requests are in flight, polling a fast request every 100ms:
one 8s request master: 503 on 70/71 polls until it ends
v2: served again after 1.2-1.3s (2 runs)
one 20s request v2: served again after 1.2s
16 concurrent 8s v2: 503 on 69/70 polls until they end
one 8s, grace 3s v2: served again after 4.3s
Cheers,
Dustin Lundquist
