[master] 33143e0 Introduce ttl_now and the new way of calculating TTLs in VCL
PÃ¥l Hermunn Johansen
hermunn at varnish-software.com
Sun Feb 18 22:47:06 UTC 2018
commit 33143e05c0720e5be0df4ac5b20a0fcc8a6874e8
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date: Sun Feb 18 23:45:08 2018 +0100
Introduce ttl_now and the new way of calculating TTLs in VCL
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.
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 2aac129..5a0b544 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -436,7 +436,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
continue;
}
- if (EXP_Ttl(req, oc) >= req->t_req) {
+ if (EXP_Ttl(req, oc) > req->t_req) {
/* If still valid, use it */
assert(oh->refcnt > 1);
assert(oc->objhead == oh);
@@ -590,13 +590,12 @@ hsh_rush2(struct worker *wrk, struct rush *r)
*/
unsigned
-HSH_Purge(struct worker *wrk, struct objhead *oh, double ttl, double grace,
-double keep)
+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);
@@ -623,7 +622,6 @@ double keep)
ocp = (void*)wrk->aws->f;
Lck_Lock(&oh->mtx);
assert(oh->refcnt > 0);
- now = VTIM_real();
VTAILQ_FOREACH(oc, &oh->objcs, hsh_list) {
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
assert(oc->objhead == oh);
@@ -663,7 +661,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, 0);
}
n_tot += nobj;
diff --git a/bin/varnishd/cache/cache_objhead.h b/bin/varnishd/cache/cache_objhead.h
index f4b7055..6bcfbd9 100644
--- a/bin/varnishd/cache/cache_objhead.h
+++ b/bin/varnishd/cache/cache_objhead.h
@@ -73,7 +73,7 @@ enum lookup_e HSH_Lookup(struct req *, struct objcore **, struct objcore **,
int always_insert);
void HSH_Ref(struct objcore *o);
void HSH_AddString(struct req *, void *ctx, const char *str);
-unsigned HSH_Purge(struct worker *, struct objhead *, double ttl, double grace,
- double keep);
+unsigned HSH_Purge(struct worker *, struct objhead *, double ttl_now,
+ double ttl, double grace, double keep);
struct objcore *HSH_Private(const struct worker *wrk);
void HSH_Abandon(struct objcore *oc);
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 142425c..56bf79e 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -933,7 +933,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
@@ -958,7 +958,7 @@ cnt_purge(struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(boc, OBJCORE_MAGIC);
VRY_Finish(req, DISCARD);
- (void)HSH_Purge(wrk, boc->objhead, 0, 0, 0);
+ (void)HSH_Purge(wrk, boc->objhead, req->t_req, 0, 0, 0);
AZ(HSH_DerefObjCore(wrk, &boc, 1));
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index ef05b9c..7bde5b8 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -1126,6 +1126,8 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
ctx.ws = req->ws;
}
if (bo != NULL) {
+ if(req)
+ assert(method == VCL_MET_PIPE);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
CHECK_OBJ_NOTNULL(bo->vcl, VCL_MAGIC);
vsl = bo->vsl;
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 296fa45..053db5c 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -661,7 +661,7 @@ VRT_purge(VRT_CTX, double ttl, double grace, double keep)
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(ctx->req->wrk, WORKER_MAGIC);
return (HSH_Purge(ctx->req->wrk, ctx->req->objcore->objhead,
- ttl, grace, keep));
+ ctx->req->t_req, ttl, grace, keep));
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index bb82fd6..ef6da50 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -35,6 +35,8 @@
#include "cache_varnishd.h"
#include "common/heritage.h"
+#include "vcl.h"
+
#include "cache_director.h"
#include "vrt_obj.h"
@@ -516,10 +518,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, oc, fld, offset) \
\
void \
@@ -551,14 +566,14 @@ VRT_r_##which##_##fld(VRT_CTX) \
}
VRT_DO_EXP_R(obj, ctx->req->objcore, ttl,
- ctx->now - ctx->req->objcore->t_origin)
+ ttl_now(ctx) - ctx->req->objcore->t_origin)
VRT_DO_EXP_R(obj, ctx->req->objcore, grace, 0)
VRT_DO_EXP_R(obj, ctx->req->objcore, keep, 0)
-
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore, ttl,
- ctx->now - ctx->bo->fetch_objcore->t_origin)
+ ttl_now(ctx) - ctx->bo->fetch_objcore->t_origin)
VRT_DO_EXP_R(beresp, ctx->bo->fetch_objcore, ttl,
- ctx->now - ctx->bo->fetch_objcore->t_origin)
+ ttl_now(ctx) - ctx->bo->fetch_objcore->t_origin)
+
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore, grace, 0)
VRT_DO_EXP_R(beresp, ctx->bo->fetch_objcore, grace, 0)
VRT_DO_EXP_L(beresp, ctx->bo->fetch_objcore, keep, 0)
@@ -574,7 +589,7 @@ VRT_r_##which##_##age(VRT_CTX) \
{ \
\
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); \
- return (ctx->now - oc->t_origin); \
+ return (ttl_now(ctx) - oc->t_origin); \
}
VRT_DO_AGE_R(obj, ctx->req->objcore)
diff --git a/bin/varnishtest/tests/r02555.vtc b/bin/varnishtest/tests/r02555.vtc
new file mode 100644
index 0000000..27182e4
--- /dev/null
+++ b/bin/varnishtest/tests/r02555.vtc
@@ -0,0 +1,68 @@
+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 std;
+ import vtc;
+
+ sub vcl_recv {
+ if (req.http.Sleep) {
+ # This will make the object expire while inside this VCL sub
+ vtc.barrier_sync("${b1_sock}");
+ vtc.barrier_sync("${b2_sock}");
+ }
+ # 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
+ txreq -url "/1" -hdr "Sleep: true"
+ rxresp
+ # Final request to verify that the right amount of requests got to the backend
+ txreq -url "/2"
+ rxresp
+} -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
+
diff --git a/bin/varnishtest/tests/s00008.vtc b/bin/varnishtest/tests/s00008.vtc
new file mode 100644
index 0000000..054c2eb
--- /dev/null
+++ b/bin/varnishtest/tests/s00008.vtc
@@ -0,0 +1,28 @@
+varnishtest "setting ttl in vcl_backend_response for slow backends"
+
+server s1 {
+ rxreq
+ delay 2
+ txresp -body "foo"
+ # The second request is never used
+ rxreq
+ txresp -body "bar"
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_backend_response {
+ set beresp.ttl = 10s;
+ }
+ sub vcl_deliver {
+ set resp.http.X-ttl = obj.ttl;
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "foo"
+ expect resp.http.X-ttl <= 10
+ expect resp.http.X-ttl >= 9
+} -run
diff --git a/bin/varnishtest/tests/s00009.vtc b/bin/varnishtest/tests/s00009.vtc
new file mode 100644
index 0000000..4f7d28d
--- /dev/null
+++ b/bin/varnishtest/tests/s00009.vtc
@@ -0,0 +1,46 @@
+varnishtest "Correct obj.ttl is when backend and processing are slow"
+
+barrier b1 sock 2
+barrier b2 sock 2
+
+server s1 {
+ rxreq
+ delay 2
+ txresp -body "foo"
+ # The second request is never used
+ rxreq
+ txresp -body "bar"
+} -start
+
+varnish v1 -vcl+backend {
+ import vtc;
+ sub vcl_backend_response {
+ # Simulate processing for 1.5 sec
+ vtc.barrier_sync("${b1_sock}");
+ vtc.barrier_sync("${b2_sock}");
+
+ # Moving this above the processing should not change
+ # anything.
+
+ set beresp.ttl = 10s;
+ }
+ sub vcl_deliver {
+ set resp.http.X-ttl = obj.ttl;
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 200
+ expect resp.body == "foo"
+ expect resp.http.X-ttl <= 9
+ expect resp.http.X-ttl >= 8
+} -start
+
+# help for sleeping inside of vcl
+barrier b1 sync
+delay 1.5
+barrier b2 sync
+
+client c1 -wait
More information about the varnish-commit
mailing list