[4.1] d2d09318c Reintroduce the req.grace variable, change keep behavior
PÃ¥l Hermunn Johansen
hermunn at varnish-software.com
Thu Sep 13 13:03:11 UTC 2018
commit d2d09318cfacbc61c7bc07cf502fddbae6ec3fbf
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date: Tue Jun 12 16:33:07 2018 +0200
Reintroduce the req.grace variable, change keep behavior
This is a back port of ff38535acd6d9c3b87d7fbbcc9c6917f7106d216
The req.grace variable can be set in vcl_recv to cap the grace
of objects in the cache, in the same way as in 3.0.x
The "keep" behavior changes with this patch. We now always go
to vcl_miss when the expired object is out of grace, or we go
to the waiting list. The result is that it is no longer
possible to deliver a "keep" object in vcl_hit.
Note that when we get to vcl_miss, we will still have the 304
candidate, but without the detour by vcl_hit.
This commit changes VCL, but only slightly, so we aim to back
port this to earlier versions of Varnish Cache.
Refs: #1799 and #2519
Conflicts:
bin/varnishd/cache/cache_hash.c
bin/varnishd/cache/cache_req_fsm.c
bin/varnishd/cache/cache_varnishd.h
bin/varnishd/cache/cache_vrt_var.c
doc/sphinx/reference/vcl_var.rst
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 1089f0a41..f27b2f6c9 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -564,6 +564,7 @@ struct req {
uint8_t digest[DIGEST_LEN];
double d_ttl;
+ double d_grace;
ssize_t req_bodybytes; /* Parsed req bodybytes */
@@ -697,6 +698,7 @@ void EXP_Clr(struct exp *e);
double EXP_Ttl(const struct req *, const struct exp*);
double EXP_When(const struct exp *exp);
+double EXP_Ttl_grace(const struct req *, const struct exp*);
void EXP_Insert(struct worker *wrk, struct objcore *oc);
void EXP_Inject(struct worker *wrk, struct objcore *oc, struct lru *lru);
void EXP_Rearm(struct objcore *, double now, double ttl, double grace,
diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c
index 28c79ab8a..4b0b95913 100644
--- a/bin/varnishd/cache/cache_expire.c
+++ b/bin/varnishd/cache/cache_expire.c
@@ -132,6 +132,24 @@ EXP_When(const struct exp *e)
return (when);
}
+/*--------------------------------------------------------------------
+ * Calculate an object's effective ttl+grace time, taking req.grace into
+ * account if it is available.
+ */
+
+double
+EXP_Ttl_grace(const struct req *req, const struct exp *e)
+{
+ double g;
+
+ AN(e);
+
+ g = e->grace;
+ if (req != NULL && req->d_grace >= 0. && req->d_grace < g)
+ g = req->d_grace;
+ return (EXP_Ttl(req, e) + g);
+}
+
/*--------------------------------------------------------------------
* Post an objcore to the exp_thread's inbox.
*/
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 30e2a06f7..d1f5f1c56 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -455,36 +455,50 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
}
}
+ if (exp_oc != NULL && EXP_Ttl_grace(req, &exp_oc->exp) < req->t_req) {
+ /* the newest object is out of grace */
+ if (!busy_found) {
+ /*
+ * here we get to insert the busy object. This
+ * translates to a MISS with a 304 candidate.
+ */
+ assert(oh->refcnt > 1);
+ assert(exp_oc->objhead == oh);
+ exp_oc->refcnt++;
+ /* Insert objcore in objecthead and release mutex */
+ *bocp = hsh_insert_busyobj(wrk, oh);
+ /* NB: no deref of objhead, new object inherits reference */
+ Lck_Unlock(&oh->mtx);
+ *ocp = exp_oc;
+ return (HSH_MISS);
+ } else {
+ /* we have no use for this very expired object */
+ exp_oc = NULL;
+ }
+ }
if (exp_oc != NULL) {
+ /*
+ * here the object is within grace, so we expect it to
+ * be delivered
+ */
assert(oh->refcnt > 1);
assert(exp_oc->objhead == oh);
+ exp_oc->refcnt++;
if (!busy_found) {
*bocp = hsh_insert_busyobj(wrk, oh);
retval = HSH_EXPBUSY;
} else {
AZ(req->hash_ignore_busy);
- /*
- * here we have a busy object, but if the stale object
- * is not under grace we go to the waiting list
- * instead of returning the stale object
- */
- if (EXP_Ttl(req, &exp_oc->exp) + exp_oc->exp.grace < req->t_req)
- retval = HSH_BUSY;
- else
- retval = HSH_EXP;
- }
- if (retval != HSH_BUSY) {
- exp_oc->refcnt++;
-
- if (exp_oc->hits < LONG_MAX)
- exp_oc->hits++;
- Lck_Unlock(&oh->mtx);
- if (retval == HSH_EXP)
- assert(HSH_DerefObjHead(wrk, &oh));
- *ocp = exp_oc;
- return (retval);
+ retval = HSH_EXP;
}
+ if (exp_oc->hits < LONG_MAX)
+ exp_oc->hits++;
+ Lck_Unlock(&oh->mtx);
+ if (retval == HSH_EXP)
+ assert(HSH_DerefObjHead(wrk, &oh));
+ *ocp = exp_oc;
+ return (retval);
}
if (!busy_found) {
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 25c548fb3..a8dcfe0ad 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -400,11 +400,11 @@ cnt_lookup(struct worker *wrk, struct req *req)
AZ(req->objcore);
if (lr == HSH_MISS) {
- /* Found nothing */
- AZ(oc);
+ /* Found nothing or an object that was out of grace */
AN(boc);
AN(boc->flags & OC_F_BUSY);
req->objcore = boc;
+ req->stale_oc = oc;
req->req_step = R_STP_MISS;
return (REQ_FSM_MORE);
}
@@ -702,6 +702,7 @@ cnt_recv(struct worker *wrk, struct req *req)
req->vdp_retval = 0;
req->d_ttl = -1;
+ req->d_grace = -1;
req->disable_esi = 0;
req->hash_always_miss = 0;
req->hash_ignore_busy = 0;
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index bd67551ef..a294ab831 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -361,7 +361,7 @@ VRT_l_beresp_storage_hint(VRT_CTX, const char *str, ...)
/*--------------------------------------------------------------------*/
-#define REQ_VAR_L(nm, elem, type,extra) \
+#define REQ_VAR_L(nm, elem, type, extra) \
\
void \
VRT_l_req_##nm(VRT_CTX, type arg) \
@@ -386,6 +386,8 @@ REQ_VAR_L(backend_hint, director_hint, const struct director *,)
REQ_VAR_R(backend_hint, director_hint, const struct director *)
REQ_VAR_L(ttl, d_ttl, double, if (!(arg>0.0)) arg = 0;)
REQ_VAR_R(ttl, d_ttl, double)
+REQ_VAR_L(grace, d_grace, double, if (!(arg>0.0)) arg = 0;)
+REQ_VAR_R(grace, d_grace, double)
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishtest/tests/b00098.vtc b/bin/varnishtest/tests/b00062.vtc
similarity index 100%
rename from bin/varnishtest/tests/b00098.vtc
rename to bin/varnishtest/tests/b00062.vtc
diff --git a/bin/varnishtest/tests/b00064.vtc b/bin/varnishtest/tests/b00064.vtc
new file mode 100644
index 000000000..8fee088d2
--- /dev/null
+++ b/bin/varnishtest/tests/b00064.vtc
@@ -0,0 +1,126 @@
+varnishtest "Test that req.grace will hold a client when a miss is anticipated"
+
+barrier b1 cond 2
+
+server s1 {
+ rxreq
+ expect req.url == "/"
+ txresp -body "0"
+
+ rxreq
+ expect req.url == "/"
+ txresp -body "1"
+
+ # second time we get a request, we use some time to serve it
+ rxreq
+ expect req.url == "/"
+ barrier b1 sync
+ delay .1
+ txresp -body "2"
+
+ # Last request, to a different URL to catch it if varnish asks for "/" too many times
+ rxreq
+ expect req.url == "/2"
+ txresp -body "x"
+} -start
+
+varnish v1 -vcl+backend {
+ import ${vmod_std};
+
+ sub vcl_recv {
+ # When we know in vcl_recv that we will have no grace, it is now
+ # possible to signal this to the lookup function:
+ if (req.http.X-no-grace) {
+ set req.grace = 0s;
+ }
+ }
+ sub vcl_hit {
+ set req.http.X-grace = obj.grace;
+ }
+ sub vcl_backend_response {
+ set beresp.ttl = 0.1s;
+ set beresp.grace = 1m;
+ }
+ sub vcl_deliver {
+ if (req.http.X-grace) {
+ set resp.http.X-grace = req.http.X-grace;
+ set resp.http.X-req-grace = req.grace;
+ }
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "0"
+ expect resp.http.X-grace == <undef>
+ # let the object's ttl expire
+ delay .2
+ # get a genuinely fresh object by disabling grace
+ # we will not get to vcl_hit to see the grace
+ txreq -hdr "X-no-grace: true"
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "1"
+ expect resp.http.X-grace == <undef>
+ expect resp.http.X-req-grace == <undef>
+} -run
+
+# let the latest object's ttl expire.
+delay .2
+
+varnish v1 -expect n_object == 1
+
+# c2 asks for the object under grace
+client c2 {
+ txreq
+ rxresp
+ # we did not disable grace in the request, so we should get the graced object here
+ expect resp.status == 200
+ expect resp.body == "1"
+ expect resp.http.X-grace == "60.000"
+ expect resp.http.X-req-grace < 0.
+} -start
+
+delay .1
+
+# c3 asks for graced object, but now we disable grace.
+client c3 {
+ txreq -hdr "X-no-grace: true"
+ rxresp
+ expect resp.status == 200
+ # Here we have disable grace and should get the object from the background fetch,
+ # which will take us into vcl_hit
+ expect resp.body == "2"
+ expect resp.http.X-grace == "60.000"
+ expect resp.http.X-req-grace == "0.000"
+} -start
+
+delay .1
+
+# c4 does not disable grace, and should get the grace object even
+# though c3 is waiting on the background thread to deliver a new
+# version.
+
+client c4 {
+ txreq
+ rxresp
+ barrier b1 sync
+ expect resp.status == 200
+ # We should get what c1 got in the very beginning
+ expect resp.body == "1"
+ expect resp.http.X-grace == "60.000"
+ expect resp.http.X-req-grace < 0.
+} -start
+
+client c2 -wait
+client c3 -wait
+client c4 -wait
+
+client c5 {
+ txreq -url "/2"
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "x"
+} -run
diff --git a/bin/varnishtest/tests/r02705.vtc b/bin/varnishtest/tests/r02705.vtc
new file mode 100644
index 000000000..54bec95c2
--- /dev/null
+++ b/bin/varnishtest/tests/r02705.vtc
@@ -0,0 +1,51 @@
+varnishtest "No vcl_hit when grace has run out, with working IMS"
+
+server s1 {
+ rxreq
+ txresp -hdr {Etag: "Foo"} -body "1"
+
+ rxreq
+ txresp -status 304
+
+ rxreq
+ expect req.url == "/2"
+ txresp -body "3"
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_backend_response {
+ set beresp.ttl = 1ms;
+ set beresp.grace = 0s;
+ set beresp.keep = 1m;
+ if (bereq.http.Hit) {
+ set beresp.http.Hit = bereq.http.Hit;
+ }
+ }
+ sub vcl_hit {
+ set req.http.Hit = "HIT";
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.http.Hit == <undef>
+ expect resp.http.Etag == {"Foo"}
+ expect resp.status == 200
+ expect resp.body == "1"
+ delay .2
+ txreq
+ rxresp
+ expect resp.http.Hit == <undef>
+ expect resp.http.Etag == {"Foo"}
+ expect resp.status == 200
+ expect resp.body == "1"
+} -run
+
+client c2 {
+ txreq -url "/2"
+ rxresp
+ expect resp.http.Hit == <undef>
+ expect resp.body == "3"
+} -run
+
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index a370b3274..a02c40229 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -268,6 +268,12 @@ sp_variables = [
( 'client',), """
"""
),
+ ('req.grace',
+ 'DURATION',
+ ( 'client',),
+ ( 'client',), """
+ """
+ ),
('req.xid',
'STRING',
( 'client',),
More information about the varnish-commit
mailing list