1berry/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MINOR: stream: decrement srv->served after detaching from the list

Browse Source 
In commit 3372a2ea00 ("BUG/MEDIUM: queues: Stricly respect maxconn for
outgoing connections"), it has been ensured that srv->served is held
as long as possible around the periods where a stream is attached to a
server. However, it's decremented early when entering sess_change_server,
and actually just before detaching from that server's list. While there
is theoretically nothing wrong with this, it prevents us from looking at
this counter to know if streams are still using a server or not.

We could imagine decrementing it much later but that wouldn't work with
leastconn, since that algo needs ->served to be final before calling
lbprm.server_drop_conn(). Thus what we're doing here is to detach from
the server, then decrement ->served, and only then call the LB callback
to update the server's position in the tree. At this moment the stream
doesn't know the server anymore anyway (except via this function's
local variable) so it's safe to consider that no stream knows the server
once the variable reaches zero.
This commit is contained in:
Willy Tarreau 2025-03-18 11:24:56 +01:00
parent 7895726bff
commit c880c32b16
1 changed files with 19 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/stream.c
View File

@ -2749,22 +2749,32 @@ void sess_change_server(struct stream *strm, struct server *newsrv)
* invocation of sess_change_server().
*/
/*
* It is assumed if the stream has a non-NULL srv_conn, then its
* served field has been incremented, so we have to decrement it now.
*/
if (oldsrv)
_HA_ATOMIC_DEC(&oldsrv->served);
if (oldsrv == newsrv)
if (oldsrv == newsrv) {
/*
* It is assumed if the stream has a non-NULL srv_conn, then its
* served field has been incremented, so we have to decrement it now.
*/
if (oldsrv)
_HA_ATOMIC_DEC(&oldsrv->served);
return;
}
if (oldsrv) {
/* Note: we cannot decrement served after calling server_drop_conn
* because that one may rely on served (e.g. for leastconn). The
* real need here is to make sure that when served==0, no stream
* knows the server anymore, mainly for server removal purposes,
* and that removal will be done under isolation anyway. Thus by
* decrementing served after detaching from the list, we're
* guaranteeing that served==0 implies that no stream is in the
* list anymore, which is a sufficient guarantee for tha goal.
*/
_HA_ATOMIC_DEC(&oldsrv->proxy->served);
stream_del_srv_conn(strm);
_HA_ATOMIC_DEC(&oldsrv->served);
__ha_barrier_atomic_store();
if (oldsrv->proxy->lbprm.server_drop_conn)
oldsrv->proxy->lbprm.server_drop_conn(oldsrv);
stream_del_srv_conn(strm);
}
if (newsrv) {