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 05d4b77..54517b1 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 f540e32..97c3231 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);