OPTIM/MINOR: server: remove unnecessary server lookup during purge

Function srv_check_for_deletion() was previously used in proxy purge
task to determine if a server is ready to be deleted. However, this
performs an unnecessary server lookup as the server instance is already
known at this stage.

Simplify this by replacing it with new helper srv_in_use(), which
reports if a server still has active/pending/idle connections. If not
the case, removal can be performed.

This code is safe as it runs in thread isolation. If a server is removed
via another path before proxy purge task, it will be removed from
backend purge list. Furthermore, it also eliminates unnecessary checks
on server configuration : only a removable server can be inserted in the
proxy purge list.

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 6044152f2dcbb2f3f4125fd58cfb004cc54d1000..23d0c0d724c834dd6556dead7e79b85e182c04db 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -96,6 +96,7 @@ const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
 const char *srv_op_st_chg_cause(enum srv_op_st_chg_cause cause);
 void srv_event_hdl_publish_check(struct server *srv, struct check *check);
 
+int srv_in_use(const struct server *srv);
 int srv_check_for_deletion(const char *bename, const char *svname, enum pr_del_mode del_mode,
                            struct proxy **pb, struct server **ps, const char **pm);
 void srv_unregister(struct server *srv);

diff --git a/src/proxy.c b/src/proxy.c
index ad87f90fa4cf6fb1e64468630f707c600fc81a61..0aaeafa7ac49e9c6b4ac545d5e9461d7c8d9bbed 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -523,7 +523,6 @@ void proxy_drop(struct proxy *p)
 static int proxy_purge_servers(struct proxy *px, int max_srv_delete)
 {
        struct server *srv;
-       int ret;
 
        while (!LIST_ISEMPTY(&px->servers_purge)) {
                if (!max_srv_delete)
@@ -535,22 +534,16 @@ static int proxy_purge_servers(struct proxy *px, int max_srv_delete)
                        continue;
                }
 
-               /* TODO unnecessary server lookup */
-               ret = srv_check_for_deletion(px->id, srv->id, PR_DEL_MODE_SYNC, NULL, NULL, NULL);
-               if (ret < 0) {
-                       srv_cancel_purge(srv);
-                       LIST_DEL_INIT(&srv->el_alt);
-               }
-               else if (ret > 0) {
-                       ha_notice("Server '%s/%s' purged.\n", px->id, srv->id);
-                       srv_unregister(srv);
-                       srv_drop(srv);
-                       --max_srv_delete;
-               }
-               else {
-                       /* Interrupt work on first server not yet removable. */
+               /* A non removable server must not be flagged for purge. */
+               BUG_ON(srv->flags & (SRV_F_NON_PURGEABLE | SRV_F_NAME_REFD));
+
+               if (srv_in_use(srv))
                        break;
-               }
+
+               ha_notice("Server '%s/%s' purged.\n", px->id, srv->id);
+               srv_unregister(srv);
+               srv_drop(srv);
+               --max_srv_delete;
        }
 
        return !(LIST_ISEMPTY(&px->servers_purge) || max_srv_delete);

diff --git a/src/server.c b/src/server.c
index f9fe785c6dc65cdd1d58a06246933178ae02da95..8031d6b13689c851c18992692f89efc341301277 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2094,7 +2094,7 @@ static int srv_parse_strict_maxconn(char **args, int *cur_arg, struct proxy *px,
        return 0;
 }
 /* Returns 1 if the server has streams pointing to it, and 0 otherwise. */
-static int srv_has_streams(struct server *srv)
+static int srv_has_streams(const struct server *srv)
 {
        int i;
 
@@ -6909,6 +6909,17 @@ out:
        return 1;
 }
 
+/* Returns true if <srv> can be considered as active, i.e. there is active,
+ * pending and/or idle connections on it. Connections scheduled for purging are
+ * still accounted there.
+ */
+int srv_in_use(const struct server *srv)
+{
+       return !!(_HA_ATOMIC_LOAD(&srv->curr_used_conns) ||
+                 _HA_ATOMIC_LOAD(&srv->queueslength) || srv_has_streams(srv) ||
+                 _HA_ATOMIC_LOAD(&srv->curr_idle_conns) || _HA_ATOMIC_LOAD(&srv->curr_sess_idle_conns));
+}
+
 /* Checks if the server <bename>/<svname> exists and is ready for being
  * deleted. This means that the server is in maintenance with no streams
  * attached to it, no queue and no used idle conns. This is not supposed to
@@ -6973,12 +6984,8 @@ int srv_check_for_deletion(const char *bename, const char *svname, enum pr_del_m
        /* Second, conditions that may change over time */
        ret = 0;
 
-       /* Ensure that there is no active/pending/idle connection on the server.
-        * Note that idle conns scheduled for purging are still accounted in idle counter.
-        */
-       if (_HA_ATOMIC_LOAD(&srv->curr_used_conns) ||
-           _HA_ATOMIC_LOAD(&srv->queueslength) || srv_has_streams(srv) ||
-           _HA_ATOMIC_LOAD(&srv->curr_idle_conns) || _HA_ATOMIC_LOAD(&srv->curr_sess_idle_conns)) {
+       /* Ensure that there is no active/pending/idle connection on the server. */
+       if (srv_in_use(srv)) {
                msg = "Server still has connections attached to it, cannot remove it.";
                goto leave;
        }
@@ -7338,6 +7345,10 @@ int srv_apply_track(struct server *srv, struct proxy *curproxy)
        strack->trackers = srv;
        strack->flags |= SRV_F_NAME_REFD;
 
+       /* Cancel background deletion on tracked server if activated. */
+       if (srv->flags & SRV_F_DYNAMIC)
+               srv_cancel_purge(strack);
+
        ha_free(&srv->trackit);
 
        return 0;