A connection attached with attach-srv is removed from the stopping list when it is reversed, and the soft-stop GOAWAY is only sent on frontend connections, so a stopping process never tells a reverse HTTP peer to leave. The connection is not a job either, so the process keeps it open until its last client stream ends. The peer sees no reason to redial, while the new process has no connection to it: requests to that server get a 503 for as long as the old process lingers, which with long-running requests and no hard-stop-after can be minutes.
Once the backends are stopped in do_soft_stop_now(), release the idle connections of every rhttp server, and refuse a reversed connection back into the idle list of a stopped backend, so one still carrying streams is closed as soon as its last stream ends. Each closure prompts the peer to redial into the new process. Gating on the stopped backend rather than on "stopping" keeps reverse connections usable during a "grace" period, while this process still serves. This only helps connections that are idle at the reload. One that carries a stream is still kept until its streams end, and the peer is not told to replace it meanwhile, so a pool whose connections are all busy is not improved. That needs a GOAWAY on busy reverse connections. Measured with pool-purge-delay 3600s and one 8s or 20s request in flight across a reload: requests to the server are served again after the peer's reconnect delay (1.1-1.2s for a peer with a 1.0-1.5s backoff, 0.44-0.56s for one that redials after 0.5s), against the whole remaining life of the in-flight request without this patch. The in-flight request still completes. With 16 concurrent requests over 4 connections, nothing is idle and the result is unchanged. This partially addresses GitHub issue #3556. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- Notes: This is the partial fix described in GitHub issue #3556, which has the reproduction (oktalz/reverse-http behind its recommended configuration) and the measurements. It applies to master at 92f34653b. On that tree: one 8s request in flight unpatched: 503 on 70/71 polls until it ends patched: served again 1.1s after the reload 16 concurrent 8s requests patched: 503 on 69/70 polls (no idle connection) one 8s request, grace 3s patched: served again 4.4s after the reload The busy case is the one that matters for long-running work: every connection carries a stream, so nothing here is closed. Sending a GOAWAY on busy reverse connections at soft-stop would let the peer open replacements while those streams finish. I did not do that for idle ones because they can be taken over by another thread while the stopping list is per thread and unlocked. Busy ones are not taken over, so that may be workable, and I would welcome guidance on the right place for it. include/haproxy/server.h | 1 + src/proxy.c | 15 +++++++++++++++ src/server.c | 4 ++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 23d0c0d72..b299cc1bf 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -112,6 +112,7 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code); struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char *ip_family); struct task *srv_cleanup_idle_conns(struct task *task, void *ctx, unsigned int state); void srv_release_conn(struct server *srv, struct connection *conn); +void srv_cleanup_connections(struct server *srv); struct connection *srv_lookup_conn(struct ceb_root **tree, uint64_t hash); struct connection *srv_lookup_conn_next(struct ceb_root **tree, struct connection *conn); diff --git a/src/proxy.c b/src/proxy.c index 2601d8b29..39a923dd7 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -58,6 +58,7 @@ #include <haproxy/quic_tp.h> #include <haproxy/quic_tune.h> #include <haproxy/server-t.h> +#include <haproxy/server.h> #include <haproxy/signal.h> #include <haproxy/stats.h> #include <haproxy/stconn.h> @@ -4154,6 +4155,20 @@ static void do_soft_stop_now() HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock); } + /* Reverse HTTP peers only redial once their idle connection to this + * stopping process is closed, and nothing else would close it before + * the process exits. Busy ones are closed on release instead (see + * srv_add_to_idle_list()). + */ + list_for_each_entry(p, &main_proxies, el) { + struct server *srv; + + list_for_each_entry(srv, &p->servers, el_px) { + if (srv->flags & SRV_F_RHTTP) + srv_cleanup_connections(srv); + } + } + /* signal zero is used to broadcast the "stopping" event */ signal_handler(0); } diff --git a/src/server.c b/src/server.c index 50c7771ce..6022eb049 100644 --- a/src/server.c +++ b/src/server.c @@ -61,7 +61,6 @@ static void srv_update_status(struct server *s, int type, int cause); static int srv_apply_lastaddr(struct server *srv, int *err_code); -static void srv_cleanup_connections(struct server *srv); /* extra keywords used as value for other arguments. They are used as * suggestions for mistyped words. @@ -8082,7 +8081,7 @@ static int srv_migrate_conns_to_remove(struct server *srv, int thr, int toremove /* cleanup connections for a given server * might be useful when going on forced maintenance or live changing ip/port */ -static void srv_cleanup_connections(struct server *srv) +void srv_cleanup_connections(struct server *srv) { struct sess_priv_conns *sess_conns; int did_remove; @@ -8221,6 +8220,7 @@ int srv_add_to_idle_list(struct server *srv, struct connection *conn, int is_saf */ if (!(conn->flags & CO_FL_PRIVATE) && srv && srv->pool_purge_delay > 0 && + !((conn->flags & CO_FL_REVERSED) && (srv->proxy->flags & PR_FL_STOPPED)) && !(srv->cur_admin & SRV_ADMF_MAINT) && ((srv->proxy->options & PR_O_REUSE_MASK) != PR_O_REUSE_NEVR) && ha_used_fds < global.tune.pool_high_count && -- 2.54.0 (Apple Git-157)
