[PATCH] Drop old grace objects when we have a new object that matches the Vary

Rogier 'DocWilco' Mulhuijzen github at bsdchicks.com
Fri Jan 6 16:54:46 CET 2012


(Lessen learned: do not email patches at 6am. I've done a few small
fixups, so here it is again.)

Also add EXP_Remove, to allow removing objects with touching them.

This patch makes it possible to have a very short TTL, but a long grace.
Normally, if you were to set a 1s TTL and a 86400s grace, you would end
up with 86400 objects on a single objecthead, not even taking into
account Vary. With this patch, any object found in grace is kept with
the request, and upon succesful retrieval of a new object from the
backend, the grace object is removed.

EXP_Remove was added so that the TTL and grace on the object aren't
touched. This, in combination with some other work we've done, prevents
pages from getting dirtied and thus having to be written out to disk
in a environment where the cache cannot be purely kept in memory.
---
 bin/varnishd/cache/cache.h           |    2 +
 bin/varnishd/cache/cache_center.c    |    4 +-
 bin/varnishd/cache/cache_expire.c    |   43 ++++++++++++
 bin/varnishd/cache/cache_hash.c      |   48 ++++++++++----
 bin/varnishd/hash/hash_slinger.h     |    2 +-
 bin/varnishtest/tests/grace-drop.vtc |  122 ++++++++++++++++++++++++++++++++++
 include/tbl/vsc_f_main.h             |    3 +
 7 files changed, 209 insertions(+), 15 deletions(-)
 create mode 100644 bin/varnishtest/tests/grace-drop.vtc

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 6b0d0a9..cb8fe4e 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -584,6 +584,7 @@ struct req {
 
 	const char		*doclose;
 	struct exp		exp;
+	struct objcore		*grace_oc;
 	unsigned		cur_method;
 	unsigned		handling;
 	unsigned char		sendbody;
@@ -753,6 +754,7 @@ void EXP_Init(void);
 void EXP_Rearm(const struct object *o);
 int EXP_Touch(struct objcore *oc);
 int EXP_NukeOne(struct worker *w, struct lru *lru);
+void EXP_Remove(struct worker *w, struct objcore *oc);
 
 /* cache_fetch.c */
 struct storage *FetchStorage(struct worker *w, ssize_t sz);
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index 4967c82..df9b9cd 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -928,7 +928,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 		EXP_Insert(req->obj);
 		AN(req->obj->objcore);
 		AN(req->obj->objcore->ban);
-		HSH_Unbusy(wrk);
+		HSH_Unbusy(wrk, 1);
 	}
 	VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	wrk->acct_tmp.fetch++;
@@ -987,7 +987,7 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
 		EXP_Insert(req->obj);
 		AN(req->obj->objcore);
 		AN(req->obj->objcore->ban);
-		HSH_Unbusy(wrk);
+		HSH_Unbusy(wrk, 1);
 	} else {
 		req->doclose = "Stream error";
 	}
diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c
index e93a1aa..a8c4254 100644
--- a/bin/varnishd/cache/cache_expire.c
+++ b/bin/varnishd/cache/cache_expire.c
@@ -451,6 +451,49 @@ EXP_NukeOne(struct worker *wrk, struct lru *lru)
 }
 
 /*--------------------------------------------------------------------
+ * Remove from expiry binheap. Caller must have a ref on the oc.
+ */
+
+void
+EXP_Remove(struct worker *w, struct objcore *oc)
+{
+	struct object *o;
+	struct lru *l;
+
+	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
+	o = oc_getobj(w, oc);
+	CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
+	l = oc_getlru(oc);
+	CHECK_OBJ_NOTNULL(l, LRU_MAGIC);
+
+	Lck_Lock(&l->mtx);
+	Lck_Lock(&exp_mtx);
+
+	if (oc->timer_idx == BINHEAP_NOIDX) { /* exp_timer has it */
+		Lck_Unlock(&exp_mtx);
+		Lck_Unlock(&l->mtx);
+		return;
+	}
+
+	/* remove from binheap */
+	binheap_delete(exp_heap, oc->timer_idx);
+	assert(oc->timer_idx == BINHEAP_NOIDX);
+
+	/* And from LRU */
+	CHECK_OBJ_NOTNULL(l, LRU_MAGIC);
+	VTAILQ_REMOVE(&l->lru_head, oc, lru_list);
+
+	Lck_Unlock(&exp_mtx);
+	Lck_Unlock(&l->mtx);
+
+	w->stats.c_removed++;
+
+	WSL(w, SLT_ExpKill, 0, "%u Removed", o->xid);
+	assert(oc->refcnt >= 2); /* exp ref and caller ref */
+	HSH_Deref(w, NULL, &o);
+}
+
+/*--------------------------------------------------------------------
  * BinHeap helper functions for objcore.
  */
 
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index d967291..a3eb77c 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -296,7 +296,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 	struct worker *wrk;
 	struct objhead *oh;
 	struct objcore *oc;
