[master] 9f27212 Make hit-for-pass cause HSH_MISS instead of HSH_HIT+OC_F_PASS.

Poul-Henning Kamp phk at FreeBSD.org
Mon May 30 19:26:06 CEST 2016


commit 9f272127c6fba76e6758d7ab7ba6527d9aad98b0
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon May 30 17:21:33 2016 +0000

    Make hit-for-pass cause HSH_MISS instead of HSH_HIT+OC_F_PASS.
    
    it is more consistent to hit vcl_miss{} every time for these objects,
    as opposed to vcl_pass{} most of the time but occationally vcl_miss{}.
    
    This also almost entirely takes the TTL of HFP objects out of the
    picture, in the sense that any cacheable object fetched as a result
    of HFP will be put into the cache and be usable, rather than the
    HFP preventing cacheable objects for whatever TTL the HFP might
    have.
    
    See c00076 for an example of this.

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 7f8bebb..b808670 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -435,10 +435,18 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 			/* If still valid, use it */
 			assert(oh->refcnt > 1);
 			assert(oc->objhead == oh);
-			oc->refcnt++;
-			if (oc->hits < LONG_MAX)
-				oc->hits++;
+			if (oc->flags & OC_F_PASS) {
+				wrk->stats->cache_hitpass++;
+				oc = NULL;
+				*bocp = hsh_insert_busyobj(wrk, oh);
+			} else {
+				oc->refcnt++;
+				if (oc->hits < LONG_MAX)
+					oc->hits++;
+			}
 			Lck_Unlock(&oh->mtx);
+			if (oc == NULL) 
+				return (HSH_MISS);
 			assert(HSH_DerefObjHead(wrk, &oh));
 			*ocp = oc;
 			return (HSH_HIT);
@@ -450,6 +458,12 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 		}
 	}
 
+	if (exp_oc != NULL && exp_oc->flags & OC_F_PASS) {
+		wrk->stats->cache_hitpass++;
+		exp_oc = NULL;
+		busy_found = 0;
+	}
+
 	if (exp_oc != NULL) {
 		assert(oh->refcnt > 1);
 		assert(exp_oc->objhead == oh);
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 4db4fe7..a2d830c 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -401,24 +401,7 @@ cnt_lookup(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	AZ(oc->flags & OC_F_BUSY);
 	req->objcore = oc;
-
-	if ((oc->flags & OC_F_PASS) && busy != NULL) {
-		/* Treat a graced Hit-For-Pass as a miss */
-		(void)HSH_DerefObjCore(wrk, &req->objcore);
-		AZ(req->objcore);
-		req->objcore = busy;
-		req->req_step = R_STP_MISS;
-		return (REQ_FSM_MORE);
-	} else if (oc->flags & OC_F_PASS) {
-		/* Found a hit-for-pass */
-		VSLb(req->vsl, SLT_Debug, "XXXX HIT-FOR-PASS");
-		VSLb(req->vsl, SLT_HitPass, "%u",
-		    ObjGetXID(wrk, req->objcore));
-		(void)HSH_DerefObjCore(wrk, &req->objcore);
-		wrk->stats->cache_hitpass++;
-		req->req_step = R_STP_PASS;
-		return (REQ_FSM_MORE);
-	}
+	AZ(oc->flags & OC_F_PASS);
 
 	VSLb(req->vsl, SLT_Hit, "%u", ObjGetXID(wrk, req->objcore));
 
diff --git a/bin/varnishd/flint.lnt b/bin/varnishd/flint.lnt
index 89bfae9..190e4e1 100644
--- a/bin/varnishd/flint.lnt
+++ b/bin/varnishd/flint.lnt
@@ -157,3 +157,4 @@
 -e663	// Suspicious array to pointer conversion
 -e778   // Constant expression evaluates to 0 in operation '___'
 -e736	// Loss of precision (___) (___ bits to ___ bits)
+-e655	// bitwise compatible enums
diff --git a/bin/varnishtest/tests/c00076.vtc b/bin/varnishtest/tests/c00076.vtc
new file mode 100644
index 0000000..fe193be
--- /dev/null
+++ b/bin/varnishtest/tests/c00076.vtc
@@ -0,0 +1,113 @@
+varnishtest "Complex son-of-hit-for-pass test"
+
+server s1 {
+	rxreq
+	txresp -hdr "Connection: close" -hdr "Set-Cookie: c1" -bodylen 1
+} -start
+
+varnish v1 -vcl+backend {} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.bodylen == 1
+	delay .1
+} -run
+
+server s1 {
+	rxreq
+	expect req.http.foo == xyz
+	txresp -hdr "Set-Cookie: c2" -hdr "Vary: foo" -bodylen 2
+	rxreq
+	expect req.http.foo == abc
+	txresp -hdr "Connection: close" -hdr "Set-Cookie: c2" -hdr "Vary: foo" -bodylen 3
+} -start
+
+client c1 {
+	txreq -hdr "foo: xyz"
+	rxresp
+	expect resp.bodylen == 2
+	delay .1
+} -run
+
+client c1 {
+	txreq -hdr "foo: abc"
+	rxresp
+	expect resp.bodylen == 3
+	delay .1
+} -run
+
+server s1 {
+	rxreq
+	expect req.http.foo == 123
+	txresp -hdr "Connection: close" -hdr "Vary: foo" -bodylen 4
+} -start
+
+client c1 {
+	txreq -hdr "foo: 123"
+	rxresp
+	expect resp.bodylen == 4
+	delay .1
+} -run
+
+client c1 {
+	# Cache hit
+	txreq -hdr "foo: 123"
+	rxresp
+	expect resp.bodylen == 4
+	delay .1
+} -run
+
+server s1 {
+	rxreq
+	expect req.http.foo == 987
+	txresp -hdr "Connection: close" -hdr "Vary: foo" -bodylen 5
+} -start
+
+client c1 {
+	txreq -hdr "foo: 987"
+	rxresp
+	expect resp.bodylen == 5
+	delay .1
+} -run
+
+client c1 {
+	# Cache hit
+	txreq -hdr "foo: 123"
+	rxresp
+	expect resp.bodylen == 4
+	delay .1
+} -run
+
+server s1 {
+	rxreq
+	expect req.http.foo == 000
+	txresp -hdr "Connection: close" -bodylen 6
+} -start
+
+client c1 {
+	txreq -hdr "foo: 000"
+	rxresp
+	expect resp.bodylen == 6
+	delay .1
+
+	txreq -hdr "foo: 123"
+	rxresp
+	expect resp.bodylen == 6
+	delay .1
+
+	txreq -hdr "foo: abc"
+	rxresp
+	expect resp.bodylen == 6
+	delay .1
+
+	txreq -hdr "foo: 987"
+	rxresp
+	expect resp.bodylen == 6
+	delay .1
+
+	txreq -hdr "foo: xyz"
+	rxresp
+	expect resp.bodylen == 6
+	delay .1
+} -run
diff --git a/bin/varnishtest/tests/r01818.vtc b/bin/varnishtest/tests/r01818.vtc
index 2fe8ae3..c7dffcd 100644
--- a/bin/varnishtest/tests/r01818.vtc
+++ b/bin/varnishtest/tests/r01818.vtc
@@ -74,5 +74,5 @@ client c2 {
 	barrier b2 sync
 	txreq -hdr "c: 1"
 	rxresp
-	expect resp.http.pass == "1"
+	expect resp.http.miss == "1"
 } -run



More information about the varnish-commit mailing list