]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: backend: use a trylock when trying to grab an idle connection
authorUbuntu <ubuntu@ip-172-31-37-79.us-east-2.compute.internal>
Mon, 1 Mar 2021 06:22:17 +0000 (06:22 +0000)
committerWilly Tarreau <w@1wt.eu>
Fri, 5 Mar 2021 07:30:08 +0000 (08:30 +0100)
In conn_backend_get() we can cause some extreme contention due to the
idle_conns_lock. Indeed, even though it's per-thread, it still causes
high contention when running with many threads. The reason is that all
threads which do not have any idle connections are quickly skipped,
till the point where there are still some, so the first reaching that
point will grab the lock and the other ones wait behind. From this
point, all threads are synchronized waiting on the same lock, and
will follow the leader in small jumps, all hindering each other.

Here instead of doing this we're using a trylock. This way when a thread
is already checking a list, other ones will continue to next thread. In
the worst case, a high contention will lead to a few new connections to
be set up, but this may actually be what is required to avoid contention
in the first place. With this change, the contention has mostly
disappeared on this lock (it's still present in muxes and transport
layers due to the takeover).

Surprisingly, checking for emptiness of the tree root before taking
the lock didn't address any contention.

A few improvements are still possible and desirable here. The first
one would be to avoid seeing all threads jump to the next one. We
could have each thread use a different prime number as the increment
so as to spread them across the entire table instead of keeping them
synchronized. The second one is that the lock in the muck layers
shouldn't be needed to check for the tasklet's context availability.

src/backend.c

index 4c1839d388716450e3ef0a0339dbe0a6dc1f90fa..88531e7f257b8a610fc07cc1ff50dded56cadcd7 100644 (file)
@@ -1183,7 +1183,8 @@ static struct connection *conn_backend_get(struct stream *s, struct server *srv,
                if (!srv->curr_idle_thr[i] || i == tid)
                        continue;
 
-               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
+               if (HA_SPIN_TRYLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock) != 0)
+                       continue;
                conn = srv_lookup_conn(&tree[i], hash);
                while (conn) {
                        if (conn->mux->takeover && conn->mux->takeover(conn, i) == 0) {