-	struct objcore *busy_oc, *grace_oc;
+	struct objcore *busy_oc;
 	struct object *o;
 	double grace_ttl;
 
@@ -332,7 +332,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 	Lck_Lock(&oh->mtx);
 	assert(oh->refcnt > 0);
 	busy_oc = NULL;
-	grace_oc = NULL;
+	sp->req->grace_oc = NULL;
 	grace_ttl = NAN;
 	VTAILQ_FOREACH(oc, &oh->objcs, list) {
 		/* Must be at least our own ref + the objcore we examine */
@@ -372,9 +372,9 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 		 * and if there are several, use the least expired one.
 		 */
 		if (EXP_Grace(sp, o) >= sp->t_req) {
-			if (grace_oc == NULL ||
+			if (sp->req->grace_oc == NULL ||
 			    grace_ttl < o->exp.entered + o->exp.ttl) {
-				grace_oc = oc;
+				sp->req->grace_oc = oc;
 				grace_ttl = o->exp.entered + o->exp.ttl;
 			}
 		}
@@ -391,15 +391,16 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 	 */
 
 	AZ(sp->req->objcore);
-	sp->req->objcore = grace_oc;		/* XXX: Hack-ish */
+	sp->req->objcore = sp->req->grace_oc;		/* XXX: Hack-ish */
 	if (oc == NULL			/* We found no live object */
-	    && grace_oc != NULL		/* There is a grace candidate */
+	    && sp->req->grace_oc != NULL	/* There is a grace candidate */
 	    && (busy_oc != NULL		/* Somebody else is already busy */
 	    || !VDI_Healthy(sp->req->director, sp))) {
 					/* Or it is impossible to fetch */
-		o = oc_getobj(sp->wrk, grace_oc);
+		o = oc_getobj(sp->wrk, sp->req->grace_oc);
 		CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
-		oc = grace_oc;
+		oc = sp->req->grace_oc;
+		sp->req->grace_oc = NULL;
 	}
 	sp->req->objcore = NULL;
 
@@ -415,6 +416,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 		assert(oh->refcnt > 1);
 		Lck_Unlock(&oh->mtx);
 		assert(hash->deref(oh));
+		sp->req->grace_oc = NULL;
 		*poh = oh;
 		return (oc);
 	}
@@ -441,6 +443,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 		 */
 		sp->req->hash_objhead = oh;
 		sp->wrk = NULL;
+		sp->req->grace_oc = NULL;
 		Lck_Unlock(&oh->mtx);
 		return (NULL);
 	}
@@ -450,6 +453,15 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 	wrk->nobjcore = NULL;
 	AN(oc->flags & OC_F_BUSY);
 	oc->refcnt = 1;
+	/* 
+	 * make sure that we don't expire the graced object while we're fetching
+	 * so that we don't panic/segfault after fetch
+	 */
+	if (sp->req->grace_oc) {
+		CHECK_OBJ_NOTNULL(sp->req->grace_oc, OBJCORE_MAGIC);
+		sp->req->grace_oc->refcnt++; 
+		assert(sp->req->grace_oc->refcnt > 0);
+	}
 
 	AZ(wrk->busyobj);
 	wrk->busyobj = VBO_GetBusyObj(wrk);
@@ -586,16 +598,16 @@ HSH_Drop(struct worker *wrk)
 	AssertObjCorePassOrBusy(o->objcore);
 	o->exp.ttl = -1.;
 	if (o->objcore != NULL)		/* Pass has no objcore */
-		HSH_Unbusy(wrk);
+		HSH_Unbusy(wrk, 0);
 	(void)HSH_Deref(wrk, NULL, &wrk->sp->req->obj);
 }
 
 void
