MAJOR: connection: purge idle conn by last usage

Backend idle connections are purged on a recurring occurence during the
process lifetime. An estimated number of needed connections is
calculated and the excess is removed periodically.

Before this patch, purge was done directly using the idle then the safe
connection tree of a server instance. This has a major drawback to take
no account of a specific ordre and it may removed functional connections
while leaving ones which will fail on the next reuse.

The problem can be worse when using criteria to differentiate idle
connections such as the SSL SNI. In this case, purge may remove
connections with a high rate of reusing while leaving connections with
criteria never matched once, thus reducing drastically the reuse rate.

To improve this, introduce an alternative storage for idle connection
used in parallel of the idle/safe trees. Now, each connection inserted
in one of this tree is also inserted in the new list at
`srv_per_thread.idle_conn_list`. This guarantees that recently used
connection is present at the end of the list.

During the purge, use this list instead of idle/safe trees. Remove first
connection in front of the list which were not reused recently. This
will ensure that connection that are frequently reused are not purged
and should increase the reuse rate, particularily if distinct idle
connection criterias are in used.

diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index 53477756db8ce5d5c19dec0a850f02efc6601585..451eb8d9715e8e6aae876b5b3759159d8bd86912 100644 (file)
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -526,7 +526,10 @@ struct connection {
 
        /* second cache line */
        struct wait_event *subs; /* Task to wake when awaited events are ready */
-       struct mt_list toremove_list; /* list for connection to clean up */
+       union {
+               struct list    idle_list; /* list element for idle connection in server idle list */
+               struct mt_list toremove_list; /* list element when idle connection is ready to be purged */
+       };
        union {
                struct list session_list;  /* used by backend conns, list of attached connections to a session */
                struct list stopping_list; /* used by frontend conns, attach point in mux stopping list */

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 0fdfd6abe004388016dd01c073e0fc9719c3c289..fc1f55e738b990485402a32a7da6a6e8551d9c50 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -237,6 +237,11 @@ struct srv_per_thread {
        struct eb_root idle_conns;              /* Shareable idle connections */
        struct eb_root safe_conns;              /* Safe idle connections */
        struct eb_root avail_conns;             /* Connections in use, but with still new streams available */
+
+       /* Secondary idle conn storage used in parallel to idle/safe trees.
+        * Used to sort them by last usage and purge them in reverse order.
+        */
+       struct list idle_conn_list;
 };
 
 /* Each server will have one occurrence of this structure per thread group */

diff --git a/src/backend.c b/src/backend.c
index 6be443a8bc03d3849e58173e332a0dec1e366ff9..2f9da87abca5b6afb229acf2471506b100fa5f29 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1492,18 +1492,13 @@ static int connect_server(struct stream *s)
 
        if (ha_used_fds > global.tune.pool_high_count && srv) {
                struct connection *tokill_conn = NULL;
-               struct conn_hash_node *conn_node = NULL;
-               struct ebmb_node *node = NULL;
-
                /* We can't reuse a connection, and e have more FDs than deemd
                 * acceptable, attempt to kill an idling connection
                 */
                /* First, try from our own idle list */
                HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-               node = ebmb_first(&srv->per_thr[tid].idle_conns);
-               if (node) {
-                       conn_node = ebmb_entry(node, struct conn_hash_node, node);
-                       tokill_conn = conn_node->conn;
+               if (!LIST_ISEMPTY(&srv->per_thr[tid].idle_conn_list)) {
+                       tokill_conn = LIST_ELEM(&srv->per_thr[tid].idle_conn_list.n, struct connection *, idle_list);
                        conn_delete_from_tree(tokill_conn);
                        HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 
@@ -1531,28 +1526,17 @@ static int connect_server(struct stream *s)
                                if (HA_SPIN_TRYLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock) != 0)
                                        continue;
 
-                               node = ebmb_first(&srv->per_thr[i].idle_conns);
-                               if (node) {
-                                       conn_node = ebmb_entry(node, struct conn_hash_node, node);
-                                       tokill_conn = conn_node->conn;
+                               if (!LIST_ISEMPTY(&srv->per_thr[i].idle_conn_list)) {
+                                       tokill_conn = LIST_ELEM(&srv->per_thr[i].idle_conn_list.n, struct connection *, idle_list);
                                        conn_delete_from_tree(tokill_conn);
                                }
-
-                               if (!tokill_conn) {
-                                       node = ebmb_first(&srv->per_thr[i].safe_conns);
-                                       if (node) {
-                                               conn_node = ebmb_entry(node, struct conn_hash_node, node);
-                                               tokill_conn = conn_node->conn;
-                                               conn_delete_from_tree(tokill_conn);
-                                       }
-                               }
                                HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
 
                                if (tokill_conn) {
                                        /* We got one, put it into the concerned thread's to kill list, and wake it's kill task */
 
                                        MT_LIST_APPEND(&idle_conns[i].toremove_conns,
-                                           (struct mt_list *)&tokill_conn->toremove_list);
+                                                      &tokill_conn->toremove_list);
                                        task_wakeup(idle_conns[i].cleanup_task, TASK_WOKEN_OTHER);
                                        break;
                                }
@@ -1566,6 +1550,9 @@ static int connect_server(struct stream *s)
                        int avail = srv_conn->mux->avail_streams(srv_conn);
 
                        if (avail <= 1) {
+                               /* connection cannot be in idle list if used as an avail idle conn. */
+                               BUG_ON(LIST_INLIST(&srv_conn->idle_list));
+
                                /* No more streams available, remove it from the list */
                                HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
                                conn_delete_from_tree(srv_conn);

diff --git a/src/connection.c b/src/connection.c
index 46d942c4511e4eaa9e3728bf0243c3f7c7d0badd..c56b4f3089c6dbbacd73600434c0a079f97b7e9c 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -52,8 +52,14 @@ struct mux_stopping_data mux_stopping_data[MAX_THREADS];
 /* disables sending of proxy-protocol-v2's LOCAL command */
 static int pp2_never_send_local;
 
+/* Remove <conn> idle connection from its attached tree (idle, safe or avail).
+ * If also present in the secondary server idle list, conn is removed from it.
+ *
+ * Must be called with idle_conns_lock held.
+ */
 void conn_delete_from_tree(struct connection *conn)
 {
+       LIST_DEL_INIT((struct list *)&conn->toremove_list);
        eb64_delete(&conn->hash_node->node);
 }
 

diff --git a/src/server.c b/src/server.c
index a51d597d88bd71d30d9c4a8b592f6dd4deae1dc7..3673340d155a6af726400c0b72f805a4465d5a10 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -4800,6 +4800,8 @@ int srv_init_per_thr(struct server *srv)
                srv->per_thr[i].safe_conns = EB_ROOT;
                srv->per_thr[i].avail_conns = EB_ROOT;
                MT_LIST_INIT(&srv->per_thr[i].streams);
+
+               LIST_INIT(&srv->per_thr[i].idle_conn_list);
        }
 
        return 0;
@@ -5867,31 +5869,28 @@ struct task *srv_cleanup_toremove_conns(struct task *task, void *context, unsign
        return task;
 }
 
-/* Move toremove_nb connections from idle_tree to toremove_list, -1 means
- * moving them all.
+/* Move <toremove_nb> count connections from <list> storage to <toremove_list>
+ * list storage. -1 means moving all of them.
+ *
  * Returns the number of connections moved.
  *
  * Must be called with idle_conns_lock held.
  */
-static int srv_migrate_conns_to_remove(struct eb_root *idle_tree, struct mt_list *toremove_list, int toremove_nb)
+static int srv_migrate_conns_to_remove(struct list *list, struct mt_list *toremove_list, int toremove_nb)
 {
-       struct eb_node *node, *next;
-       struct conn_hash_node *hash_node;
+       struct connection *conn;
        int i = 0;
 
-       node = eb_first(idle_tree);
-       while (node) {
-               next = eb_next(node);
+       while (!LIST_ISEMPTY(list)) {
                if (toremove_nb != -1 && i >= toremove_nb)
                        break;
 
-               hash_node = ebmb_entry(node, struct conn_hash_node, node);
-               eb_delete(node);
-               MT_LIST_APPEND(toremove_list, &hash_node->conn->toremove_list);
+               conn = LIST_ELEM(list->n, struct connection *, toremove_list);
+               conn_delete_from_tree(conn);
+               MT_LIST_APPEND(toremove_list, &conn->toremove_list);
                i++;
-
-               node = next;
        }
+
        return i;
 }
 /* cleanup connections for a given server
@@ -5910,9 +5909,7 @@ static void srv_cleanup_connections(struct server *srv)
        for (i = tid;;) {
                did_remove = 0;
                HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
-               if (srv_migrate_conns_to_remove(&srv->per_thr[i].idle_conns, &idle_conns[i].toremove_conns, -1) > 0)
-                       did_remove = 1;
-               if (srv_migrate_conns_to_remove(&srv->per_thr[i].safe_conns, &idle_conns[i].toremove_conns, -1) > 0)
+               if (srv_migrate_conns_to_remove(&srv->per_thr[i].idle_conn_list, &idle_conns[i].toremove_conns, -1) > 0)
                        did_remove = 1;
                HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
                if (did_remove)
@@ -6000,7 +5997,12 @@ void _srv_add_idle(struct server *srv, struct connection *conn, int is_safe)
 {
        struct eb_root *tree = is_safe ? &srv->per_thr[tid].safe_conns :
                                         &srv->per_thr[tid].idle_conns;
+
+       /* first insert in idle or safe tree. */
        eb64_insert(tree, &conn->hash_node->node);
+
+       /* insert in list sorted by connection usage. */
+       LIST_APPEND(&srv->per_thr[tid].idle_conn_list, &conn->idle_list);
 }
 
 /* This adds an idle connection to the server's list if the connection is
@@ -6132,12 +6134,9 @@ struct task *srv_cleanup_idle_conns(struct task *task, void *context, unsigned i
                                   curr_idle + 1;
 
                        HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
-                       j = srv_migrate_conns_to_remove(&srv->per_thr[i].idle_conns, &idle_conns[i].toremove_conns, max_conn);
+                       j = srv_migrate_conns_to_remove(&srv->per_thr[i].idle_conn_list, &idle_conns[i].toremove_conns, max_conn);
                        if (j > 0)
                                did_remove = 1;
-                       if (max_conn - j > 0 &&
-                           srv_migrate_conns_to_remove(&srv->per_thr[i].safe_conns, &idle_conns[i].toremove_conns, max_conn - j) > 0)
-                               did_remove = 1;
                        HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[i].idle_conns_lock);
 
                        if (did_remove)
@@ -6193,7 +6192,7 @@ static void srv_close_idle_conns(struct server *srv)
 
                                if (conn->ctrl->ctrl_close)
                                        conn->ctrl->ctrl_close(conn);
-                               ebmb_delete(node);
+                               conn_delete_from_tree(conn);
                        }
                }
        }