MEDIUM: counters: manage shared counters using dedicated helpers

proxies, listeners and server shared counters are now managed via helpers
added in one of the previous commits.

When guid is not set (ie: when not yet assigned), shared counters pointer
is allocated using calloc() (local memory) and a flag is set on the shared
counters struct to know how to manipulate (and free it). Else if guid is
set, then it means that the counters may be shared so while for now we
don't actually use a shared memory location the API is ready for that.

The way it works, for proxies and servers (for which guid is not known
during creation), we first call counters_{fe,be}_shared_get with guid not
set, which results in local pointer being retrieved (as if we just
manually called calloc() to retrieve a pointer). Later (during postparsing)
if guid is set we try to upgrade the pointer from local to shared.

Lastly, since the memory location for some objects (proxies and servers
counters) may change from creation to postparsing, let's update
counters->last_change member directly under counters_{fe,be}_shared_get()
so we don't miss it.

No change of behavior is expected, this is only preparation work.

diff --git a/include/haproxy/counters-t.h b/include/haproxy/counters-t.h
index 28f456ea413b715ed452932c6eb6c7d4e90514c8..ad7ac9eed2bbbca511f2abd7b94154f7f7f5324d 100644 (file)
--- a/include/haproxy/counters-t.h
+++ b/include/haproxy/counters-t.h
@@ -26,11 +26,13 @@
 #include <haproxy/freq_ctr-t.h>
 
 #define COUNTERS_SHARED_F_NONE    0x0000
+#define COUNTERS_SHARED_F_LOCAL   0x0001 // shared counter struct is actually process-local
 
 // common to fe_counters_shared and be_counters_shared
 #define COUNTERS_SHARED                                                              \
        struct {                                                                     \
                uint16_t flags;                         /* COUNTERS_SHARED_F flags */\
+               unsigned long last_change;              /* last time, when the state was changed */\
        }
 
 // for convenience (generic pointer)
@@ -74,7 +76,6 @@ struct fe_counters_shared {
                } http;
        } p;                                    /* protocol-specific stats */
        struct freq_ctr sess_per_sec;           /* sessions per second on this server */
-       unsigned long last_change;              /* last time, when the state was changed */
 
        long long failed_req;                   /* failed requests (eg: invalid or timeout) */
        long long denied_resp;                  /* blocked responses because of security concerns */
@@ -146,8 +147,6 @@ struct be_counters_shared {
        long long bytes_in;                     /* number of bytes transferred from the client to the server */
 
        long long    cum_sess;                  /* cumulated number of accepted connections */
-
-       unsigned long last_change;              /* last time, when the state was changed */
 };
 
 /* counters used by servers and backends */

diff --git a/src/cfgparse.c b/src/cfgparse.c
index d8ea334f079d909dae19521104ef8d9a7d5d94f0..d0b86d65a71d4e46daa88b5c41b0f88ea7fa5cc1 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -49,6 +49,7 @@
 #include <haproxy/check.h>
 #include <haproxy/chunk.h>
 #include <haproxy/clock.h>
+#include <haproxy/counters.h>
 #ifdef USE_CPU_AFFINITY
 #include <haproxy/cpuset.h>
 #include <haproxy/cpu_topo.h>
