From 59459fc32a423eb1650a1205c10eded0a1d4fa04 Mon Sep 17 00:00:00 2001 From: phk Date: Sat, 31 May 2008 21:26:20 +0000 Subject: [PATCH] Overhaul the regexp purge/ban code, with a detour around the CLI code. CLI code: In CLI help, don't list commands with no syntax description. Add a CLI_HIDDEN macro to construct such entries. They are useful as backwards compatibility entries which we do not want to show in the help. CLI interface to BAN (purge) code: Get the CLI names right for purging so they are purge.FOO instead of FOO.purge. This means that you should use "purge.url" and "purge.hash" instead of "url.purge" and "hash.purge". Add compat entries for the old, and keep them through the 2.x release series. Add purge.list command to list purges currently in effect. NB: This is not 100% locking safe, so don't abuse it. BAN (purge) code: Add reference counting and GC to bans. Since we now have full reference counting, drop the sequence number based soft references and go to "hard" pointer references from the object to the purges. Give the "ban" structure the miniobj treatment while we are at it. The cost of this is a lock operation to update refcounts once all applicable bans have been checked on an object. There is no locking cost if there is no bans to check. Add explicit call to new BAN_DestroyObj() when objects are destroyed to release the refcount. When we release an object refcount in BAN_DestroyObj(), check if we can destroy the last purge in the list also. We only destroy one ban per BAN_DestroyObj() call, to avoid getting stuck too long time, (tacitly assuming that we will destroy more objects than bans.) git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@2645 d4fa192b-c00b-0410-8231-f00ffab90ce4 --- varnish-cache/bin/varnishd/cache.h | 4 +- varnish-cache/bin/varnishd/cache_ban.c | 152 ++++++++++++++++++++---- varnish-cache/bin/varnishd/cache_hash.c | 5 + varnish-cache/include/cli.h | 23 ++-- varnish-cache/lib/libvarnish/cli.c | 5 +- 5 files changed, 155 insertions(+), 34 deletions(-) diff --git a/varnish-cache/bin/varnishd/cache.h b/varnish-cache/bin/varnishd/cache.h index 3aaabb2e..e52b0b69 100644 --- a/varnish-cache/bin/varnishd/cache.h +++ b/varnish-cache/bin/varnishd/cache.h @@ -83,6 +83,7 @@ struct addrinfo; struct esi_bit; struct vrt_backend; struct cli_proto; +struct ban; /*--------------------------------------------------------------------*/ @@ -244,7 +245,7 @@ struct object { struct ws ws_o[1]; unsigned char *vary; - unsigned ban_seq; + struct ban *ban; unsigned pass; @@ -420,6 +421,7 @@ void VBE_SelectBackend(struct sess *sp); void AddBan(const char *, int hash); void BAN_Init(void); void BAN_NewObj(struct object *o); +void BAN_DestroyObj(struct object *o); int BAN_CheckObject(struct object *o, const char *url, const char *hash); /* cache_center.c [CNT] */ diff --git a/varnish-cache/bin/varnishd/cache_ban.c b/varnish-cache/bin/varnishd/cache_ban.c index 5dcbccd3..9536af18 100644 --- a/varnish-cache/bin/varnishd/cache_ban.c +++ b/varnish-cache/bin/varnishd/cache_ban.c @@ -28,7 +28,7 @@ * * $Id$ * - * Ban processing + * Ban ("purge") processing */ #include "config.h" @@ -45,16 +45,25 @@ #include "cache.h" struct ban { + unsigned magic; +#define BAN_MAGIC 0x700b08ea VTAILQ_ENTRY(ban) list; - unsigned gen; + unsigned refcount; regex_t regexp; char *ban; int hash; }; -static VTAILQ_HEAD(,ban) ban_head = VTAILQ_HEAD_INITIALIZER(ban_head); -static unsigned ban_next; -static struct ban *ban_start; +static VTAILQ_HEAD(banhead,ban) ban_head = VTAILQ_HEAD_INITIALIZER(ban_head); +static MTX ban_mtx; + +/* + * We maintain ban_start as a pointer to the first element of the list + * as a separate variable from the VTAILQ, to avoid depending on the + * internals of the VTAILQ macros. We tacitly assume that a pointer + * write is always atomic in doing so. + */ +static struct ban * volatile ban_start; void AddBan(const char *regexp, int hash) @@ -62,7 +71,7 @@ AddBan(const char *regexp, int hash) struct ban *b; int i; - b = calloc(sizeof *b, 1); + ALLOC_OBJ(b, BAN_MAGIC); XXXAN(b); i = regcomp(&b->regexp, regexp, REG_EXTENDED | REG_ICASE | REG_NOSUB); @@ -71,60 +80,152 @@ AddBan(const char *regexp, int hash) (void)regerror(i, &b->regexp, buf, sizeof buf); VSL(SLT_Debug, 0, "REGEX: <%s>", buf); + return; } b->hash = hash; - b->gen = ++ban_next; b->ban = strdup(regexp); + AN(b->ban); + LOCK(&ban_mtx); VTAILQ_INSERT_HEAD(&ban_head, b, list); ban_start = b; + UNLOCK(&ban_mtx); } void BAN_NewObj(struct object *o) { - o->ban_seq = ban_next; + CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC); + AZ(o->ban); + LOCK(&ban_mtx); + o->ban = ban_start; + ban_start->refcount++; + UNLOCK(&ban_mtx); +} + +void +BAN_DestroyObj(struct object *o) +{ + struct ban *b; + + CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC); + if (o->ban == NULL) + return; + CHECK_OBJ_NOTNULL(o->ban, BAN_MAGIC); + LOCK(&ban_mtx); + o->ban->refcount--; + o->ban = NULL; + + /* Check if we can purge the last ban entry */ + b = VTAILQ_LAST(&ban_head, banhead); + if (b != VTAILQ_FIRST(&ban_head) && b->refcount == 0) + VTAILQ_REMOVE(&ban_head, b, list); + else + b = NULL; + UNLOCK(&ban_mtx); + if (b != NULL) { + free(b->ban); + regfree(&b->regexp); + FREE_OBJ(b); + } + } int BAN_CheckObject(struct object *o, const char *url, const char *hash) { - struct ban *b, *b0; - int i; + struct ban *b; + struct ban * volatile b0; + + CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC); + CHECK_OBJ_NOTNULL(o->ban, BAN_MAGIC); b0 = ban_start; - for (b = b0; - b != NULL && b->gen > o->ban_seq; - b = VTAILQ_NEXT(b, list)) { - i = regexec(&b->regexp, b->hash ? hash : url, 0, NULL, 0); - if (!i) - return (1); + + if (b0 == o->ban) + return (0); + + /* + * This loop is safe without locks, because we know we hold + * a refcount on a ban somewhere in the list and we do not + * inspect the list past that ban. + */ + for (b = b0; b != o->ban; b = VTAILQ_NEXT(b, list)) { + if (!regexec(&b->regexp, b->hash ? hash : url, 0, NULL, 0)) + break; + } + + LOCK(&ban_mtx); + o->ban->refcount--; + if (b == o->ban) /* not banned */ + b0->refcount++; + UNLOCK(&ban_mtx); + + if (b == o->ban) { /* not banned */ + o->ban = b0; + return (0); + } else { + o->ban = NULL; + return (1); } - o->ban_seq = b0->gen; - return (0); } +/*-------------------------------------------------------------------- + * CLI functions to add bans + */ + static void -ccf_url_purge(struct cli *cli, const char * const *av, void *priv) +ccf_purge_url(struct cli *cli, const char * const *av, void *priv) { (void)priv; AddBan(av[2], 0); - cli_out(cli, "PURGE %s\n", av[2]); + cli_out(cli, "URL_PURGE %s\n", av[2]); } static void -ccf_hash_purge(struct cli *cli, const char * const *av, void *priv) +ccf_purge_hash(struct cli *cli, const char * const *av, void *priv) { (void)priv; AddBan(av[2], 1); - cli_out(cli, "PURGE %s\n", av[2]); + cli_out(cli, "HASH_PURGE %s\n", av[2]); +} + +static void +ccf_purge_list(struct cli *cli, const char * const *av, void *priv) +{ + struct ban * volatile b0; + + (void)av; + (void)priv; + /* + * XXX: Strictly speaking, this loop traversal is not lock-safe + * XXX: because we might inspect the last ban while it gets + * XXX: destroyed. To properly fix this, we would need to either + * XXX: hold the lock over the entire loop, or grab refcounts + * XXX: under lock for each element of the list. + * XXX: We do neither, and hope for the best. + */ + for (b0 = ban_start; b0 != NULL; b0 = VTAILQ_NEXT(b0, list)) { + if (b0->refcount == 0 && VTAILQ_NEXT(b0, list) == NULL) + break; + cli_out(cli, "%5u %s \"%s\"\n", + b0->refcount, b0->hash ? "hash" : "url ", b0->ban); + } } static struct cli_proto ban_cmds[] = { - { CLI_URL_PURGE, ccf_url_purge }, - { CLI_HASH_PURGE, ccf_hash_purge }, + /* + * XXX: COMPAT: Retain these two entries for entire 2.x series + * XXX: COMPAT: to stay compatible with 1.x series syntax. + */ + { CLI_HIDDEN("url.purge", 1, 1) ccf_purge_url }, + { CLI_HIDDEN("hash.purge", 1, 1) ccf_purge_hash }, + + { CLI_PURGE_URL, ccf_purge_url }, + { CLI_PURGE_HASH, ccf_purge_hash }, + { CLI_PURGE_LIST, ccf_purge_list }, { NULL } }; @@ -132,6 +233,7 @@ void BAN_Init(void) { + MTX_INIT(&ban_mtx); CLI_AddFuncs(PUBLIC_CLI, ban_cmds); - AddBan("\001", 0); + AddBan("^\001$", 0); } diff --git a/varnish-cache/bin/varnishd/cache_hash.c b/varnish-cache/bin/varnishd/cache_hash.c index d0bd0952..439e0a21 100644 --- a/varnish-cache/bin/varnishd/cache_hash.c +++ b/varnish-cache/bin/varnishd/cache_hash.c @@ -263,6 +263,10 @@ HSH_Lookup(struct sess *sp) grace_o->refcnt++; } UNLOCK(&oh->mtx); + /* + * XXX: This may be too early, relative to pass objects. + * XXX: possibly move to when we commit to have it in the cache. + */ BAN_NewObj(o); return (o); } @@ -364,6 +368,7 @@ HSH_Deref(struct object *o) if (r != 0) return; + BAN_DestroyObj(o); DSL(0x40, SLT_Debug, 0, "Object %u workspace min free %u", o->xid, WS_Free(o->ws_o)); diff --git a/varnish-cache/include/cli.h b/varnish-cache/include/cli.h index 7b13918f..be3d7311 100644 --- a/varnish-cache/include/cli.h +++ b/varnish-cache/include/cli.h @@ -60,20 +60,26 @@ "\tReturns the TTL, size and checksum of the object.", \ 1, 1 -#define CLI_URL_PURGE \ - "url.purge", \ - "url.purge ", \ +#define CLI_PURGE_URL \ + "purge.url", \ + "purge.url ", \ "\tAll objects where the urls matches regexp will be " \ "marked obsolete.", \ 1, 1 -#define CLI_HASH_PURGE \ - "hash.purge", \ - "hash.purge ", \ +#define CLI_PURGE_HASH \ + "purge.hash", \ + "purge.hash ", \ "\tAll objects where the hash string matches regexp will be " \ "marked obsolete.", \ 1, 1 +#define CLI_PURGE_LIST \ + "purge.list", \ + "purge.list", \ + "\tList the active purges.", \ + 0, 0 + #define CLI_URL_STATUS \ "url.status", \ "url.status ", \ @@ -206,12 +212,15 @@ "\tClose connection", \ 0, 0 -# define CLI_SERVER_STATUS \ +#define CLI_SERVER_STATUS \ "status", \ "status", \ "\tCheck status of Varnish cache process.", \ 0, 0 +#define CLI_HIDDEN(foo, min_arg, max_arg) \ + foo, NULL, NULL, min_arg, max_arg, + /* * Status/return codes in the CLI protocol */ diff --git a/varnish-cache/lib/libvarnish/cli.c b/varnish-cache/lib/libvarnish/cli.c index eb04d091..4b42d663 100644 --- a/varnish-cache/lib/libvarnish/cli.c +++ b/varnish-cache/lib/libvarnish/cli.c @@ -55,10 +55,13 @@ cli_func_help(struct cli *cli, const char * const *av, void *priv) if (av[2] == NULL || *av[2] == '-') { for (cp = priv; cp->request != NULL; cp++) - cli_out(cli, "%s\n", cp->syntax); + if (cp->syntax != NULL) + cli_out(cli, "%s\n", cp->syntax); return; } for (cp = priv; cp->request != NULL; cp++) { + if (cp->syntax == NULL) + continue; if (!strcmp(cp->request, av[2])) { cli_out(cli, "%s\n%s\n", cp->syntax, cp->help); return; -- 2.39.5