From 002e660cde6443fc83641be83dca76f42b7a312f Mon Sep 17 00:00:00 2001 From: Walid Boudebouda Date: Wed, 15 Jan 2025 14:03:03 +0100 Subject: [PATCH 1/4] param: Introduce new ban_any_variant parameter This will control the number of object variants that we evaluate against the ban list before looking for a vary match. Refs: #4236 --- include/tbl/params.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/include/tbl/params.h b/include/tbl/params.h index 7c093baa95e..cf342992146 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -721,6 +721,20 @@ PARAM_SIMPLE( /* flags */ EXPERIMENTAL ) +PARAM_SIMPLE( + /* name */ ban_any_variant, + /* type */ uint, + /* min */ "0", + /* max */ NULL, + /* def */ "10000", + /* units */ "checks", + /* descr */ + "Maximum number of possibly non matching variants that we evaluate " + "against the ban list during a lookup.\n" + "Setting this to 0 means that only the matching variants will be " + "evaluated against the current ban list." +) + PARAM_SIMPLE( /* name */ max_esi_depth, /* type */ uint, From 5835761b0c800028f31fbcbfd5f6585e985aa2cc Mon Sep 17 00:00:00 2001 From: Walid Boudebouda Date: Wed, 15 Jan 2025 14:50:55 +0100 Subject: [PATCH 2/4] HSH: Limit the number of ban checks before vary matching Refs: #4236 --- bin/varnishd/cache/cache_hash.c | 14 ++++++++++- bin/varnishtest/tests/c00133.vtc | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 bin/varnishtest/tests/c00133.vtc diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c index 6446f3beb00..1ebc2700594 100644 --- a/bin/varnishd/cache/cache_hash.c +++ b/bin/varnishd/cache/cache_hash.c @@ -371,6 +371,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) const uint8_t *vary; intmax_t boc_progress; unsigned xid = 0; + unsigned ban_checks; + unsigned ban_any_variant; float dttl = 0.0; AN(ocp); @@ -420,6 +422,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) busy_found = 0; exp_oc = NULL; exp_t_origin = 0.0; + ban_checks = 0; + ban_any_variant = cache_param->ban_any_variant; VTAILQ_FOREACH(oc, &oh->objcs, hsh_list) { /* Must be at least our own ref + the objcore we examine */ assert(oh->refcnt > 1); @@ -451,7 +455,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) if (oc->ttl <= 0.) continue; - if (BAN_CheckObject(wrk, oc, req)) { + if (ban_checks++ < ban_any_variant + && BAN_CheckObject(wrk, oc, req)) { oc->flags |= OC_F_DYING; EXP_Remove(oc, NULL); continue; @@ -466,6 +471,13 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) } } + if (ban_checks >= ban_any_variant + && BAN_CheckObject(wrk, oc, req)) { + oc->flags |= OC_F_DYING; + EXP_Remove(oc, NULL); + continue; + } + if (req->vcf != NULL) { vr = req->vcf->func(req, &oc, &exp_oc, 0); if (vr == VCF_CONTINUE) diff --git a/bin/varnishtest/tests/c00133.vtc b/bin/varnishtest/tests/c00133.vtc new file mode 100644 index 00000000000..a52174867a7 --- /dev/null +++ b/bin/varnishtest/tests/c00133.vtc @@ -0,0 +1,41 @@ +varnishtest "test ban + vary behavior" + +server s0 { + rxreq + txresp -hdr "vary: version" -body "Variant A" + rxreq + txresp -hdr "vary: version" -body "Variant B" + rxreq + txresp -hdr "vary: version" -body "New variant A" + rxreq + txresp -hdr "vary: version" -body "New variant B" +} -start + +varnish v1 -arg "-p ban_any_variant=0" -vcl+backend {} -start + +client c1 { + txreq -hdr "version: a" + rxresp + expect resp.body == "Variant A" +} -run + +client c2 { + txreq -hdr "version: b" + rxresp + expect resp.body == "Variant B" +} -run + +varnish v1 -cliok "ban req.http.version == a" + +# Should this remove a single variant from cache +client c3 { + txreq -hdr "version: a" + rxresp + expect resp.body == "New variant A" +} -run + +client c4 { + txreq -hdr "version: b" + rxresp + expect resp.body == "Variant B" +} -run From d601dd9659e3ca14e0c200e1db4b51e87306855c Mon Sep 17 00:00:00 2001 From: Sergei Sobolev Date: Sun, 24 Nov 2024 14:18:26 -0600 Subject: [PATCH 3/4] HSH: Add more coverage for ban_any_variant Refs: #4236 --- bin/varnishtest/tests/c00134.vtc | 86 ++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 bin/varnishtest/tests/c00134.vtc diff --git a/bin/varnishtest/tests/c00134.vtc b/bin/varnishtest/tests/c00134.vtc new file mode 100644 index 00000000000..67de581bc23 --- /dev/null +++ b/bin/varnishtest/tests/c00134.vtc @@ -0,0 +1,86 @@ +varnishtest "Optimized HSH_Lookup - ban is checked after Vary is matched" + +server s1 { + rxreq + expect req.url == /foo + expect req.http.foobar == "1" + txresp -hdr "Vary: Foobar" -body "1111" + + rxreq + expect req.url == /foo + expect req.http.foobar == "2" + txresp -hdr "Vary: Foobar" -body "2222" + + rxreq + expect req.url == /foo + expect req.http.foobar == "3" + txresp -hdr "Vary: Foobar" -body "3333" + + rxreq + expect req.url == /foo + expect req.http.foobar == "1" + txresp -hdr "Vary: Foobar" -body "1111" + + rxreq + expect req.url == /foo + expect req.http.foobar == "1" + txresp -hdr "Vary: Foobar" -body "1111" + +} -start + +varnish v1 -arg "-p ban_any_variant=0" -vcl+backend { + sub vcl_backend_response { + set beresp.http.url = bereq.url; + } +} -start + + +client c1 { + txreq -url /foo -hdr "Foobar: 1" + rxresp + expect resp.body == "1111" +} -run + +client c1 { + txreq -url /foo -hdr "Foobar: 2" + rxresp + expect resp.body == "2222" +} -run + +client c1 { + txreq -url /foo -hdr "Foobar: 3" + rxresp + expect resp.body == "3333" + +} -run + +varnish v1 -expect n_object == 3 +varnish v1 -expect cache_hit == 0 +varnish v1 -expect cache_miss == 3 + +client c1 { + txreq -url /foo -hdr "Foobar: 1" + rxresp +} -run + +varnish v1 -expect cache_hit == 1 +varnish v1 -expect cache_miss == 3 + +varnish v1 -cliok "ban obj.http.url == /foo" +varnish v1 -cliok "ban obj.http.url == /bar" +varnish v1 -cliok "ban obj.http.url == /baz" + +client c1 { + txreq -url /foo -hdr "Foobar: 1" + rxresp +} -run + +varnish v1 -expect bans_tested == 1 +varnish v1 -expect bans_tests_tested == 3 +varnish v1 -expect bans_obj_killed == 1 +varnish v1 -expect n_object == 3 + + +varnish v1 -expect cache_hit == 1 +varnish v1 -expect cache_miss == 4 +varnish v1 -expect client_req == 5 From f7e0137725f9b2ede98429134e4c43bd71b79b9a Mon Sep 17 00:00:00 2001 From: Walid Boudebouda Date: Wed, 15 Jan 2025 17:04:43 +0100 Subject: [PATCH 4/4] Docs: Mention the new ban_any_variant param in purging.rst --- doc/sphinx/users-guide/purging.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/doc/sphinx/users-guide/purging.rst b/doc/sphinx/users-guide/purging.rst index 4ca4eae3022..6724c7635af 100644 --- a/doc/sphinx/users-guide/purging.rst +++ b/doc/sphinx/users-guide/purging.rst @@ -88,6 +88,23 @@ the regular expression is required by the Varnish cli interface. Bans are checked when we hit an object in the cache, but before we deliver it. *An object is only checked against newer bans*. +During lookup, object variants that may not satisfy the current request +are also tested against the ban list, which means that a ban may also +hit a non matching variant. + +However, the parameter `ban_any_variant` can be used to limit the number +of possibly non matching variants that are checked against the ban list during +lookup for a particular request. This means that at most `ban_any_variant` +variants will be evaluated, and possibly evicted, before looking for matching +variants. A value of 0 means that every request would only evaluate bans +against matching variants. In contrast, a value that is too high may cause a +request to evaluate all variants against all active bans, which can add +significant delays for configurations having a large number of variants +and/or bans. + +In the next major release of varnish (8.0), the default value of +`ban_any_variant` will be set to 0. + Bans that only match against `obj.*` are also processed by a background worker threads called the `ban lurker`. The `ban lurker` will walk the heap and try to match objects and will evict the matching objects. How