@@ -4259,7 +4260,7 @@ init_proxies_list_stage2:
                        if (curproxy->options2 & PR_O2_SOCKSTAT) {
                                listener->counters = calloc(1, sizeof(*listener->counters));
                                if (listener->counters) {
-                                       listener->counters->shared = calloc(1, sizeof(*listener->counters->shared));
+                                       listener->counters->shared = counters_fe_shared_get(&listener->guid);
                                        if (!listener->counters->shared) {
                                                ha_free(&listener->counters);
                                                ha_alert("config: %s '%s': out of memory.\n",

diff --git a/src/counters.c b/src/counters.c
index 41d029350455ea6bde0d3ce592a6d2dc68592011..e9385135aea3240ec22a898a7da0b37c754413a1 100644 (file)
--- a/src/counters.c
+++ b/src/counters.c
@@ -19,18 +19,32 @@
  */
 
 #include <stdlib.h>
+#include <haproxy/atomic.h>
+#include <haproxy/clock.h>
 #include <haproxy/counters.h>
+#include <haproxy/time.h>
 
 /* retrieved shared counters pointer for a given <guid> object
  * <size> hint is expected to reflect the actual type size (fe/be)
+ * if <guid> is not set, then sharing is disabled
  * Returns the pointer on success or NULL on failure
  */
-static void *_counters_shared_get(const struct guid_node *guid, size_t size)
+static void*_counters_shared_get(const struct guid_node *guid, size_t size)
 {
+       struct counters_shared *shared;
+       uint last_change;
+
        /* no shared memory for now, simply allocate a memory block
         * for the counters (zero-initialized), ignore guid
         */
-       return calloc(1, size);
+       shared = calloc(1, size);
+       if (!shared)
+               return NULL;
+       if (!guid->node.key)
+               shared->flags |= COUNTERS_SHARED_F_LOCAL;
+       last_change = ns_to_sec(now_ns);
+       HA_ATOMIC_STORE(&shared->last_change, last_change);
+       return shared;
 }
 
 /* retrieve shared fe counters pointer for a given <guid> object */

diff --git a/src/proxy.c b/src/proxy.c
index b39f31621324e673b87e9e6703ded632cf4e0a03..566f3f6161c8afcd274ff0327b52712b6405665e 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -25,6 +25,7 @@
 #include <haproxy/capture-t.h>
 #include <haproxy/cfgparse.h>
 #include <haproxy/cli.h>
+#include <haproxy/counters.h>
 #include <haproxy/errors.h>
 #include <haproxy/fd.h>
 #include <haproxy/filters.h>
@@ -221,8 +222,8 @@ static inline void proxy_free_common(struct proxy *px)
        ha_free(&px->id);
        LIST_DEL_INIT(&px->global_list);
        drop_file_name(&px->conf.file);
-       ha_free(&px->fe_counters.shared);
-       ha_free(&px->be_counters.shared);
+       counters_fe_shared_drop(px->fe_counters.shared);
+       counters_be_shared_drop(px->be_counters.shared);
        ha_free(&px->check_command);
        ha_free(&px->check_path);
        ha_free(&px->cookie_name);
@@ -398,7 +399,7 @@ void deinit_proxy(struct proxy *p)
                free(l->label);
                free(l->per_thr);
                if (l->counters) {
-                       free(l->counters->shared);
+                       counters_fe_shared_drop(l->counters->shared);
                        free(l->counters);
                }
                task_destroy(l->rx.rhttp.task);
@@ -1710,39 +1711,8 @@ void proxy_unref_defaults(struct proxy *px)
  */
 int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char **errmsg)
 {
-       uint last_change;
-
        init_new_proxy(px);
 
-       last_change = ns_to_sec(now_ns);
-       /* allocate private memory for shared counters: used as a fallback
-        * or when sharing is disabled. If sharing is enabled pointers will
-        * be updated to point to the proper shared memory location during
-        * proxy postparsing, see proxy_postparse()
-        */
-       if (cap & PR_CAP_FE) {
-               px->fe_counters.shared = calloc(1, sizeof(*px->fe_counters.shared));
-               if (!px->fe_counters.shared) {
-                       memprintf(errmsg, "out of memory");
-                       goto fail;
-               }
-               HA_ATOMIC_STORE(&px->fe_counters.shared->last_change, last_change);
-       }
-       if (cap & (PR_CAP_FE|PR_CAP_BE)) {
-               /* by default stream->be points to stream->fe, thus proxy
-                * be_counters may be used even if the proxy lacks the backend
-                * capability
-                */
-               px->be_counters.shared = calloc(1, sizeof(*px->be_counters.shared));
-               if (!px->be_counters.shared) {
-                       memprintf(errmsg, "out of memory");
-                       goto fail;
-               }
-
-               HA_ATOMIC_STORE(&px->be_counters.shared->last_change, last_change);
-       }
-
-
        if (name) {
                px->id = strdup(name);
                if (!px->id) {
@@ -1766,8 +1736,8 @@ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char *
                memprintf(errmsg, "proxy '%s': %s", name, *errmsg);
 
        ha_free(&px->id);
-       ha_free(&px->fe_counters.shared);
-       ha_free(&px->be_counters.shared);
+       counters_fe_shared_drop(px->fe_counters.shared);
+       counters_be_shared_drop(px->be_counters.shared);
 
        return 0;
 }
@@ -1799,6 +1769,46 @@ struct proxy *alloc_new_proxy(const char *name, unsigned int cap, char **errmsg)
        return NULL;
 }
 
+/* post-check for proxies */
+static int proxy_postcheck(struct proxy *px)
+{
+       int err_code = ERR_NONE;
+
+       /* allocate private memory for shared counters: used as a fallback
+        * or when sharing is disabled. If sharing is enabled pointers will
+        * be updated to point to the proper shared memory location during
+        * proxy postparsing, see proxy_postparse()
+        */
+       if (px->cap & PR_CAP_FE) {
+               px->fe_counters.shared = counters_fe_shared_get(&px->guid);
+               if (!px->fe_counters.shared) {
+                       ha_alert("out of memory while setting up shared counters for %s %s\n",
+                                proxy_type_str(px), px->id);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto out;
+               }
+       }
+       if (px->cap & (PR_CAP_FE|PR_CAP_BE)) {
+               /* by default stream->be points to stream->fe, thus proxy
+                * be_counters may be used even if the proxy lacks the backend
+                * capability
+                */
+               px->be_counters.shared = counters_be_shared_get(&px->guid);
+               if (!px->be_counters.shared) {
+                       ha_alert("out of memory while setting up shared counters for %s %s\n",
+                                proxy_type_str(px), px->id);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto out;
+               }
+
+       }
+
+
+ out:
+       return err_code;
+}
+REGISTER_POST_PROXY_CHECK(proxy_postcheck);
+
 /* Copy the proxy settings from <defproxy> to <curproxy>.
  * Returns 0 on success.
  * Returns 1 on error. <errmsg> will be allocated with an error description.

diff --git a/src/server.c b/src/server.c
index 758d7e2a7d253f360cf33574784f6a6662bec993..7a8d1ba6157977466c5525cc2716989a090c88e9 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -25,6 +25,7 @@
 #include <haproxy/check.h>
 #include <haproxy/cli.h>
 #include <haproxy/connection.h>
+#include <haproxy/counters.h>
 #include <haproxy/dict-t.h>
 #include <haproxy/errors.h>
 #include <haproxy/global.h>
@@ -3028,18 +3029,11 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
 struct server *new_server(struct proxy *proxy)
 {
        struct server *srv;
-       struct be_counters_shared *scounters;
 
        srv = calloc(1, sizeof *srv);
        if (!srv)
                return NULL;
 
-       scounters = calloc(1, sizeof(*scounters));
-       if (!scounters) {
-               ha_free(&srv);
-               return NULL;
-       }
-
        srv_take(srv);
 
        srv->obj_type = OBJ_TYPE_SERVER;
@@ -3052,8 +3046,6 @@ struct server *new_server(struct proxy *proxy)
        srv->rid = 0; /* rid defaults to 0 */
 
        srv->next_state = SRV_ST_RUNNING; /* early server setup */
-       srv->counters.shared = scounters;
-       HA_ATOMIC_STORE(&srv->counters.shared->last_change, ns_to_sec(now_ns));
 
        srv->check.obj_type = OBJ_TYPE_CHECK;
        srv->check.status = HCHK_STATUS_INI;
@@ -3125,7 +3117,7 @@ void srv_free_params(struct server *srv)
        free(srv->resolvers_id);
        free(srv->addr_node.key);
        free(srv->lb_nodes);
-       free(srv->counters.shared);
+       counters_be_shared_drop(srv->counters.shared);
        if (srv->log_target) {
                deinit_log_target(srv->log_target);
                free(srv->log_target);
@@ -3434,6 +3426,21 @@ int srv_init(struct server *srv)
        if (err_code & ERR_CODE)
                goto out;
 
+       srv->counters.shared = counters_be_shared_get(&srv->guid);
+       if (!srv->counters.shared) {
+               ha_alert("memory error while setting up shared counters for %s/%s server\n", srv->proxy->id, srv->id);
+               err_code |= ERR_ALERT | ERR_FATAL;
+               goto out;
+       }
+
+       if (srv->flags & SRV_F_DYNAMIC) {
+               /* A dynamic server is disabled on startup */
+               srv->next_admin = SRV_ADMF_FMAINT;
+               srv->next_state = SRV_ST_STOPPED;
+               server_recalc_eweight(srv, 0); // relies on srv counters
+               srv_lb_commit_status(srv);
+       }
+
        err_code |= init_srv_requeue(srv);
 
        if (err_code & ERR_CODE)
@@ -3646,14 +3653,8 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
                if (!(parse_flags & SRV_PARSE_DYNAMIC)) {
                        /* Copy default server settings to new server */
                        srv_settings_cpy(newsrv, &curproxy->defsrv, 0);
-               } else {
+               } else
                        srv_settings_init(newsrv);
-
-                       /* A dynamic server is disabled on startup */
-                       newsrv->next_admin = SRV_ADMF_FMAINT;
-                       newsrv->next_state = SRV_ST_STOPPED;
-                       server_recalc_eweight(newsrv, 0);
-               }
                HA_SPIN_INIT(&newsrv->lock);
        }
        else {