MEDIUM: log/proxy: store log-steps selection using a bitmask, not an eb tree

An eb tree was used to anticipate for infinite amount of custom log steps
configured at a proxy level. In turns out this makes no sense to configure
that much logging steps for a proxy, and the cost of the eb tree is non
negligible in terms of memory footprint, especially when used in a default
section.

Instead, let's use a simple bitmask, which allows up to 64 logging steps
configured at proxy level. If we lack space some day (and need more than
64 logging steps to be configured), we could simply modify
"struct log_steps" to spread the bitmask over multiple 64bits integers,
minor some adjustments where the mask is set and checked.

diff --git a/include/haproxy/log-t.h b/include/haproxy/log-t.h
index a2cd187548ab5676d80b68a77c518ffdf188fc7a..7bca3b7a611d6550e57aab45ecada1b20f881fea 100644 (file)
--- a/include/haproxy/log-t.h
+++ b/include/haproxy/log-t.h
@@ -335,6 +335,13 @@ struct log_profile {
        struct eb_root extra;           // extra log profile steps (if any)
 };
 
+/* add additional bitmasks in this struct if needed but don't
+ * forget to update px_parse_log_steps() and log_orig_proxy() accordingly
+ */
+struct log_steps {
+       uint64_t steps_1; // first 64 steps
+};
+
 #endif /* _HAPROXY_LOG_T_H */
 
 /*

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 2d4971012b70783cc9c8371311e6b80049620163..cb6fc318a031df33c4f643dab32a4f353e019386 100644 (file)
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -470,7 +470,7 @@ struct proxy {
                struct arg_list args;           /* sample arg list that need to be resolved */
                struct ebpt_node by_name;       /* proxies are stored sorted by name here */
                struct list lf_checks;          /* list of logformats found in the proxy section that needs to be checked during postparse */
-               struct eb_root log_steps;       /* tree of log origins where log should be generated during request handling */
+               struct log_steps log_steps;     /* bitfield of log origins where log should be generated during request handling */
                const char *file_prev;          /* file of the previous instance found with the same name, or NULL */
                int line_prev;                  /* line of the previous instance found with the same name, or 0 */
                unsigned int refcount;          /* refcount on this proxy (only used for default proxy for now) */

diff --git a/src/log.c b/src/log.c
index c0d4392391b9dee4868bbf87ac349a415e5a5cb5..fc6aeb9d1b94c8a7826909e04deda0d60260b9dc 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -137,8 +137,8 @@ const char *log_orig_to_str(enum log_orig_id orig)
  */
 int log_orig_proxy(enum log_orig_id orig, struct proxy *px)
 {
-       if (eb_is_empty(&px->conf.log_steps)) {
-               /* empty tree means all log steps are enabled, thus
+       if (px->conf.log_steps.steps_1 == 0) {
+               /* logstep bitmasks to 0 means all log steps are enabled, thus
                 * all log origins are considered
                 */
                return 1;
@@ -146,7 +146,7 @@ int log_orig_proxy(enum log_orig_id orig, struct proxy *px)
        /* selectively check if the current log origin is referenced in
         * proxy log-steps
         */
-       return !!eb32_lookup(&px->conf.log_steps, orig);
+       return (px->conf.log_steps.steps_1 & (1ULL << orig));
 }
 
 /*
@@ -6893,7 +6893,8 @@ static int px_parse_log_steps(char **args, int section_type, struct proxy *curpx
                        goto end;
                }
                cur_step->key = cur_id;
-               eb32_insert(&curpx->conf.log_steps, cur_step);
+               BUG_ON(cur_id > 64); // for now we don't support more than 64 log origins
+               curpx->conf.log_steps.steps_1 |= (1ULL << cur_id);
  next:
                if (str[cur_sep])
                        str += cur_sep + 1;

diff --git a/src/proxy.c b/src/proxy.c
index 8c3bbaac2b5365af2a5c573fe9c645e52e521d16..7030eb80318a444abd870bd9df41765a7f7e90a2 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -215,7 +215,6 @@ static inline void proxy_free_common(struct proxy *px)
        struct acl *acl, *aclb;
        struct logger *log, *logb;
        struct lf_expr *lf, *lfb;
-       struct eb32_node *node;
 
        /* note that the node's key points to p->id */
        ebpt_delete(&px->conf.by_name);
@@ -278,16 +277,6 @@ static inline void proxy_free_common(struct proxy *px)
 
        chunk_destroy(&px->log_tag);
 
-       node = eb32_first(&px->conf.log_steps);
-       while (node) {
-               struct eb32_node *prev_node = node;
-
-               /* log steps directly use the node key as id, they are not encapsulated */
-               node = eb32_next(node);
-               eb32_delete(prev_node);
-               free(prev_node);
-       }
-
        free_email_alert(px);
        stats_uri_auth_drop(px->uri_auth);
        px->uri_auth = NULL;
@@ -1483,7 +1472,6 @@ void init_new_proxy(struct proxy *p)
        p->conf.used_listener_id = EB_ROOT;
        p->conf.used_server_id   = EB_ROOT;
        p->used_server_addr      = EB_ROOT_UNIQUE;
-       p->conf.log_steps        = EB_ROOT_UNIQUE;
 
        /* Timeouts are defined as -1 */
        proxy_reset_timeouts(p);
@@ -1808,7 +1796,6 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro
 {
        struct logger *tmplogger;
        char *tmpmsg = NULL;
-       struct eb32_node *node = NULL;
 
        /* set default values from the specified default proxy */
 
@@ -2037,25 +2024,8 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro
        curproxy->email_alert.level = defproxy->email_alert.level;
        curproxy->email_alert.flags = defproxy->email_alert.flags;
 
-       /* defproxy is const pointer, so we need to typecast log_steps to
-        * drop the const in order to use EB tree API, please note however
-        * that the operations performed below should theoretically be read-only
-        */
        if (curproxy->cap & PR_CAP_FE) // don't inherit on backends
-               node = eb32_first((struct eb_root *)&defproxy->conf.log_steps);
-       while (node) {
-               struct eb32_node *new_node;
-
-               new_node = malloc(sizeof(*new_node));
-               if (!new_node) {
-                       memprintf(errmsg, "proxy '%s': out of memory for log_steps option", curproxy->id);
-                       return 1;
-               }
-
-               new_node->key = node->key;
-               eb32_insert(&curproxy->conf.log_steps, new_node);
-               node = eb32_next(node);
-       }
+               curproxy->conf.log_steps = defproxy->conf.log_steps;
 
        return 0;
 }