On Wed, Nov 04, 2026 at 11:37:42AM -0800, Dustin Lundquist wrote: > 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.
Willy and I just had a meeting this afternoon about the issue of rHTTP and soft-stop, and your proposed patch. See my remarks below. > 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. It is probably not desirable to kill every idle connections, as this could cause 503s for clients still connected to the older worker process. Thus, a middleground approach would be to kill at most half of the connections currently present on the server. Of course, this is still limited to idle connections only. Note that srv_migrate_conns_to_remove() already has a parameter to limit the purged entries. Regarding the case where every connection is active, we have to change srv_add_to_idle_list() as you already tried. Maybe the simplest solution would be to take again into account the estimated number of needed connections, which is normally ignored for rHTTP, in case soft stop has been set. The general idea is to ensure that the transition from the older to the newer worker process is as smooth as possible and won't generate too many 503s. Also, it's not desirable to implement connection closure directly in do_soft_stop_now() by looping over each proxies, as it could possibly trigger the watchdog with a pretty large configs. This work can instead be moved directly in manage_proxy() which is the proxy internal task handler. This is already automatically called on soft stop. However, this requires extra care to ensure that the rHTTP related operation is only performed once per proxy, as manage_proxy() can be called multiple times. Finally, a nice addition would be to return ACT_RET_DENY in tcp_action_attach_srv() in case soft stop is active. This should ensure the dialer won't accidentaly reopen new connections to the legacy worker process. > 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. It's really interesting that you mentionned that as we talked exactly about it for haproxy as a dialer too. However, this part must be implemented with lot of care in H2 to ensure it does not break standard connections as well. > 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. That sounds fine to me. > 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. Maybe the solution is to call the wake H2 handler for rHTTP connection when entering soft stop and then implement the GOAWAY emission in h2_process(). > > 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. Willy told me that it was my idea to generalize it to every proxies is incorrect. Clients still connected to the legacy worker process must have the time to finish their connection, which includes the possibility to open new streams. My approach would prevent this, so we should go back to your first patch version which was specific to rHTTP servers. To conclude, if you want I let you propose a patch that implement the connection closure based on the mechanism described in the first part of this email. If you do not have the time I can provide it myself. If this performs well with your tests and the changes are not too invasive, this solution could be backported. Once this part is done, we can then try to implement the GOAWAY emission, which is enticing but probably a little harder and with more impact on H2 code. Regards, -- Amaury Denoyelle
