[master] ce9fbe44d lookup: Add a notice for high numbers of variants

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Feb 15 14:26:07 UTC 2021


commit ce9fbe44dae592c7ea38339f3653eca782c2ece6
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Feb 15 07:50:54 2021 +0100

    lookup: Add a notice for high numbers of variants
    
    As a recurring pitfall sometimes hard to identify, it deserves its own
    notice. The accounting of variants reuses the worker's strangelove field
    since it doesn't conflict where other parts of the code base use it.

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index a5dae200d..25beaf3d8 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -397,6 +397,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
 	busy_found = 0;
 	exp_oc = NULL;
 	exp_t_origin = 0.0;
+	wrk->strangelove = 0;
 	VTAILQ_FOREACH(oc, &oh->objcs, hsh_list) {
 		/* Must be at least our own ref + the objcore we examine */
 		assert(oh->refcnt > 1);
@@ -415,8 +416,10 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
 				continue;
 
 			if (oc->boc && oc->boc->vary != NULL &&
-			    !VRY_Match(req, oc->boc->vary))
+			    !VRY_Match(req, oc->boc->vary)) {
+				wrk->strangelove++;
 				continue;
+			}
 
 			busy_found = 1;
 			continue;
@@ -434,8 +437,10 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
 		if (ObjHasAttr(wrk, oc, OA_VARY)) {
 			vary = ObjGetAttr(wrk, oc, OA_VARY, NULL);
 			AN(vary);
-			if (!VRY_Match(req, vary))
+			if (!VRY_Match(req, vary)) {
+				wrk->strangelove++;
 				continue;
+			}
 		}
 
 		if (req->vcf != NULL) {
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index df1a674cf..b8bb8b97b 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -543,6 +543,9 @@ cnt_lookup(struct worker *wrk, struct req *req)
 	if (req->hash_objhead)
 		had_objhead = 1;
 	lr = HSH_Lookup(req, &oc, &busy);
+	if (wrk->strangelove >= 10)
+		VSLb(req->vsl, SLT_Notice, "vsl: High number of variants (%d)",
+		    wrk->strangelove);
 	if (lr == HSH_BUSY) {
 		/*
 		 * We lost the session to a busy object, disembark the
diff --git a/bin/varnishtest/tests/r02372.vtc b/bin/varnishtest/tests/r02372.vtc
index 0df38cb79..d624d1141 100644
--- a/bin/varnishtest/tests/r02372.vtc
+++ b/bin/varnishtest/tests/r02372.vtc
@@ -13,11 +13,17 @@ varnish v1 -arg "-p workspace_thread=512" -vcl+backend {
 	}
 } -start
 
+logexpect l1 -v v1 {
+	expect * * Notice "High number of variants .71."
+} -start
+
 client c1 -repeat 72 -keepalive {
 	txreq
 	rxresp
 } -run
 
+logexpect l1 -wait
+
 client c2 {
 	txreq -req "PURGE"
 	rxresp
diff --git a/doc/sphinx/reference/vsl.rst b/doc/sphinx/reference/vsl.rst
index e1d608edb..5835f5a0d 100644
--- a/doc/sphinx/reference/vsl.rst
+++ b/doc/sphinx/reference/vsl.rst
@@ -134,6 +134,19 @@ it. The fetch is halted until the stale object is fully fetched, upon which
 the new object is created as normal. While waiting, any grace time on the
 stale object will be in effect.
 
+High number of variants
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Objects are primarily looked up from an index using the hash key computed from
+the ``hash_data()`` VCL function. When variants are involved, that is to say
+when a backend response was stored with a ``Vary`` header, a secondary lookup
+is performed but it is not indexed. As the number of variants for a given key
+increases, this can slow cache lookups down, and since this happens under a
+lock, this can greatly increase lock contention, even more so for frequently
+requested objects. Variants should therefore be used sparingly on cacheable
+responses, but since they can originate from either VCL or origin servers,
+this notice should help identify problematic resources.
+
 
 HISTORY
 =======


More information about the varnish-commit mailing list