CLEANUP: backend: factor the connection lookup loop

The connection lookup loop is made of two identical blocks, one looking
in the idle or safe lists and the other one looking into the safe list
only. The second one is skipped if a connection was found or if the request
looks for a safe one (since already done). Also the two are slightly
different due to leftovers from earlier versions in that the second one
checks for safe connections and not the first one, and the second one
sets is_safe which is not used later.

Let's just rationalize all this by placing them in a loop which checks
first from the idle conns and second from the safe ones, or skips the
first step if the request wants a safe connection. This reduces the
code and shortens the time spent under the lock.

diff --git a/src/backend.c b/src/backend.c
index e54a101dad0d43e93dac13afff9d5a7345c26858..a835be5a12797794c6fa34b6ce7b7055fa4122bc 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1361,37 +1361,36 @@ struct connection *conn_backend_get(int reuse_mode,
 check_tgid:
        i = stop;
        do {
+               /* safe requests looked up conns in idle tree first, then safe
+                * tree; unsafe requests are looked up in the safe conns tree.
+                */
+               int search_tree = is_safe ? 1 : 0; // 0 = idle, 1 = safe
+               struct eb_root *tree;
+
                if (!srv->curr_idle_thr[i] || i == tid)
                        continue;
 
                if (HA_SPIN_TRYLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock) != 0)
                        continue;
-               conn = srv_lookup_conn(is_safe ? &srv->per_thr[i].safe_conns : &srv->per_thr[i].idle_conns, hash);
-               while (conn) {
-                       if (conn->mux->takeover && conn->mux->takeover(conn, i, 0) == 0) {
-                               conn_delete_from_tree(conn);
-                               _HA_ATOMIC_INC(&activity[tid].fd_takeover);
-                               found = 1;
-                               break;
-                       }
 
-                       conn = srv_lookup_conn_next(conn);
-               }
+               do {
+                       if ((search_tree && !srv->curr_safe_nb) ||
+                           (!search_tree && !srv->curr_idle_nb))
+                               continue;
 
-               if (!found && !is_safe && srv->curr_safe_nb > 0) {
-                       conn = srv_lookup_conn(&srv->per_thr[i].safe_conns, hash);
+                       tree = search_tree ? &srv->per_thr[i].safe_conns : &srv->per_thr[i].idle_conns;
+                       conn = srv_lookup_conn(tree, hash);
                        while (conn) {
                                if (conn->mux->takeover && conn->mux->takeover(conn, i, 0) == 0) {
                                        conn_delete_from_tree(conn);
                                        _HA_ATOMIC_INC(&activity[tid].fd_takeover);
                                        found = 1;
-                                       is_safe = 1;
                                        break;
                                }
-
                                conn = srv_lookup_conn_next(conn);
                        }
-               }
+               } while (!found && ++search_tree <= 1);
+
                HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
        } while (!found && (i = (i + 1 == curtg->base + curtg->count) ? curtg->base : i + 1) != stop);