-HSH_Unbusy(struct worker *wrk)
+HSH_Unbusy(struct worker *wrk, int dropgrace)
 {
-	struct object *o;
+	struct object *o, *go;
 	struct objhead *oh;
-	struct objcore *oc;
+	struct objcore *oc, *grace_oc;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	o = wrk->sp->req->obj;
@@ -627,6 +639,18 @@ HSH_Unbusy(struct worker *wrk)
 		hsh_rush(oh);
 	AN(oc->ban);
 	Lck_Unlock(&oh->mtx);
+	if (wrk->sp->req->grace_oc) {
+		grace_oc = wrk->sp->req->grace_oc;
+		CHECK_OBJ_NOTNULL(grace_oc, OBJCORE_MAGIC);
+		go = oc_getobj(wrk, grace_oc);
+		CHECK_OBJ_NOTNULL(go, OBJECT_MAGIC);
+		if (dropgrace) {
+			wrk->stats.c_grace_obj_dropped++;
+			EXP_Remove(wrk, grace_oc);
+		}
+		assert(grace_oc->refcnt > 0);
+		HSH_Deref(wrk, NULL, &go);
+	}
 	assert(oc_getobj(wrk, oc) == o);
 }
 
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index b45e604..c0cd54c 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -54,7 +54,7 @@ struct hash_slinger {
 void HSH_Prealloc(const struct sess *sp);
 void HSH_Cleanup(struct worker *w);
 struct objcore *HSH_Lookup(struct sess *sp, struct objhead **poh);
-void HSH_Unbusy(struct worker *wrk);
+void HSH_Unbusy(struct worker *wrk, int dropgrace);
 void HSH_Ref(struct objcore *o);
 void HSH_Drop(struct worker *wrk);
 void HSH_Init(const struct hash_slinger *slinger);
diff --git a/bin/varnishtest/tests/grace-drop.vtc b/bin/varnishtest/tests/grace-drop.vtc
new file mode 100644
index 0000000..05cdbac
--- /dev/null
+++ b/bin/varnishtest/tests/grace-drop.vtc
@@ -0,0 +1,122 @@
+varnishtest "Drop graced object when new object arrives"
+
+server s1 {
+	rxreq 
+	txresp -hdr "Vary: X-Foo" -hdr "Cache-Control: max-age=1" -body "1"
+	rxreq 
+	txresp -hdr "Vary: X-Foo" -hdr "Cache-Control: max-age=1" -body "12"
+	rxreq 
+	txresp -hdr "Vary: X-Foo" -hdr "Cache-Control: max-age=1" -body "123"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		set req.grace = 0s;
+	}
+	sub vcl_fetch {
+		set beresp.grace = 3600s;
+	}
+} -start
+
+varnish v1 -expect n_object == 0
+varnish v1 -expect sess_conn == 0
+varnish v1 -expect client_req == 0
+varnish v1 -expect cache_miss == 0
+varnish v1 -expect cache_hit == 0
+varnish v1 -expect s_sess == 0
+varnish v1 -expect s_req == 0
+
+client c1 {
+	txreq -hdr "X-Foo: bar"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 1
+	txreq -hdr "X-Foo: blargh"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 2
+} -run
+
+delay 1.1
+
+varnish v1 -expect n_object == 2
+varnish v1 -expect sess_conn == 1
+varnish v1 -expect client_req == 2
+varnish v1 -expect cache_miss == 2
+varnish v1 -expect cache_hit == 0
+varnish v1 -expect s_sess == 1
+varnish v1 -expect s_req == 2
+
+client c1 {
+	txreq -hdr "X-Foo: bar"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 3
+} -run
+
+varnish v1 -expect n_object == 2
+varnish v1 -expect sess_conn == 2
+varnish v1 -expect client_req == 3
+varnish v1 -expect cache_miss == 3
+varnish v1 -expect cache_hit == 0
+varnish v1 -expect s_sess == 2
+varnish v1 -expect s_req == 3
+
+server s1 -wait
+
+delay 1.1
+
+client c1 {
+	txreq -hdr "X-Foo: bar"
+	rxresp
+	expect resp.status == 503
+} -run
+
+varnish v1 -expect n_object == 2
+varnish v1 -expect sess_conn == 3
+varnish v1 -expect client_req == 4
+varnish v1 -expect cache_miss == 4
+varnish v1 -expect cache_hit == 0
+varnish v1 -expect s_sess == 3
+varnish v1 -expect s_req == 4
+
+varnish v1 -vcl {
+	backend default {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+		.max_connections = 1;
+		.probe = {
+			.url = "/";
+			.timeout = 1s;
+			.interval = 1000s;
+			.window = 1;
+			.threshold = 1;
+			.initial = 0;
+		}
+	}
+
+	sub vcl_recv {
+		set req.grace = 3600s;
+	}
+}
+
+delay 0.1
+
+client c1 {
+	txreq -hdr "X-Foo: bar"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 3
+	txreq -hdr "X-Foo: blargh"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 2
+} -run
+
+varnish v1 -expect n_object == 2
+varnish v1 -expect sess_conn == 4
+varnish v1 -expect client_req == 6
+varnish v1 -expect cache_miss == 4
+varnish v1 -expect cache_hit == 2
+varnish v1 -expect s_sess == 4
+varnish v1 -expect s_req == 6
diff --git a/include/tbl/vsc_f_main.h b/include/tbl/vsc_f_main.h
index df7984a..60401a6 100644
--- a/include/tbl/vsc_f_main.h
+++ b/include/tbl/vsc_f_main.h
@@ -232,7 +232,10 @@ VSC_F(n_backend,		uint64_t, 0, 'i', "N backends", "")
 
 VSC_F(n_expired,		uint64_t, 0, 'i', "N expired objects", "")
 VSC_F(n_lru_nuked,		uint64_t, 0, 'i', "N LRU nuked objects", "")
+VSC_F(c_removed,		uint64_t, 1, 'a', "N removed objects", "")
 VSC_F(n_lru_moved,		uint64_t, 0, 'i', "N LRU moved objects", "")
+VSC_F(c_grace_obj_dropped,	uint64_t, 1, 'i',
+		"Objects in grace dropped due to new object from backend", "")
 
 VSC_F(losthdr,		uint64_t, 0, 'a', "HTTP header overflows", "")
 
-- 
1.7.5.4




More information about the varnish-dev mailing list