MINOR: backend: don't always takeover from the same threads

The next thread walking algorithm in commit 566df309c ("MEDIUM:
connections: Attempt to get idle connections from other threads.")
proved to be sufficient for most cases, but it still has some rough
edges when threads are unevenly loaded. If one thread wakes up with
10 streams to process in a burst, it will mainly take over connections
from the next one until it doesn't have anymore.

This patch implements a rotating index that is stored into the server
list and that any thread taking over a connection is responsible for
updating. This way it starts mostly random and avoids always picking
from the same place. This results in a smoother distribution overall
and a slightly lower takeover rate.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 05d4b777d2aacf8d042d12224c4e47516b9900bf..54517b17c6c302e3fed6316c013f6ddd76144a7f 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -235,6 +235,7 @@ struct server {
        unsigned int max_used_conns;            /* Max number of used connections (the counter is reset at each connection purges */
        unsigned int est_need_conns;            /* Estimate on the number of needed connections (max of curr and previous max_used) */
        unsigned int *curr_idle_thr;            /* Current number of orphan idling connections per thread */
+       unsigned int next_takeover;             /* thread ID to try to steal connections from next time */
        int max_reuse;                          /* Max number of requests on a same connection */
        struct eb32_node idle_node;             /* When to next do cleanup in the idle connections */
        struct task *warmup;                    /* the task dedicated to the warmup when slowstart is set */

diff --git a/src/backend.c b/src/backend.c
index f540e321d87e58f83cb4d7433650e5aaaf4e0514..97c3231d5103d860c26f40e6dfaeb6fbc7056922 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1079,6 +1079,7 @@ static struct connection *conn_backend_get(struct server *srv, int is_safe)
        struct connection *conn;
        int i; // thread number
        int found = 0;
+       int stop;
 
        /* We need to lock even if this is our own list, because another
         * thread may be trying to migrate that connection, and we don't want
@@ -1115,11 +1116,17 @@ static struct connection *conn_backend_get(struct server *srv, int is_safe)
            ha_used_fds < global.tune.pool_low_count)
                goto done;
 
-       /* Lookup all other threads for an idle connection, starting from tid + 1 */
-       while (!found && (i = ((i + 1 == global.nbthread) ? 0 : i + 1)) != tid) {
+       /* Lookup all other threads for an idle connection, starting from last
+        * unvisited thread.
+        */
+       stop = srv->next_takeover;
+       if (stop >= global.nbthread)
+               stop = 0;
+
+       for (i = stop; !found && (i = ((i + 1 == global.nbthread) ? 0 : i + 1)) != stop;) {
                struct mt_list *elt1, elt2;
 
-               if (!srv->curr_idle_thr[i])
+               if (!srv->curr_idle_thr[i] || i == tid)
                        continue;
 
                HA_SPIN_LOCK(OTHER_LOCK, &idle_conns[i].toremove_lock);
@@ -1152,6 +1159,7 @@ static struct connection *conn_backend_get(struct server *srv, int is_safe)
  done:
        if (conn) {
                conn->idle_time = 0;
+               _HA_ATOMIC_STORE(&srv->next_takeover, (i + 1 == global.nbthread) ? 0 : i + 1);
                _HA_ATOMIC_SUB(&srv->curr_idle_conns, 1);
                _HA_ATOMIC_SUB(&srv->curr_idle_thr[i], 1);
                _HA_ATOMIC_SUB(is_safe ? &srv->curr_safe_nb : &srv->curr_idle_nb, 1);