[PATCH] Drop old grace objects when we have a new object that matches the Vary
Rogier 'DocWilco' Mulhuijzen
github at bsdchicks.com
Mon Jan 16 00:08:24 CET 2012
(Now with yummy VRY_Compare!)
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.
VRY_Compare compares 2 vary strings for equality (taking gzip into
account) and was added so that the grace object is only dropped if its
vary matches to the vary on the new object.
---
bin/varnishd/cache/cache.h | 3 +
bin/varnishd/cache/cache_center.c | 4 +-
bin/varnishd/cache/cache_expire.c | 43 ++++++++++++
bin/varnishd/cache/cache_hash.c | 52 +++++++++++----
bin/varnishd/cache/cache_vary.c | 23 +++++++
bin/varnishd/hash/hash_slinger.h | 2 +-
bin/varnishtest/tests/grace-drop.vtc | 122 ++++++++++++++++++++++++++++++++++
include/tbl/vsc_f_main.h | 3 +
8 files changed, 235 insertions(+), 17 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 356d138..d60ad8b 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; /* == stale_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);
@@ -969,6 +971,7 @@ void RES_StreamPoll(struct worker *);
/* cache_vary.c */
struct vsb *VRY_Create(const struct sess *sp, const struct http *hp);
+int VRY_Compare(const uint8_t *vary1, const uint8_t *vary2);
int VRY_Match(const struct sess *sp, const uint8_t *vary);
void VRY_Validate(const uint8_t *vary);
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index d80842a..a4aef8b 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..22ad508 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 */
- if (oc == NULL /* We found no live object */
- && grace_oc != NULL /* There is a grace candidate */
- && (busy_oc != NULL /* Somebody else is already busy */
+ sp->req->objcore = sp->req->grace_oc; /* XXX: Hack-ish */
+ if (oc == NULL /* We found no live object */
+ && 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 && VRY_Compare(o->vary, go->vary)) {
+ 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/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index 9ad41d0..dea9d95 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -174,6 +174,29 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2)
}
int
+VRY_Compare(const uint8_t *vary1, const uint8_t *vary2)
+{
+
+ if (vary1 == NULL && vary2 == NULL)
+ /* both are NULL */
+ return 1;
+ if (vary1 == NULL || vary2 == NULL)
+ /* only one of them is NULL */
+ return 0;
+ while (vary1[2] && vary2[2]) {
+ if (vry_cmp(vary1, vary2))
+ /* header doesn't match */
+ return 0;
+ vary1 += vry_len(vary1);
+ vary2 += vry_len(vary2);
+ }
+ if (vary1[2] || vary2[2])
+ /* number of headers doesn't match */
+ return 0;
+ return 1;
+}
+
+int
VRY_Match(const struct sess *sp, const uint8_t *vary)
{
uint8_t *vsp = sp->req->vary_b;
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