From: Amaury Denoyelle Date: Mon, 21 Aug 2023 16:32:29 +0000 (+0200) Subject: MAJOR: connection: purge idle conn by last usage X-Git-Tag: v2.9-dev4~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5afcb686b;p=thirdparty%2Fhaproxy.git 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 53477756db..451eb8d971 100644 --- 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 0fdfd6abe0..fc1f55e738 100644 --- 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 6be443a8bc..2f9da87abc 100644 --- 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 46d942c451..c56b4f3089 100644 --- 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 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 a51d597d88..3673340d15 100644 --- 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 count connections from storage to + * 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); } } }