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.
(cherry picked from commit d1d005d7f6b894ed243c177937626d888b3d03e1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 5baab7460cf976c26a0afa8433ff41a36b5d9a4e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/include/proto/pattern.h b/include/proto/pattern.h
index 73d3cdc..1480d79 100644
--- a/include/proto/pattern.h
+++ b/include/proto/pattern.h
@@ -191,7 +191,7 @@
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 1a2190d..16b1c5f 100644
--- a/src/map.c
+++ b/src/map.c
@@ -1041,6 +1041,24 @@
}
+/* 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) {
@@ -1080,28 +1098,22 @@
return 1;
}
- /* 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 a82900b..3dbc285 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -2100,18 +2100,27 @@
}
/* 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
@@ -2132,9 +2141,11 @@
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>. */