]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: sessions: Use an unlimited number of servers for the conn list.
authorOlivier Houchard <ohouchard@haproxy.com>
Thu, 27 Dec 2018 16:20:54 +0000 (17:20 +0100)
committerWilly Tarreau <w@1wt.eu>
Fri, 28 Dec 2018 15:33:13 +0000 (16:33 +0100)
When a session adds a connection to its connection list, we used to remove
connections for an another server if there were not enough room for our
server. This can't work, because those lists are now the list of connections
we're responsible for, not just the idle connections.
To fix this, allow for an unlimited number of servers, instead of using
an array, we're now using a linked list.

include/proto/connection.h
include/proto/session.h
include/types/session.h
src/backend.c
src/mux_h1.c
src/mux_h2.c
src/proto_http.c
src/session.c

index f67465832ed91a1852895434e6592870ae77eaec..28e7580913e6f9c8f9b646bdc43820416d566a35 100644 (file)
@@ -29,6 +29,7 @@
 #include <types/listener.h>
 #include <proto/fd.h>
 #include <proto/obj_type.h>
+#include <proto/session.h>
 #include <proto/task.h>
 
 extern struct pool_head *pool_head_connection;
@@ -676,7 +677,7 @@ static inline void conn_free(struct connection *conn)
        if (!LIST_ISEMPTY(&conn->session_list)) {
                struct session *sess = conn->owner;
                sess->resp_conns--;
-               LIST_DEL(&conn->session_list);
+               session_unown_conn(sess, conn);
        }
 
        /* By convention we always place a NULL where the ctx points to if the
index 8c99e7d3c9a292d8d45e02c92e6af1ed25127ee3..c34866d3d590b7ed5634132490925e3158e519a0 100644 (file)
@@ -35,6 +35,8 @@
 #include <proto/server.h>
 
 extern struct pool_head *pool_head_session;
+extern struct pool_head *pool_head_sess_srv_list;
+
 struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type *origin);
 void session_free(struct session *sess);
 int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr);
@@ -73,46 +75,46 @@ static inline void session_store_counters(struct session *sess)
        }
 }
 
-static inline void session_add_conn(struct session *sess, struct connection *conn, void *target)
+/* Remove the connection from the session list, and destroy the srv_list if it's now empty */
+static inline void session_unown_conn(struct session *sess, struct connection *conn)
 {
-       int avail = -1;
-       int i;
-
-       sess->resp_conns++;
-       for (i = 0; i < MAX_SRV_LIST; i++) {
-               if (sess->srv_list[i].target == target) {
-                       avail = i;
+       struct sess_srv_list *srv_list = NULL;
+       LIST_DEL(&conn->session_list);
+       LIST_INIT(&conn->session_list);
+       list_for_each_entry(srv_list, &sess->srv_list, srv_list) {
+               if (srv_list->target == conn->target) {
+                       if (LIST_ISEMPTY(&srv_list->conn_list)) {
+                               LIST_DEL(&srv_list->srv_list);
+                               pool_free(pool_head_sess_srv_list, srv_list);
+                       }
                        break;
                }
-               if (LIST_ISEMPTY(&sess->srv_list[i].list) && avail == -1)
-                       avail = i;
        }
-       if (avail == -1) {
-               struct connection *conn, *conn_back;
-               int count = 0;
-               /* We have no slot free, let's free the one with the fewer connections */
-               for (i = 0; i < MAX_SRV_LIST; i++) {
-                       int count_list = 0;
-                       list_for_each_entry(conn, &sess->srv_list[i].list, session_list)
-                           count_list++;
-                       if (count == 0 || count_list < count) {
-                               count = count_list;
-                               avail = i;
-                       }
-               }
-               /* Now unown all the connections */
-               list_for_each_entry_safe(conn, conn_back, &sess->srv_list[avail].list, session_list) {
-                       sess->resp_conns--;
-                       conn->owner = NULL;
-                       LIST_DEL(&conn->session_list);
-                       LIST_INIT(&conn->session_list);
-                       if (conn->mux)
-                               conn->mux->destroy(conn);
-               }
+}
+
+static inline int session_add_conn(struct session *sess, struct connection *conn, void *target)
+{
+       struct sess_srv_list *srv_list = NULL;
+       int found = 0;
 
+       list_for_each_entry(srv_list, &sess->srv_list, srv_list) {
+               if (srv_list->target == target) {
+                       found = 1;
+                       break;
+               }
+       }
+       if (!found) {
+               /* The session has no connection for the server, create a new entry */
+               srv_list = pool_alloc(pool_head_sess_srv_list);
+               if (!srv_list)
+                       return 0;
+               srv_list->target = target;
+               LIST_INIT(&srv_list->conn_list);
+               LIST_ADDQ(&sess->srv_list, &srv_list->srv_list);
        }
-       sess->srv_list[avail].target = target;
-       LIST_ADDQ(&sess->srv_list[avail].list, &conn->session_list);
+       sess->resp_conns++;
+       LIST_ADDQ(&srv_list->conn_list, &conn->session_list);
+       return 1;
 }
 
 /* Returns 0 if the session can keep the idle conn, -1 if it was destroyed, or 1 if it was added to the server list */
@@ -120,8 +122,7 @@ static inline int session_check_idle_conn(struct session *sess, struct connectio
 {
        if (sess->resp_conns > sess->fe->max_out_conns) {
                /* We can't keep the connection, let's try to add it to the server idle list */
-               LIST_DEL(&conn->session_list);
-               LIST_INIT(&conn->session_list);
+               session_unown_conn(sess, conn);
                conn->owner = NULL;
                sess->resp_conns--;
                if (!srv_add_to_idle_list(objt_server(conn->target), conn)) {
index 33ce4fc3211f99b3b4257c1486ec38abc7c1462d..1675f8f96403d9e40cc44f094674c04b3cd5186a 100644 (file)
@@ -39,7 +39,8 @@
 
 struct sess_srv_list {
        void *target;
-       struct list list;
+       struct list conn_list; /* Head of the connections list */
+       struct list srv_list; /* Next element of the server list */
 };
 
 #define MAX_SRV_LIST   5
@@ -55,7 +56,7 @@ struct session {
        struct task *task;              /* handshake timeout processing */
        long t_handshake;               /* handshake duration, -1 = not completed */
        int resp_conns;                 /* Number of connections we're currently responsible for */
-       struct sess_srv_list srv_list[MAX_SRV_LIST]; /* List of servers and the connections the session is currently responsible for */
+       struct list srv_list;           /* List of servers and the connections the session is currently responsible for */
 };
 
 #endif /* _TYPES_SESSION_H */
index 0c17ef893c52482021eed4fe5ce0298e7662df7d..babe96eb8657ad7a98aca34eabe90cd800a54ba5 100644 (file)
@@ -562,7 +562,6 @@ int assign_server(struct stream *s)
        struct server *conn_slot;
        struct server *srv = NULL, *prev_srv;
        int err;
-       int i;
 
        DPRINTF(stderr,"assign_server : s=%p\n",s);
 
@@ -590,8 +589,9 @@ int assign_server(struct stream *s)
        if ((s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_HI &&
            ((s->txn && s->txn->flags & TX_PREFER_LAST) ||
             (s->be->options & PR_O_PREF_LAST))) {
-               for (i = 0; i < MAX_SRV_LIST; i++) {
-                       struct server *tmpsrv = objt_server(s->sess->srv_list[i].target);
+               struct sess_srv_list *srv_list;
+               list_for_each_entry(srv_list, &s->sess->srv_list, srv_list) {
+                       struct server *tmpsrv = objt_server(srv_list->target);
 
                        if (tmpsrv && tmpsrv->proxy == s->be &&
                            ((s->txn && s->txn->flags & TX_PREFER_LAST) ||
@@ -599,7 +599,7 @@ int assign_server(struct stream *s)
                              server_has_room(tmpsrv) || (
                              tmpsrv->nbpend + 1 < s->be->max_ka_queue))) &&
                            srv_currently_usable(tmpsrv)) {
-                               list_for_each_entry(conn, &s->sess->srv_list[i].list, session_list) {
+                               list_for_each_entry(conn, &srv_list->conn_list, session_list) {
                                        if (conn->flags & CO_FL_CONNECTED) {
 
                                                srv = tmpsrv;
@@ -1119,7 +1119,6 @@ int connect_server(struct stream *s)
        int reuse = 0;
        int reuse_orphan = 0;
        int err;
-       int i;
 
 
        /* Some, such as http_proxy and the LUA, create their connection and
@@ -1135,9 +1134,10 @@ int connect_server(struct stream *s)
                        reuse = 1;
                }
        } else {
-               for (i = 0; i < MAX_SRV_LIST; i++) {
-                       if (s->sess->srv_list[i].target == s->target) {
-                               list_for_each_entry(srv_conn, &s->sess->srv_list[i].list,
+               struct sess_srv_list *srv_list;
+               list_for_each_entry(srv_list, &s->sess->srv_list, srv_list) {
+                       if (srv_list->target == s->target) {
+                               list_for_each_entry(srv_conn, &srv_list->conn_list,
                                    session_list) {
                                        if (conn_xprt_ready(srv_conn) &&
                                            srv_conn->mux && (srv_conn->mux->avail_streams(srv_conn) > 0)) {
@@ -1145,17 +1145,19 @@ int connect_server(struct stream *s)
                                                break;
                                        }
                                }
+                               break;
                        }
                }
                if (reuse == 0) {
                        srv_conn = NULL;
-                       for (i = 0; i < MAX_SRV_LIST; i++) {
-                               if (!LIST_ISEMPTY(&s->sess->srv_list[i].list)) {
-                                       srv_conn = LIST_ELEM(s->sess->srv_list[i].list.n,
-                                           struct connection *, session_list);
-                                       break;
-                               }
+                       if (!LIST_ISEMPTY(&s->sess->srv_list)) {
+                               srv_list = LIST_ELEM(s->sess->srv_list.n,
+                                       struct sess_srv_list *, srv_list);
+                               if (!LIST_ISEMPTY(&srv_list->conn_list))
+                                       srv_conn = LIST_ELEM(srv_list->conn_list.n,
+                                               struct connection *, session_list);
                        }
+
                }
        }
        old_conn = srv_conn;
@@ -1252,11 +1254,13 @@ int connect_server(struct stream *s)
                                    old_conn->mux != NULL &&
                                    (old_conn->mux->avail_streams(old_conn) > 0) &&
                                    (srv_conn->mux->avail_streams(srv_conn) == 1)) {
-                                       LIST_DEL(&old_conn->session_list);
-                                       LIST_INIT(&old_conn->session_list);
+                                       session_unown_conn(s->sess, old_conn);
                                        old_conn->owner = sess;
-                                       session_add_conn(sess, old_conn, s->target);
-                                       session_check_idle_conn(sess, old_conn);
+                                       if (!session_add_conn(sess, old_conn, s->target)) {
+                                               old_conn->owner = NULL;
+                                               old_conn->mux->destroy(old_conn);
+                                       } else
+                                               session_check_idle_conn(sess, old_conn);
                                }
                        }
 
@@ -1284,10 +1288,20 @@ int connect_server(struct stream *s)
                }
        }
        if (srv_conn && old_conn != srv_conn) {
+               if (srv_conn->owner)
+                       session_unown_conn(srv_conn->owner, srv_conn);
                srv_conn->owner = s->sess;
-               LIST_DEL(&srv_conn->session_list);
-               LIST_INIT(&srv_conn->session_list);
-               session_add_conn(s->sess, srv_conn, s->target);
+               if (!session_add_conn(s->sess, srv_conn, s->target)) {
+                       /* If we failed to attach the connection, detach the
+                        * conn_stream, possibly destroying the connection */
+                       cs_destroy(srv_cs);
+                       srv_conn->owner = NULL;
+                       if (!srv_add_to_idle_list(objt_server(srv_conn->target), srv_conn))
+                       /* The server doesn't want it, let's kill the connection right away */
+                               srv_conn->mux->destroy(srv_conn);
+                       srv_conn = NULL;
+
+               }
        }
 
        if (!srv_conn)
index 7b00ce9ac590472121a38979e603ea508d9d0271..e27aec3f8d655a25bc2428247472868f7b7804a1 100644 (file)
@@ -1948,7 +1948,16 @@ static void h1_detach(struct conn_stream *cs)
 
                if (!(h1c->conn->owner)) {
                        h1c->conn->owner = sess;
-                       session_add_conn(sess, h1c->conn, h1c->conn->target);
+                       if (!session_add_conn(sess, h1c->conn, h1c->conn->target)) {
+                               h1c->conn->owner = NULL;
+                               if (!srv_add_to_idle_list(objt_server(h1c->conn->target), h1c->conn))
+                                       /* The server doesn't want it, let's kill the connection right away */
+                                       h1c->conn->mux->destroy(h1c->conn);
+                               else
+                                       tasklet_wakeup(h1c->wait_event.task);
+                               return;
+
+                       }
                }
                if (h1c->conn->owner == sess) {
                        int ret = session_check_idle_conn(sess, h1c->conn);
index 3786d032097383c313bc21e4912792344d73ae4e..5803a84ff6fb63b75db70f9a3cbaa88c00ef24d1 100644 (file)
@@ -2960,7 +2960,15 @@ static void h2_detach(struct conn_stream *cs)
                    (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH))) {
                        if (!h2c->conn->owner) {
                                h2c->conn->owner = sess;
-                               session_add_conn(sess, h2c->conn, h2c->conn->target);
+                               if (!session_add_conn(sess, h2c->conn, h2c->conn->target)) {
+                                       h2c->conn->owner = NULL;
+                                       if (eb_is_empty(&h2c->streams_by_id)) {
+                                               if (!srv_add_to_idle_list(objt_server(h2c->conn->target), h2c->conn))
+                                                       /* The server doesn't want it, let's kill the connection right away */
+                                                       h2c->conn->mux->destroy(h2c->conn);
+                                               return;
+                                       }
+                               }
                        }
                        if (eb_is_empty(&h2c->streams_by_id)) {
                                if (session_check_idle_conn(h2c->conn->owner, h2c->conn) != 0)
index 71609f22850b4337b28c385c018e6f96c55b0558..bf661ed4406f9b2f48d89c4ffd26a5fcccd88d5e 100644 (file)
@@ -3494,7 +3494,17 @@ void http_end_txn_clean_session(struct stream *s)
                 * have released the endpoint and know if it no longer has
                 * attached streams, and so an idling connection
                 */
-               session_add_conn(s->sess, srv_conn, s->target);
+               if (!session_add_conn(s->sess, srv_conn, s->target)) {
+                       srv_conn->owner = NULL;
+                       /* Try to add the connection to the server idle list.
+                        * If it fails, as the connection no longer has an
+                        * owner, it will be destroy later by
+                        * si_release_endpoint(), anyway
+                        */
+                       srv_add_to_idle_list(objt_server(srv_conn->target), srv_conn);
+                       srv_conn = NULL;
+
+               }
        }
        si_release_endpoint(&s->si[1]);
        if (srv_conn && srv_conn->owner == s->sess) {
index 211181c3a555c99ee6e943db6c1ece6c50439439..48a5d9a70aba501da8a767236779cd1f64ef6883 100644 (file)
@@ -29,6 +29,8 @@
 #include <proto/vars.h>
 
 DECLARE_POOL(pool_head_session, "session", sizeof(struct session));
+DECLARE_POOL(pool_head_sess_srv_list, "session server list",
+               sizeof(struct sess_srv_list));
 
 static int conn_complete_session(struct connection *conn);
 static struct task *session_expire_embryonic(struct task *t, void *context, unsigned short state);
@@ -41,7 +43,6 @@ static struct task *session_expire_embryonic(struct task *t, void *context, unsi
 struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type *origin)
 {
        struct session *sess;
-       int i;
 
        sess = pool_alloc(pool_head_session);
        if (sess) {
@@ -60,10 +61,7 @@ struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type
                        proxy_inc_fe_conn_ctr(li, fe);
                HA_ATOMIC_ADD(&totalconn, 1);
                HA_ATOMIC_ADD(&jobs, 1);
-               for (i = 0; i < MAX_SRV_LIST; i++) {
-                       sess->srv_list[i].target = NULL;
-                       LIST_INIT(&sess->srv_list[i].list);
-               }
+               LIST_INIT(&sess->srv_list);
                sess->resp_conns = 0;
        }
        return sess;
@@ -72,7 +70,7 @@ struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type
 void session_free(struct session *sess)
 {
        struct connection *conn, *conn_back;
-       int i;
+       struct sess_srv_list *srv_list, *srv_list_back;
 
        HA_ATOMIC_SUB(&sess->fe->feconn, 1);
        if (sess->listener)
@@ -82,10 +80,8 @@ void session_free(struct session *sess)
        conn = objt_conn(sess->origin);
        if (conn != NULL && conn->mux)
                conn->mux->destroy(conn);
-       for (i = 0; i < MAX_SRV_LIST; i++) {
-               int count = 0;
-               list_for_each_entry_safe(conn, conn_back, &sess->srv_list[i].list, session_list) {
-                       count++;
+       list_for_each_entry_safe(srv_list, srv_list_back, &sess->srv_list, srv_list) {
+               list_for_each_entry_safe(conn, conn_back, &srv_list->conn_list, session_list) {
                        if (conn->mux) {
 
                                LIST_DEL(&conn->session_list);
@@ -102,6 +98,7 @@ void session_free(struct session *sess)
                                conn_free(conn);
                        }
                }
+               pool_free(pool_head_sess_srv_list, srv_list);
        }
        pool_free(pool_head_session, sess);
        HA_ATOMIC_SUB(&jobs, 1);