MEDIUM: map: make the "clear map" operation yield

As reported in issue #419, a "clear map" operation on a very large map
can take a lot of time and freeze the entire process for several seconds.

This patch makes sure that pat_ref_prune() can regularly yield after
clearing some entries so that the rest of the process continues to work.
The first part, the removal of the patterns, can take quite some time
by itself in one run but it's still relatively fast. It may block for
up to 100ms for 16M IP addresses in a tree typically. This change needed
to declare an I/O handler for the clear operation so that we can get
back to it after yielding.

The second part can be much slower because it deconstructs the elements
and its users, but it iterates progressively so we can yield less often
here.

The patch was tested with traffic in parallel sollicitating the map being
released and showed no problem. Some traffic will definitely notice an
incomplete map but the filling is already not atomic anyway thus this is
not different.

It may be backported to stable versions once sufficiently tested for side
effects, at least as far as 2.0 in order to avoid the watchdog triggering
when the process is frozen there. For a better behaviour, all these
prune_* functions should support yielding so that the callers have a
chance to continue also yield in turn.

diff --git a/include/haproxy/pattern.h b/include/haproxy/pattern.h
index e1c246c54f28f74ae81cc22919e301da7a56ced8..62fb5c04ce8f8f2b2dc9b6af8c2420c08dcf70a3 100644 (file)
--- a/include/haproxy/pattern.h
+++ b/include/haproxy/pattern.h
@@ -195,7 +195,7 @@ int pat_ref_set(struct pat_ref *ref, const char *pattern, const char *sample, ch
 int pat_ref_set_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt, const char *value, char **err);
 int pat_ref_delete(struct pat_ref *ref, const char *key);
 int pat_ref_delete_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt);
-void pat_ref_prune(struct pat_ref *ref);
+int pat_ref_prune(struct pat_ref *ref);
 int pat_ref_load(struct pat_ref *ref, struct pattern_expr *expr, int patflags, int soe, char **err);
 void pat_ref_reload(struct pat_ref *ref, struct pat_ref *replace);
 

diff --git a/src/map.c b/src/map.c
index adc857ba9d82a6306c46f246b1689e9b0ecb7495..405a84b54c6b765b793c51511f66430865ec9851 100644 (file)
--- a/src/map.c
+++ b/src/map.c
@@ -919,6 +919,24 @@ static int cli_parse_del_map(char **args, char *payload, struct appctx *appctx,
 }
 
 
+/* continue to clear a map which was started in the parser */
+static int cli_io_handler_clear_map(struct appctx *appctx)
+{
+       struct stream_interface *si = appctx->owner;
+       int finished;
+
+       HA_SPIN_LOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock);
+       finished = pat_ref_prune(appctx->ctx.map.ref);
+       HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock);
+
+       if (!finished) {
+               /* let's come back later */
+               si_rx_endp_more(si);
+               return 0;
+       }
+       return 1;
+}
+
 static int cli_parse_clear_map(char **args, char *payload, struct appctx *appctx, void *private)
 {
        if (strcmp(args[1], "map") == 0 || strcmp(args[1], "acl") == 0) {
@@ -946,28 +964,22 @@ static int cli_parse_clear_map(char **args, char *payload, struct appctx *appctx
                                return cli_err(appctx, "Unknown ACL identifier. Please use #<id> or <file>.\n");
                }
 
-               /* Clear all. */
-               HA_SPIN_LOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock);
-               pat_ref_prune(appctx->ctx.map.ref);
-               HA_SPIN_UNLOCK(PATREF_LOCK, &appctx->ctx.map.ref->lock);
-
-               /* return response */
-               appctx->st0 = CLI_ST_PROMPT;
-               return 1;
+               /* delegate the clearing to the I/O handler which can yield */
+               return 0;
        }
-       return 0;
+       return 1;
 }
 
 /* register cli keywords */
 
 static struct cli_kw_list cli_kws = {{ },{
        { { "add",   "acl", NULL }, "add acl        : add acl entry", cli_parse_add_map, NULL },
-       { { "clear", "acl", NULL }, "clear acl <id> : clear the content of this acl", cli_parse_clear_map, NULL },
+       { { "clear", "acl", NULL }, "clear acl <id> : clear the content of this acl", cli_parse_clear_map, cli_io_handler_clear_map, NULL },
        { { "del",   "acl", NULL }, "del acl        : delete acl entry", cli_parse_del_map, NULL },
        { { "get",   "acl", NULL }, "get acl        : report the patterns matching a sample for an ACL", cli_parse_get_map, cli_io_handler_map_lookup, cli_release_mlook },
        { { "show",  "acl", NULL }, "show acl [id]  : report available acls or dump an acl's contents", cli_parse_show_map, NULL },
        { { "add",   "map", NULL }, "add map        : add map entry", cli_parse_add_map, NULL },
-       { { "clear", "map", NULL }, "clear map <id> : clear the content of this map", cli_parse_clear_map, NULL },
+       { { "clear", "map", NULL }, "clear map <id> : clear the content of this map", cli_parse_clear_map, cli_io_handler_clear_map, NULL },
        { { "del",   "map", NULL }, "del map        : delete map entry", cli_parse_del_map, NULL },
        { { "get",   "map", NULL }, "get map        : report the keys and values matching a sample for a map", cli_parse_get_map, cli_io_handler_map_lookup, cli_release_mlook },
        { { "set",   "map", NULL }, "set map        : modify map entry", cli_parse_set_map, NULL },

diff --git a/src/pattern.c b/src/pattern.c
index bd8b25a6185e7e26f25287069bcfc17a14cd1b32..fbb5ae671087fdf036d54fce55a86cf947c8661a 100644 (file)
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -2098,18 +2098,27 @@ void pat_ref_reload(struct pat_ref *ref, struct pat_ref *replace)
 }
 
 /* This function prune all entries of <ref>. This function
- * prune the associated pattern_expr.
+ * prunes the associated pattern_expr. It may return before the end of
+ * the list is reached, returning 0, to yield. The caller must call it
+ * again. Otherwise it returns 1 once done.
  */
-void pat_ref_prune(struct pat_ref *ref)
+int pat_ref_prune(struct pat_ref *ref)
 {
        struct pat_ref_elt *elt, *safe;
        struct pattern_expr *expr;
        struct bref *bref, *back;
+       int loops = 0;
 
        list_for_each_entry(expr, &ref->pat, list) {
                HA_RWLOCK_WRLOCK(PATEXP_LOCK, &expr->lock);
                expr->pat_head->prune(expr);
                HA_RWLOCK_WRUNLOCK(PATEXP_LOCK, &expr->lock);
+               loops++;
+               /* yield often, some lists may be huge, especially those
+                * having to be freed through free_pattern_tree()
+                */
+               if (loops > 10)
+                       return 0;
        }
 
        /* we trash pat_ref_elt in a second time to ensure that data is
@@ -2130,9 +2139,11 @@ void pat_ref_prune(struct pat_ref *ref)
                free(elt->pattern);
                free(elt->sample);
                free(elt);
+               loops++;
+               if (loops > 100000)
+                       return 0;
        }
-
-
+       return 1;
 }
 
 /* This function lookup for existing reference <ref> in pattern_head <head>. */