MEDIUM: server: extend refcount for all servers

In a future patch, it will be possible to remove at runtime every
servers, both static and dynamic. This requires to extend the server
refcount for all instances.

First, refcount manipulation functions have been renamed to better
express the API usage.

* srv_refcount_use -> srv_take
The refcount is always initialize to 1 on the server creation in
new_server. It's also incremented for each check/agent configured on a
server instance.

* free_server -> srv_drop
This decrements the refcount and if null, the server is freed, so code
calling it must not use the server reference after it. As a bonus, this
function now returns the next server instance. This is useful when
calling on the server loop without having to save the next pointer
before each invocation.

In these functions, remove the checks that prevent refcount on
non-dynamic servers. Each reference to "dynamic" in variable/function
naming have been eliminated as well.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 02f15fc01a9b61afc837753eef43e6d3511993e2..a01081d549774cca6faab8ee9c8ca98f160abf79 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -259,7 +259,7 @@ struct server {
        unsigned cumulative_weight;             /* weight of servers prior to this one in the same group, for chash balancing */
        int maxqueue;                           /* maximum number of pending connections allowed */
 
-       uint refcount_dynsrv;                   /* refcount used for dynamic servers */
+       uint refcount;                          /* refcount used to remove a server at runtime */
 
        /* The elements below may be changed on every single request by any
         * thread, and generally at the same time.

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index b39be2a1ce4aa7cd24b563a59b91434cd62b6ad9..99d26ba0ddcffe22ba32e5f3198c4f2ef1d4cfa7 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -59,8 +59,8 @@ int srv_set_addr_via_libc(struct server *srv, int *err_code);
 int srv_init_addr(void);
 struct server *cli_find_server(struct appctx *appctx, char *arg);
 struct server *new_server(struct proxy *proxy);
-void srv_use_dynsrv(struct server *srv);
-struct server *free_server(struct server *srv);
+void srv_take(struct server *srv);
+struct server *srv_drop(struct server *srv);
 int srv_init_per_thr(struct server *srv);
 
 /* functions related to server name resolution */

diff --git a/src/check.c b/src/check.c
index 4a4e4c77097c8e56246bb5af9339d195126ca75c..55f07939b15f1df40bb9fb2432fdc96190f65ef1 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -1241,13 +1241,13 @@ struct task *process_chk_conn(struct task *t, void *context, unsigned int state)
        TRACE_LEAVE(CHK_EV_TASK_WAKE, check);
 
        /* Free the check if set to PURGE. After this, the check instance may be
-        * freed via the free_server invocation, so it must not be accessed
-        * after this point.
+        * freed via the srv_drop invocation, so it must not be accessed after
+        * this point.
         */
        if (unlikely(check->state & CHK_ST_PURGE)) {
                free_check(check);
                if (check->server)
-                       free_server(check->server);
+                       srv_drop(check->server);
 
                t = NULL;
        }
@@ -1688,6 +1688,7 @@ int init_srv_check(struct server *srv)
                goto out;
        }
        srv->check.state |= CHK_ST_CONFIGURED | CHK_ST_ENABLED;
+       srv_take(srv);
 
        /* Only increment maxsock for servers from the configuration. Dynamic
         * servers at the moment are not taken into account for the estimation
@@ -1743,6 +1744,7 @@ int init_srv_agent_check(struct server *srv)
                srv->agent.inter = srv->check.inter;
 
        srv->agent.state |= CHK_ST_CONFIGURED | CHK_ST_ENABLED | CHK_ST_AGENT;
+       srv_take(srv);
 
        /* Only increment maxsock for servers from the configuration. Dynamic
         * servers at the moment are not taken into account for the estimation

diff --git a/src/hlua.c b/src/hlua.c
index 4ec28a08324187784d8da980cccf4d3417d64306..ea9d31cdbd2c2ab6cc9a7b73c34f3b30877b185c 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11759,10 +11759,10 @@ static void hlua_deinit()
                        lua_close(hlua_states[thr]);
        }
 
-       free_server(socket_tcp);
+       srv_drop(socket_tcp);
 
 #ifdef USE_OPENSSL
-       free_server(socket_ssl);
+       srv_drop(socket_ssl);
 #endif
 
        free_proxy(socket_proxy);

diff --git a/src/http_client.c b/src/http_client.c
index 6719ce50714b7be867354d63ce138c4444f67355..9f06ccc44dfceced9fb3962e1811d497bc7ffb56 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -738,9 +738,9 @@ static int httpclient_init()
 err:
        ha_alert("httpclient: cannot initialize.\n");
        free(errmsg);
-       free_server(httpclient_srv_raw);
+       srv_drop(httpclient_srv_raw);
 #ifdef USE_OPENSSL
-       free_server(httpclient_srv_ssl);
+       srv_drop(httpclient_srv_ssl);
 #endif
        free_proxy(httpclient_proxy);
        return err_code;

diff --git a/src/proxy.c b/src/proxy.c
index 5d701866bbf19dbeef42852add7219944079c903..3391dc90bbd9ecc0d486e97659e40757f731860d 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -291,7 +291,7 @@ void free_proxy(struct proxy *p)
        while (s) {
                list_for_each_entry(srvdf, &server_deinit_list, list)
                        srvdf->fct(s);
-               s = free_server(s);
+               s = srv_drop(s);
        }/* end while(s) */
 
        list_for_each_entry_safe(l, l_next, &p->conf.listeners, by_fe) {

diff --git a/src/server.c b/src/server.c
index 54a569f7234052606dffffe984f49bf91af8fb25..fdce968f0868a086b6373053e6f2064c468ab71c 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2160,6 +2160,8 @@ struct server *new_server(struct proxy *proxy)
        if (!srv)
                return NULL;
 
+       srv_take(srv);
+
        srv->obj_type = OBJ_TYPE_SERVER;
        srv->proxy = proxy;
        queue_init(&srv->queue, proxy, srv);
@@ -2196,18 +2198,10 @@ struct server *new_server(struct proxy *proxy)
        return srv;
 }
 
