MEDIUM: cfgparse: drop duplicate named defaults sections after use

It has never been permitted to explicitly reference named defaults
sections for which there are duplicate names. This means that when
a duplicate defaults section is found, there's no point in keeping
it since it will never be used for lookups, so it can be dropped.

However, some such defaults sections might have some rules in them
that are implicitly referenced by proxies placed after them. In this
case they cannot be removed.

What is done here is that upon each new named section creation, if
another one is found with the same name, its config location is stored
into the new proxy's {prev_file,prev_line} pair, and the old section is
either destroyed if its refcount is null, or just unindexed. The dup
check when creating a new proxy now consists in checking the prev_line
instead of performing a dup lookup on the defaults section.

This will guarantee that we can't find duplicate defaults sections in
their tree anymore, while still keeping track of what's allocated and
releasing everything upon exit.

Beyond the consistency gain, there are nice savings for large configs
involving many defaults sections: a test with 300k sections saved
about 1.9 GB of RAM, and started 25% faster likely thanks to spending
less time allocating memory.

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index a1e01dd4e1f0ac262f3b859abb1f861afaef1ceb..eaca98899a6922e156a9c0848085afb8122dceda 100644 (file)
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -424,9 +424,11 @@ struct proxy {
                struct list listeners;          /* list of listeners belonging to this frontend */
                struct list errors;             /* list of all custom error files */
                struct arg_list args;           /* sample arg list that need to be resolved */
-               unsigned int refcount;          /* refcount on this proxy (only used for default proxy for now) */
                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 */
+               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) */
        } conf;                                 /* config information */
        struct http_ext *http_ext;              /* http ext options */
        struct eb_root used_server_addr;        /* list of server addresses in use */

diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index e190e6e16f509a19e29cbd66791cbe36ea4714ad..bb722b3eef3c59b5c83c00122320c52be76699b9 100644 (file)
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -229,6 +229,8 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
        struct acl_cond *cond = NULL;
        char *errmsg = NULL;
        struct bind_conf *bind_conf;
+       const char *file_prev = NULL;
+       int line_prev = 0;
 
        if (!last_defproxy) {
                /* we need a default proxy and none was created yet */
@@ -325,6 +327,18 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                                err_code |= ERR_ALERT | ERR_ABORT;
                                goto out;
                        }
+
+                       /* if the other proxy exists, we don't need to keep it
+                        * since neither will support being explicitly referenced
+                        * so let's drop it from the index but keep a reference to
+                        * its location for error messages.
+                        */
+                       if (curproxy) {
+                               file_prev = curproxy->conf.file;
+                               line_prev = curproxy->conf.line;
+                               proxy_unref_or_destroy_defaults(curproxy);
+                               curproxy = NULL;
+                       }
                }
 
                curproxy = proxy_find_by_name(args[1], 0, 0);
@@ -351,8 +365,6 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                        curr_defproxy = last_defproxy;
 
                if (strcmp(args[arg], "from") == 0) {
-                       struct ebpt_node *next_by_name;
-
                        curr_defproxy = proxy_find_by_name(args[arg+1], PR_CAP_DEF, 0);
 
                        if (!curr_defproxy) {
@@ -361,12 +373,11 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                                goto out;
                        }
 
-                       if ((next_by_name = ebpt_next_dup(&curr_defproxy->conf.by_name))) {
-                               struct proxy *px2 = container_of(next_by_name, struct proxy, conf.by_name);
-
+                       if (curr_defproxy->conf.line_prev) {
                                ha_alert("parsing [%s:%d] : ambiguous defaults section name '%s' referenced by %s '%s' exists at least at %s:%d and %s:%d.\n",
                                         file, linenum, args[arg+1], proxy_cap_str(rc), name,
-                                        curr_defproxy->conf.file, curr_defproxy->conf.line, px2->conf.file, px2->conf.line);
+                                        curr_defproxy->conf.file, curr_defproxy->conf.line,
+                                        curr_defproxy->conf.file_prev, curr_defproxy->conf.line_prev);
                                err_code |= ERR_ALERT | ERR_FATAL;
                        }
 
@@ -394,6 +405,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                        goto out;
                }
 
+               curproxy->conf.file_prev = file_prev;
+               curproxy->conf.line_prev = line_prev;
+
                if (curr_defproxy && (!LIST_ISEMPTY(&curr_defproxy->http_req_rules)        ||
                                      !LIST_ISEMPTY(&curr_defproxy->http_res_rules)        ||
                                      !LIST_ISEMPTY(&curr_defproxy->http_after_res_rules)  ||