[4.1] a02e4f2 Introduce ttl_now and the new way of calculating TTLs in VCL
PÃ¥l Hermunn Johansen
hermunn at varnish-software.com
Tue Feb 20 16:20:13 UTC 2018
commit a02e4f277c3ff12c8ef05b692713f87813a85da9
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date: Tue Feb 20 16:25:26 2018 +0100
Introduce ttl_now and the new way of calculating TTLs in VCL
This is a back port of 33143e05c0720e5be0df4ac5b20a0fcc8a6874e8 in
master, and for this reason it is a little strange.
The strangeness is due to the fact that obj.ttl is not available
in vcl_deliver here, but it is in master. This commit could have
been much simpler without ttl_now, but the function is taken to
4.1 regardles. The reason is that introducing obj.ttl in
vcl_deliver is straightforward, and if someone is to do that in
the future, the code in ttl_now(VRT_CTX) makes sure that obj.ttl
will behave as in master, also in vcl_deliver.
If obj.ttl is introduced in vcl_deliver, than also the two test
cases s00008.vtc and s00009.vtc should be brought in, to make sure
that obj.ttl works as expected.
The following is the test from the commit in master:
A new fucntion, ttl_now(VRT_CTX), defines what "now" is when ttl
and age are calculated in various VCL subs. To sum up,
* Before a backend fetch on the client side (vcl_recv, vcl_hit,
vcl_miss) we use t_req from the request. This is the significance
in this commit, and fixes the bug demonstrated by r02555.vtc.
* On the backend side, most notably vcl_backend_responce, we keep
the old "now" by simply using ctx->now.
* In vcl_deliver we use ctx->now, as before.
It was necessary to make all purges use t_req as their base time.
Then, to not break c00041.vtc it was necessary to change from ">="
to ">" in HSH_Lookup.
All VMODs that currently use HSH_purge must change to using
VRT_purge.
Conflicts:
bin/varnishd/cache/cache_hash.c
bin/varnishd/cache/cache_objhead.h
bin/varnishd/cache/cache_req_fsm.c
bin/varnishd/cache/cache_vrt.c
bin/varnishd/cache/cache_vrt_var.c
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 42f2db6..88f50a7 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -429,7 +429,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
if (vary != NULL && !VRY_Match(req, vary))
continue;
- if (EXP_Ttl(req, &oc->exp) >= req->t_req) {
+ if (EXP_Ttl(req, &oc->exp) > req->t_req) {
/* If still valid, use it */
assert(oh->refcnt > 1);
assert(oc->objhead == oh);
@@ -574,13 +574,12 @@ hsh_rush(struct worker *wrk, struct objhead *oh)
*/
void
-HSH_Purge(struct worker *wrk, struct objhead *oh, double ttl, double grace,
+HSH_Purge(struct worker *wrk, struct objhead *oh, double ttl_now, double ttl, double grace,
double keep)
{
struct objcore *oc, **ocp;
unsigned spc, ospc, nobj, n, n_tot = 0;
int more = 0;
- double now;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
@@ -607,7 +606,6 @@ double keep)
ocp = (void*)wrk->aws->f;
Lck_Lock(&oh->mtx);
assert(oh->refcnt > 0);
- now = VTIM_real();
VTAILQ_FOREACH(oc, &oh->objcs, list) {
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
assert(oc->objhead == oh);
@@ -647,7 +645,7 @@ double keep)
for (n = 0; n < nobj; n++) {
oc = ocp[n];
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
- EXP_Rearm(oc, now, ttl, grace, keep);
+ EXP_Rearm(oc, ttl_now, ttl, grace, keep);
(void)HSH_DerefObjCore(wrk, &oc);
}
n_tot += nobj;
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 46e7765..25c548f 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -766,7 +766,7 @@ cnt_recv(struct worker *wrk, struct req *req)
/*--------------------------------------------------------------------
* Find the objhead, purge it.
*
- * XXX: We should ask VCL if we should fetch a new copy of the object.
+ * In VCL, a restart is necessary to get a new object
*/
static enum req_fsm_nxt
@@ -791,7 +791,7 @@ cnt_purge(struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(boc, OBJCORE_MAGIC);
VRY_Finish(req, DISCARD);
- HSH_Purge(wrk, boc->objhead, 0, 0, 0);
+ HSH_Purge(wrk, boc->objhead, req->t_req, 0, 0, 0);
AZ(HSH_DerefObjCore(wrk, &boc));
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 678d8aa..f1955e7 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -510,14 +510,13 @@ VRT_purge(VRT_CTX, double ttl, double grace, double keep)
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(ctx->req->wrk, WORKER_MAGIC);
- if (ctx->method == VCL_MET_HIT)
- HSH_Purge(ctx->req->wrk, ctx->req->objcore->objhead,
- ttl, grace, keep);
- else if (ctx->method == VCL_MET_MISS)
+
+ if (ctx->method == VCL_MET_HIT || ctx->method == VCL_MET_MISS)
HSH_Purge(ctx->req->wrk, ctx->req->objcore->objhead,
- ttl, grace, keep);
+ ctx->req->t_req, ttl, grace, keep);
}
+
/*--------------------------------------------------------------------
* Simple stuff
*/
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index 94441ed..bd67551 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -34,6 +34,8 @@
#include "common/heritage.h"
#include "hash/hash_slinger.h"
+#include "vcl.h"
+
#include "cache_director.h"
#include "vrt.h"
#include "vrt_obj.h"
@@ -485,10 +487,23 @@ VRT_r_bereq_retries(VRT_CTX)
* ttl is relative to t_origin
* grace&keep are relative to ttl
* In VCL:
- * ttl is relative to now
+ * ttl is relative to "ttl_now", which is t_req on the client
+ * side, except in vcl_deliver, where it is ctx->now. On the
+ * fetch side "ttl_now" is ctx->now (which is bo->t_prev).
* grace&keep are relative to ttl
*/
+static double ttl_now(VRT_CTX)
+{
+ if (ctx->bo) {
+ return (ctx->now);
+ } else {
+ CHECK_OBJ(ctx->req, REQ_MAGIC);
+ return (ctx->method == VCL_MET_DELIVER
+ ? ctx->now : ctx->req->t_req);
+ }
+}
+
#define VRT_DO_EXP_L(which, sexp, fld, offset) \
\
void \
@@ -520,14 +535,14 @@ VRT_r_##which##_##fld(VRT_CTX) \
}
VRT_DO_EXP_R(obj, ctx->req->objcore->exp, ttl,
- ctx->now - ctx->req->objcore->exp.t_origin)
+ ttl_now(ctx) - ctx->req->objcore->exp.t_origin)
VRT_DO_EXP_R(obj, ctx->req->objcore->exp, grace, 0)
VRT_DO_EXP_R(obj, ctx->req->objcore->exp, keep, 0)
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore->exp, ttl,
- ctx->now - ctx->bo->fetch_objcore->exp.t_origin)
+ ttl_now(ctx) - ctx->bo->fetch_objcore->exp.t_origin)
VRT_DO_EXP_R(beresp, ctx->bo->fetch_objcore->exp, ttl,
- ctx->now - ctx->bo->fetch_objcore->exp.t_origin)
+ ttl_now(ctx) - ctx->bo->fetch_objcore->exp.t_origin)
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore->exp, grace, 0)
VRT_DO_EXP_R(beresp, ctx->bo->fetch_objcore->exp, grace, 0)
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore->exp, keep, 0)
@@ -543,7 +558,7 @@ VRT_r_##which##_##age(VRT_CTX) \
{ \
\
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); \
- return(ctx->now - sexp.t_origin); \
+ return(ttl_now(ctx) - sexp.t_origin); \
}
VRT_DO_AGE_R(obj, ctx->req->objcore->exp)
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index 736f1b5..e9cf3d7 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -69,8 +69,8 @@ void HSH_Ref(struct objcore *o);
void HSH_Init(const struct hash_slinger *slinger);
void HSH_AddString(struct req *, void *ctx, const char *str);
void HSH_Insert(struct worker *, const void *hash, struct objcore *);
-void HSH_Purge(struct worker *, struct objhead *, double ttl, double grace,
- double keep);
+void HSH_Purge(struct worker *, struct objhead *, double ttl_now, double ttl,
+ double grace, double keep);
void HSH_config(const char *h_arg);
struct busyobj *HSH_RefBusy(const struct objcore *oc);
struct objcore *HSH_Private(struct worker *wrk);
diff --git a/bin/varnishtest/tests/r02555.vtc b/bin/varnishtest/tests/r02555.vtc
new file mode 100644
index 0000000..7a0fd8b
--- /dev/null
+++ b/bin/varnishtest/tests/r02555.vtc
@@ -0,0 +1,75 @@
+varnishtest "Expiry during processing"
+
+# When a object runs out of ttl+grace during processing of a
+# request, we want the calculation in the builtin VCL to match
+# the calculation of hit+grace in the C code.
+
+barrier b1 sock 2
+barrier b2 sock 2
+
+server s1 {
+ rxreq
+ expect req.url == "/1"
+ txresp
+
+ # If we get a second request to /1 here, it will be because a
+ # hit was became a miss in the builtin sub vcl_hit or because
+ # we never got to vcl_hit. We want a hit and a deliver.
+
+ rxreq
+ expect req.url == "/2"
+ txresp
+} -start
+
+varnish v1 -vcl+backend {
+ import ${vmod_std};
+ import ${vmod_debug};
+
+ sub vcl_recv {
+ if (req.http.Sleep) {
+ # This will make the object expire while inside this VCL sub
+ if (!debug.barrier_sync("${b1_sock}")) {
+ return (synth(400));
+ }
+ if (!debug.barrier_sync("${b2_sock}")) {
+ return (synth(400));
+ }
+ }
+ # Update the last timestamp inside varnish
+ std.timestamp("T");
+ }
+ sub vcl_hit {
+ set req.http.X-was-hit = "true";
+ # no return here. Will the builtin VCL take us to vcl_miss?
+ }
+ sub vcl_miss {
+ set req.http.X-was-miss = "true";
+ }
+ sub vcl_backend_response {
+ # Very little ttl, a lot of keep
+ set beresp.ttl = 1s;
+ set beresp.grace = 0s;
+ set beresp.keep = 1w;
+ }
+} -start
+
+client c1 {
+ delay .5
+ txreq -url "/1"
+ rxresp
+ expect resp.status == 200
+ txreq -url "/1" -hdr "Sleep: true"
+ rxresp
+ expect resp.status == 200
+ # Final request to verify that the right amount of requests got to the backend
+ txreq -url "/2"
+ rxresp
+ expect resp.status == 200
+} -start
+
+# help for sleeping inside of vcl
+barrier b1 sync
+delay 1.5
+barrier b2 sync
+
+client c1 -wait
diff --git a/bin/varnishtest/tests/s00007.vtc b/bin/varnishtest/tests/s00007.vtc
new file mode 100644
index 0000000..39eecf5
--- /dev/null
+++ b/bin/varnishtest/tests/s00007.vtc
@@ -0,0 +1,42 @@
+varnishtest "Effective TTL for for a slow backend"
+
+server s1 {
+ rxreq
+ delay 2
+ txresp -body "foo"
+
+ # The second request is never used, but is here to give a
+ # better error if varnish decides to fetch the object the
+ # second time
+
+ rxreq
+ txresp -body "bar"
+} -start
+
+varnish v1 -arg "-p default_ttl=3 -p default_grace=0" -vcl+backend {
+ sub vcl_backend_response {
+ set beresp.http.X-ttl = beresp.ttl;
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "foo"
+ expect resp.http.x-ttl <= 3
+ expect resp.http.x-ttl >= 2
+ delay 2
+
+ # It is now 2 seconds since the first response was received
+ # from the backend, but 4 seconds since the first request was
+ # sent to the backend. Timeout is 3 seconds, and here we
+ # consider the object _not_ expired, and thus do not want a
+ # refetch.
+
+ txreq
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "foo"
+} -run
+
More information about the varnish-commit
mailing list