-/* Increment the dynamic server refcount. */
-void srv_use_dynsrv(struct server *srv)
+/* Increment the server refcount. */
+void srv_take(struct server *srv)
 {
-       BUG_ON(!(srv->flags & SRV_F_DYNAMIC));
-       HA_ATOMIC_INC(&srv->refcount_dynsrv);
-}
-
-/* Decrement the dynamic server refcount. */
-static uint srv_release_dynsrv(struct server *srv)
-{
-       BUG_ON(!(srv->flags & SRV_F_DYNAMIC));
-       return HA_ATOMIC_SUB_FETCH(&srv->refcount_dynsrv, 1);
+       HA_ATOMIC_INC(&srv->refcount);
 }
 
 /* Deallocate a server <srv> and its member. <srv> must be allocated. For
@@ -2215,9 +2209,9 @@ static uint srv_release_dynsrv(struct server *srv)
  * conducted only if the refcount is nul, unless the process is stopping.
  *
  * As a convenience, <srv.next> is returned if srv is not NULL. It may be useful
- * when calling free_server on the list of servers.
+ * when calling srv_drop on the list of servers.
  */
-struct server *free_server(struct server *srv)
+struct server *srv_drop(struct server *srv)
 {
        struct server *next = NULL;
 
@@ -2230,10 +2224,8 @@ struct server *free_server(struct server *srv)
         * server when reaching zero.
         */
        if (likely(!(global.mode & MODE_STOPPING))) {
-               if (srv->flags & SRV_F_DYNAMIC) {
-                       if (srv_release_dynsrv(srv))
-                               goto end;
-               }
+               if (HA_ATOMIC_SUB_FETCH(&srv->refcount, 1))
+                       goto end;
        }
 
        task_destroy(srv->warmup);
@@ -4590,7 +4582,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
                        goto out;
 
                srv->check.state &= ~CHK_ST_ENABLED;
-               srv_use_dynsrv(srv);
        }
 
        if (srv->do_agent) {
@@ -4598,7 +4589,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
                        goto out;
 
                srv->agent.state &= ~CHK_ST_ENABLED;
-               srv_use_dynsrv(srv);
        }
 
        /* Attach the server to the end of the proxy linked list. Note that this
@@ -4649,7 +4639,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
        ebis_insert(&be->conf.used_server_name, &srv->conf.name);
        ebis_insert(&be->used_server_addr, &srv->addr_node);
 
-       srv_use_dynsrv(srv);
        thread_release();
 
        /* Start the check task. The server must be fully initialized.
@@ -4701,7 +4690,7 @@ out:
                cli_err(appctx, usermsgs_str());
 
        if (srv)
-               free_server(srv);
+               srv_drop(srv);
 
        return 1;
 }
@@ -4823,7 +4812,7 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
        thread_release();
 
        ha_notice("Server deleted.\n");
-       free_server(srv);
+       srv_drop(srv);
 
        cli_msg(appctx, LOG_INFO, "Server deleted.");
 

diff --git a/src/stats.c b/src/stats.c
index 8140d87f953a4402d9732eebc36015c6b4b201ad..35415b30988a0b58e825df0ccd2952c8863f01dc 100644 (file)
--- a/src/stats.c
+++ b/src/stats.c
@@ -3113,17 +3113,15 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
        case STAT_PX_ST_SV:
                /* obj2 points to servers list as initialized above.
                 *
-                * A dynamic server may be removed during the stats dumping.
+                * A server may be removed during the stats dumping.
                 * Temporarily increment its refcount to prevent its
                 * anticipated cleaning. Call free_server to release it.
                 */
                for (; appctx->ctx.stats.obj2 != NULL;
-                      appctx->ctx.stats.obj2 =
-                       (!(sv->flags & SRV_F_DYNAMIC)) ? sv->next : free_server(sv)) {
+                      appctx->ctx.stats.obj2 = srv_drop(sv)) {
 
                        sv = appctx->ctx.stats.obj2;
-                       if (sv->flags & SRV_F_DYNAMIC)
-                               srv_use_dynsrv(sv);
+                       srv_take(sv);
 
                        if (htx) {
                                if (htx_almost_full(htx))
@@ -3136,8 +3134,7 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
 
                        if (appctx->ctx.stats.flags & STAT_BOUND) {
                                if (!(appctx->ctx.stats.type & (1 << STATS_TYPE_SV))) {
-                                       if (sv->flags & SRV_F_DYNAMIC)
-                                               free_server(appctx->ctx.stats.obj2);
+                                       srv_drop(sv);
                                        break;
                                }