HSH: Limit the number of non matching variants evaluated against the ban list during lookup

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)

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

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

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

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,