Hi

At the Varnish Summit in Paris last week, we got some feedback from a
few early adopters of Varnish 4.0, who were very surprised with the
new behavior of obj.hits.

To understand obj.hits in its current form, we need to understand the
concept of an objhead, which is fairly esoteric knowledge, and
something most users aren't familiar with. Also, I find that obj.hits
currently simply is not very useful.

I have attached a patch that moves the obj.hits counter to struct
objcore. This should give the old 3.0 functionality, where obj.hits is
separate for each object. The cost of this turns out to be (on my
computer) and extra 8 bytes per OC (and 8 less per OH), which IMO is
manageable.

Regards,
Dag

-- 
Dag Haavi Finstad
Software Developer | Varnish Software
Mobile: +47 476 64 134
We Make Websites Fly!
From e8290df88bd4040a233a3645b436e0c30322c67c Mon Sep 17 00:00:00 2001
From: Dag Haavi Finstad <[email protected]>
Date: Wed, 22 Oct 2014 18:10:57 +0200
Subject: [PATCH] Move obj.hits to the objcore

---
 bin/varnishd/cache/cache.h         |  1 +
 bin/varnishd/cache/cache_hash.c    |  8 ++++----
 bin/varnishd/cache/cache_vrt_var.c |  3 +--
 bin/varnishd/hash/hash_slinger.h   |  2 --
 bin/varnishtest/tests/v00039.vtc   |  8 ++++----
 lib/libvcc/generate.py             | 10 +++-------
 6 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 3ba86b7..c1a4576 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -403,6 +403,7 @@ struct objcore {
 	struct objhead		*objhead;
 	struct busyobj		*busyobj;
 	double			timer_when;
+	long			hits;
 
 	struct exp		exp;
 
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index dc18cfe..de35e8a 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -434,8 +434,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 			assert(oh->refcnt > 1);
 			assert(oc->objhead == oh);
 			oc->refcnt++;
-			if (oh->hits < LONG_MAX)
-				oh->hits++;
+			if (oc->hits < LONG_MAX)
+				oc->hits++;
 			Lck_Unlock(&oh->mtx);
 			assert(HSH_DerefObjHead(wrk, &oh));
 			*ocp = oc;
@@ -461,8 +461,8 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 			AZ(req->hash_ignore_busy);
 			retval = HSH_EXP;
 		}
-		if (oh->hits < LONG_MAX)
-			oh->hits++;
+		if (exp_oc->hits < LONG_MAX)
+			exp_oc->hits++;
 		Lck_Unlock(&oh->mtx);
 		if (retval == HSH_EXP)
 			assert(HSH_DerefObjHead(wrk, &oh));
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index 8448317..07f386b 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -651,8 +651,7 @@ VRT_r_obj_hits(VRT_CTX)
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
 	CHECK_OBJ_NOTNULL(ctx->req->objcore, OBJCORE_MAGIC);
-	CHECK_OBJ_NOTNULL(ctx->req->objcore->objhead, OBJHEAD_MAGIC);
-	return (ctx->req->objcore->objhead->hits);
+	return (ctx->req->objcore->hits);
 }
 
 unsigned
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index db722da..189f938 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -94,8 +94,6 @@ struct objhead {
 	uint8_t			digest[DIGEST_LEN];
 	struct waitinglist	*waitinglist;
 
-	long			hits;
-
 	/*----------------------------------------------------
 	 * The fields below are for the sole private use of
 	 * the hash implementation(s).
diff --git a/bin/varnishtest/tests/v00039.vtc b/bin/varnishtest/tests/v00039.vtc
index 90bc865..029e114 100644
--- a/bin/varnishtest/tests/v00039.vtc
+++ b/bin/varnishtest/tests/v00039.vtc
@@ -31,19 +31,19 @@ client c1 {
 	expect resp.bodylen == 6
 	expect resp.http.hits == 1
 
-	# This is a miss on different vary -> hits still == 1
+	# This is a miss on different vary -> hits == 0
 	txreq -url "/" -hdr "Bar: 2"
 	rxresp
 	expect resp.status == 200
 	expect resp.bodylen == 4
-	expect resp.http.hits == 1
+	expect resp.http.hits == 0
 
-	# This is a hit -> hits == 2
+	# This is a hit -> hits == 1
 	txreq -url "/" -hdr "Bar: 2"
 	rxresp
 	expect resp.status == 200
 	expect resp.bodylen == 4
-	expect resp.http.hits == 2
+	expect resp.http.hits == 1
 
 } -run
 
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index ea07a32..4dd6288 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -571,13 +571,9 @@ sp_variables = [
 		'INT',
 		( 'hit', 'deliver',),
 		( ), """
-		The count of cache-hits on this hash-key since it was
-		last instantiated.  This counts cache-hits across all
-		Vary:-ants on this hash-key.
-		The counter will only be reset to zero if/when all objects
-		with this hash-key have disappeared from cache.
-		NB: obj.hits == 0 does *not* indicate a cache miss.
-		"""
+                The count of cache-hits on this object. A value of 0 indicates a
+		cache miss.
+                """
 	),
 	('obj.http.',
 		'HEADER',
-- 
2.1.1

_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to