Revert "BUG/MEDIUM: connections: force connections cleanup on server changes"
authorWilliam Dauchy <w.dauchy@criteo.com>
Tue, 2 Jun 2020 14:03:58 +0000 (16:03 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Tue, 2 Jun 2020 17:09:50 +0000 (19:09 +0200)
As explained by Christopher on github issue #665:
In 2.2, srv->idle_conns and srv->safe_conns are thread-safe lists. But
not in 2.1. So the patch must be reverted or the lists must be changed
to use mt_list instead. The same must be done in 2.0, but the mt_list
does not exist on this version.

I choose to revert it as the original bug is truly revealed in v2.2
after commit 079cb9af22da6 ("MEDIUM: connections: Revamp the way idle
connections are killed")

this should resolve github issue #665

this reverts commit 7eab37b6819af685c647cf5a581e29fca2f3e079.
this reverts commit 3ad3306ec0bcb0cd4ca2b9ba134ed67663473ee8.

The patch was directly introduced on 2.1, there is no upstream commit ID for
this patch.

src/server.c

index 7d44921..408458f 100644 (file)
@@ -55,7 +55,6 @@ static int srv_apply_lastaddr(struct server *srv, int *err_code);
 static int srv_set_fqdn(struct server *srv, const char *fqdn, int dns_locked);
 static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params);
 static int srv_state_get_version(FILE *f);
-static void srv_cleanup_connections(struct server *srv);
 
 /* List head of all known server keywords */
 static struct srv_kw_list srv_keywords = {
@@ -3945,11 +3944,8 @@ const char *update_server_addr_port(struct server *s, const char *addr, const ch
        }
 
 out:
-       if (changed) {
-               /* force connection cleanup on the given server */
-               srv_cleanup_connections(s);
+       if (changed)
                srv_set_dyncookie(s);
-       }
        if (updater)
                chunk_appendf(msg, " by '%s'", updater);
        chunk_appendf(msg, "\n");
@@ -5110,8 +5106,6 @@ static void srv_update_status(struct server *s)
                        if (s->onmarkeddown & HANA_ONMARKEDDOWN_SHUTDOWNSESSIONS)
                                srv_shutdown_streams(s, SF_ERR_DOWN);
 
-                       /* force connection cleanup on the given server */
-                       srv_cleanup_connections(s);
                        /* we might have streams queued on this server and waiting for
                         * a connection. Those which are redispatchable will be queued
                         * to another server or to the proxy itself.
@@ -5440,37 +5434,6 @@ struct task *srv_cleanup_toremove_connections(struct task *task, void *context,
        return task;
 }
 
-/* cleanup connections for a given server
- * might be useful when going on forced maintenance or live changing ip/port
- */
-static void srv_cleanup_connections(struct server *srv)
-{
-       struct connection *conn;
-       int did_remove;
-       int i;
-       int j;
-
-       HA_SPIN_LOCK(OTHER_LOCK, &idle_conn_srv_lock);
-       for (i = 0; i < global.nbthread; i++) {
-               did_remove = 0;
-               HA_SPIN_LOCK(OTHER_LOCK, &toremove_lock[i]);
-               for (j = 0; j < srv->curr_idle_conns; j++) {
-                       conn = LIST_ELEM(srv->idle_conns[tid].n, struct connection *, list);
-                       if (!conn)
-                               conn = LIST_ELEM(srv->safe_conns[tid].n,
-                                                struct connection *, list);
-                       if (!conn)
-                               break;
-                       did_remove = 1;
-                       MT_LIST_ADDQ(&toremove_connections[i], (struct mt_list *)&conn->list);
-               }
-               HA_SPIN_UNLOCK(OTHER_LOCK, &toremove_lock[i]);
-               if (did_remove)
-                       task_wakeup(idle_conn_cleanup[i], TASK_WOKEN_OTHER);
-       }
-       HA_SPIN_UNLOCK(OTHER_LOCK, &idle_conn_srv_lock);
-}
-
 struct task *srv_cleanup_idle_connections(struct task *task, void *context, unsigned short state)
 {
        struct server *srv;