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.

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 */

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 */

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 (!LIST_ISEMPTY(&srv->per_thr[tid].idle_conn_list)) {
-		if (node) {
+			tokill_conn = LIST_ELEM(&srv->per_thr[tid].idle_conn_list.n, struct connection *, idle_list);
-			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[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 (!LIST_ISEMPTY(&srv->per_thr[i].idle_conn_list)) {
-				if (node) {
+					tokill_conn = LIST_ELEM(&srv->per_thr[i].idle_conn_list.n, struct connection *, idle_list);
-					conn_node = ebmb_entry(node, struct conn_hash_node, node);
-					tokill_conn = conn_node->conn;
 					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);

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);
 }
 

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
+/* Move <toremove_nb> count connections from <list> storage to <toremove_list>
- * moving them all.
+ * 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 connection *conn;
-	struct conn_hash_node *hash_node;
 	int i = 0;
 
-	node = eb_first(idle_tree);
+	while (!LIST_ISEMPTY(list)) {
-	while (node) {
-		next = eb_next(node);
 		if (toremove_nb != -1 && i >= toremove_nb)
 			break;
 
-		hash_node = ebmb_entry(node, struct conn_hash_node, node);
+		conn = LIST_ELEM(list->n, struct connection *, toremove_list);
-		eb_delete(node);
+		conn_delete_from_tree(conn);
-		MT_LIST_APPEND(toremove_list, &hash_node->conn->toremove_list);
+		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)
+		if (srv_migrate_conns_to_remove(&srv->per_thr[i].idle_conn_list, &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)
 			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);
 			}
 		}
 	}