From 72653c5a85066ddc65275045c70ab47a572b7b43 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 4 Mar 2021 10:47:54 +0100 Subject: [PATCH] MINOR: server: move actconns to the per-thread structure The actconns list creates massive contention on low server counts because it's in fact a list of streams using a server, all threads compete on the list's head and it's still possible to see some watchdog panics on 48 threads under extreme contention with 47 threads trying to add and one thread trying to delete. Moving this list per thread is trivial because it's only used by srv_shutdown_streams(), which simply required to iterate over the list. The field was renamed to "streams" as it's really a list of streams rather than a list of connections. (cherry picked from commit d4e78d873cecf9938885c90e736f8b761a35fb55) [wt: for 2.3 and older, this is a simplified version. We don't want to touch all the server initialization code and sequence with the risks of causing new trouble, so instead we declare actconns as an array of the maximum number of supported threads, this will eat a bit more memory on smaller machines but will remain safe. Those building with MAX_THREADS close or equal to their target number of threads may even save some RAM compared to 2.4. The only servers not initialized via new_server() are the two Lua socket servers, and they've been handled here] Signed-off-by: Willy Tarreau --- include/haproxy/server-t.h | 2 +- include/haproxy/stream.h | 8 +++++++- src/hlua.c | 6 ++++-- src/server.c | 12 ++++++++---- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index 4b26e43..de4a2d5 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -222,7 +222,7 @@ struct server { struct be_counters counters; /* statistics counters */ struct eb_root pendconns; /* pending connections */ - struct mt_list actconns; /* active connections (used by "shutdown server sessions") */ + struct mt_list actconns[MAX_THREADS]; /* active connections (used by "shutdown server sessions") */ struct mt_list *idle_conns; /* shareable idle connections*/ struct mt_list *safe_conns; /* safe idle connections */ struct list *available_conns; /* Connection in used, but with still new streams available */ diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h index 8500fb6..735ef70 100644 --- a/include/haproxy/stream.h +++ b/include/haproxy/stream.h @@ -343,7 +343,13 @@ static inline void stream_inc_http_err_ctr(struct stream *s) static inline void stream_add_srv_conn(struct stream *sess, struct server *srv) { - MT_LIST_ADD(&srv->actconns, &sess->by_srv); + /* note: this inserts in reverse order but we do not care, it's only + * used for massive kills (i.e. almost never). MT_LIST_ADD() is a bit + * faster than MT_LIST_ADDQ under contention due to a faster recovery + * from a conflict with an adjacent MT_LIST_DEL, and using it improves + * the performance by about 3% on 32-cores. + */ + MT_LIST_ADD(&srv->actconns[tid], &sess->by_srv); HA_ATOMIC_STORE(&sess->srv_conn, srv); } diff --git a/src/hlua.c b/src/hlua.c index 8dd2f31..8244593 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -8750,7 +8750,8 @@ void hlua_init(void) socket_tcp.next = NULL; socket_tcp.proxy = &socket_proxy; socket_tcp.obj_type = OBJ_TYPE_SERVER; - MT_LIST_INIT(&socket_tcp.actconns); + for (i = 0; i < MAX_THREADS; i++) + MT_LIST_INIT(&socket_tcp.actconns[i]); socket_tcp.pendconns = EB_ROOT; socket_tcp.idle_conns = NULL; socket_tcp.safe_conns = NULL; @@ -8795,7 +8796,8 @@ void hlua_init(void) socket_ssl.next = NULL; socket_ssl.proxy = &socket_proxy; socket_ssl.obj_type = OBJ_TYPE_SERVER; - MT_LIST_INIT(&socket_ssl.actconns); + for (i = 0; i < MAX_THREADS; i++) + MT_LIST_INIT(&socket_ssl.actconns[i]); socket_ssl.pendconns = EB_ROOT; socket_ssl.idle_conns = NULL; socket_ssl.safe_conns = NULL; diff --git a/src/server.c b/src/server.c index 13eb676..c01c948 100644 --- a/src/server.c +++ b/src/server.c @@ -865,10 +865,12 @@ void srv_shutdown_streams(struct server *srv, int why) { struct stream *stream; struct mt_list *elt1, elt2; + int thr; - mt_list_for_each_entry_safe(stream, &srv->actconns, by_srv, elt1, elt2) - if (stream->srv_conn == srv) - stream_shutdown(stream, why); + for (thr = 0; thr < global.nbthread; thr++) + mt_list_for_each_entry_safe(stream, &srv->actconns[thr], by_srv, elt1, elt2) + if (stream->srv_conn == srv) + stream_shutdown(stream, why); } /* Shutdown all connections of all backup servers of a proxy. The caller must @@ -1718,6 +1720,7 @@ static void srv_settings_cpy(struct server *srv, struct server *src, int srv_tmp struct server *new_server(struct proxy *proxy) { struct server *srv; + int i; srv = calloc(1, sizeof *srv); if (!srv) @@ -1725,7 +1728,8 @@ struct server *new_server(struct proxy *proxy) srv->obj_type = OBJ_TYPE_SERVER; srv->proxy = proxy; - MT_LIST_INIT(&srv->actconns); + for (i = 0; i < MAX_THREADS; i++) + MT_LIST_INIT(&srv->actconns[i]); srv->pendconns = EB_ROOT; srv->next_state = SRV_ST_RUNNING; /* early server setup */ -- 1.7.10.4