MEDIUM: cfgparse: do not store unnamed defaults in name tree

Defaults section are indexed by their name in defproxy_by_name tree. For
named sections, there is no duplicate : if two instances have the same
name, the older one is removed from the tree. However, this was not the
case for unnamed defaults which are all stored inconditionnally in
defproxy_by_name.

This commit introduces a new approach for unnamed defaults. Now, these
instances are never inserted in the defproxy_by_name tree. Indeed, this
is not needed as no tree lookup is performed with empty names. This may
optimize slightly config parsing with a huge number of named and unnamed
defaults sections, as the first ones won't fill up the tree needlessly.

However, defproxy_by_name tree is also used to purge unreferenced
defaults instances, both on postparsing and deinit. Thus, a new approach
is needed for unnamed sections cleanup. Now, each time a new defaults is
parsed, if the previous instance is unnamed, it is freed unless if
referenced by a proxy. When config parsing is ended, a similar operation
is performed to ensure the last unnamed defaults section won't stay in
memory. To implement this, last_defproxy static variable is now set to
global. Unnamed sections which cannot be removed due to proxies
referencing proxies will still be removed when such proxies are freed
themselves, at runtime or on deinit.

diff --git a/include/haproxy/cfgparse.h b/include/haproxy/cfgparse.h
index 56535b783b31b84e1ba09308d13a007f2650be3d..161b7d31d6a0a47f9fb5f3c3681fd29380c12769 100644 (file)
--- a/include/haproxy/cfgparse.h
+++ b/include/haproxy/cfgparse.h
@@ -111,6 +111,7 @@ extern char *cursection;
 extern int non_global_section_parsed;
 
 extern struct proxy *curproxy;
+extern struct proxy *last_defproxy;
 extern char initial_cwd[PATH_MAX];
 
 int cfg_parse_global(const char *file, int linenum, char **args, int inv);

diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index ebea793856203888bd46d8c655a3be19bba04d80..bcb085936bf36089994beb978a7bf6021ccc44f6 100644 (file)
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -299,7 +299,6 @@ int cfg_parse_listen_match_option(const char *file, int linenum, int kwm,
 int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
 {
        static struct proxy *curr_defproxy = NULL;
-       static struct proxy *last_defproxy = NULL;
        const char *err;
        int rc;
        int err_code = 0;
@@ -388,35 +387,49 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
                        err_code |= ERR_ALERT | ERR_FATAL;
                }
 
-               if (*args[1] && rc & PR_CAP_DEF) {
-                       /* for default proxies, if another one has the same
-                        * name and was explicitly referenced, this is an error
-                        * that we must reject. E.g.
-                        *     defaults def
-                        *     backend bck from def
-                        *     defaults def
+               if (rc & PR_CAP_DEF) {
+                       /* If last defaults is unnamed, it will be made
+                        * invisible by the current newer section. It must be
+                        * freed unless it is still referenced by proxies.
                         */
-                       curproxy = proxy_find_by_name(args[1], PR_CAP_DEF, 0);
-                       if (curproxy && curproxy->flags & PR_FL_EXPLICIT_REF) {
-                               ha_alert("Parsing [%s:%d]: %s '%s' has the same name as another defaults section declared at"
-                                        " %s:%d which was explicitly referenced hence cannot be replaced. Please remove or"
-                                        " rename one of the offending defaults section.\n",
-                                        file, linenum, proxy_cap_str(rc), args[1],
-                                        curproxy->conf.file, curproxy->conf.line);
-                               err_code |= ERR_ALERT | ERR_ABORT;
-                               goto out;
-                       }
+                       if (last_defproxy && last_defproxy->id[0] == '\0' &&
+                           !last_defproxy->conf.refcount) {
+                               defaults_px_destroy(last_defproxy);
+                       }
+                       last_defproxy = NULL;
+
+                       /* If current defaults is named, check collision with previous instances. */
+                       if (*args[1]) {
+                               curproxy = proxy_find_by_name(args[1], PR_CAP_DEF, 0);
+
+                               /* for default proxies, if another one has the same
+                                * name and was explicitly referenced, this is an error
+                                * that we must reject. E.g.
+                                *     defaults def
+                                *     backend bck from def
+                                *     defaults def
+                                */
+                               if (curproxy && curproxy->flags & PR_FL_EXPLICIT_REF) {
+                                       ha_alert("Parsing [%s:%d]: %s '%s' has the same name as another defaults section declared at"
+                                                " %s:%d which was explicitly referenced hence cannot be replaced. Please remove or"
+                                                " rename one of the offending defaults section.\n",
+                                                file, linenum, proxy_cap_str(rc), args[1],
+                                                curproxy->conf.file, curproxy->conf.line);
+                                       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;
-                               defaults_px_detach(curproxy);
-                               curproxy = NULL;
+                               /* 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;
+                                       defaults_px_detach(curproxy);
+                                       curproxy = NULL;
+                               }
                        }
                }
 

diff --git a/src/cfgparse.c b/src/cfgparse.c
index edc9c518fb8fc87d2a8eeb99cedcfcabec16730d..1940330a780debf500a8f891d34fb373953769f4 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -110,6 +110,8 @@ extern struct proxy *mworker_proxy;
 
 /* curproxy is only valid during parsing and will be NULL afterwards. */
 struct proxy *curproxy = NULL;
+/* last defaults section parsed, NULL after parsing */
+struct proxy *last_defproxy = NULL;
 
 char *cursection = NULL;
 int cfg_maxpconn = 0;                   /* # of simultaneous connections per proxy (-N) */

diff --git a/src/haproxy.c b/src/haproxy.c
index 95b2019d912bf5ec8d5fac05a1097f396209ae32..929a86537dbae1c66356cc0a513441b5d2694eee 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2098,6 +2098,13 @@ static void step_init_2(int argc, char** argv)
        struct pre_check_fct *prcf;
        const char *cc, *cflags, *opts;
 
+       /* Free last defaults if it is unnamed and unreferenced. */
+       if (last_defproxy && last_defproxy->id[0] == '\0' &&
+           !last_defproxy->conf.refcount) {
+               defaults_px_destroy(last_defproxy);
+       }
+       last_defproxy = NULL; /* This variable is not used after parsing. */
+
        /* destroy unreferenced defaults proxies  */
        defaults_px_destroy_all_unref();
 

diff --git a/src/proxy.c b/src/proxy.c
index b3aa601455f0e0151f814be704f0a7bd1faa50a4..d828ded9c2237c3f3470778bac9efe67fdd09b4c 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1718,7 +1718,8 @@ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char *
        px->cap = cap;
        px->last_change = ns_to_sec(now_ns);
 
-       if (name && !(cap & PR_CAP_INT))
+       /* Internal proxies or with empty name are not stored in the named tree. */
+       if (name && name[0] != '\0' && !(cap & PR_CAP_INT))
                proxy_store_name(px);
 
        if (!(cap & PR_CAP_DEF))