diff --git a/bin/varnishd/builtin.vcl b/bin/varnishd/builtin.vcl index 770081366ca..6b49d754e17 100644 --- a/bin/varnishd/builtin.vcl +++ b/bin/varnishd/builtin.vcl @@ -257,6 +257,19 @@ sub vcl_backend_error { } sub vcl_builtin_backend_error { + call vcl_refresh_error; + call vcl_beresp_error; +} + +sub vcl_refresh_error { + if (beresp.was_304) { + unset bereq.http.If-Modified-Since; + unset bereq.http.If-None-Match; + return (retry(fetch)); + } +} + +sub vcl_beresp_error { set beresp.http.Content-Type = "text/html; charset=utf-8"; set beresp.http.Retry-After = "5"; set beresp.body = {" diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 0f86d0305c7..78d50ba5305 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -42,6 +42,7 @@ #define FETCH_STEPS \ FETCH_STEP(mkbereq, MKBEREQ) \ FETCH_STEP(retry, RETRY) \ + FETCH_STEP(prepfetch, PREPFETCH) \ FETCH_STEP(startfetch, STARTFETCH) \ FETCH_STEP(condfetch, CONDFETCH) \ FETCH_STEP(fetch, FETCH) \ @@ -269,7 +270,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo) } http_ForceField(bo->bereq0, HTTP_HDR_PROTO, "HTTP/1.1"); - if (bo->stale_oc != NULL && + if (bo->stale_oc != NULL && !(bo->stale_oc->flags & OC_F_DYING) && ObjCheckFlag(bo->wrk, bo->stale_oc, OF_IMSCAND) && (bo->stale_oc->boc != NULL || ObjGetLen(wrk, bo->stale_oc) != 0)) { AZ(bo->stale_oc->flags & (OC_F_HFM|OC_F_PRIVATE)); @@ -298,7 +299,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo) bo->req = NULL; ObjSetState(bo->wrk, oc, BOS_REQ_DONE); } - return (F_STP_STARTFETCH); + return (F_STP_PREPFETCH); } /*-------------------------------------------------------------------- @@ -326,24 +327,30 @@ vbf_stp_retry(struct worker *wrk, struct busyobj *bo) assert(bo->director_state == DIR_S_NULL); /* reset other bo attributes - See VBO_GetBusyObj */ - bo->storage = NULL; bo->do_esi = 0; bo->do_stream = 1; bo->was_304 = 0; bo->err_code = 0; bo->err_reason = NULL; + if (bo->htc != NULL) + bo->htc->doclose = SC_NULL; + + if (bo->retry_fetch) { + bo->retry_fetch = 0; + return (F_STP_STARTFETCH); + } + + bo->storage = NULL; bo->connect_timeout = NAN; bo->first_byte_timeout = NAN; bo->between_bytes_timeout = NAN; - if (bo->htc != NULL) - bo->htc->doclose = SC_NULL; // XXX: BereqEnd + BereqAcct ? VSL_ChgId(bo->vsl, "bereq", "retry", VXID_Get(wrk, VSL_BACKENDMARKER)); VSLb_ts_busyobj(bo, "Start", bo->t_prev); http_VSL_log(bo->bereq); - return (F_STP_STARTFETCH); + return (F_STP_PREPFETCH); } /*-------------------------------------------------------------------- @@ -388,17 +395,11 @@ vbf_304_logic(struct busyobj *bo) */ static const struct fetch_step * v_matchproto_(vbf_state_f) -vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) +vbf_stp_prepfetch(struct worker *wrk, struct busyobj *bo) { - int i; - vtim_real now; - unsigned handling; - struct objcore *oc; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); - oc = bo->fetch_objcore; - CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); AZ(bo->storage); bo->storage = bo->uncacheable ? stv_transient : STV_next(); @@ -416,6 +417,25 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) assert (wrk->vpi->handling == VCL_RET_FETCH || wrk->vpi->handling == VCL_RET_ERROR); + return (F_STP_STARTFETCH); +} + +/*-------------------------------------------------------------------- + * Send bereq, fetch beresp, run vcl_backend_response + */ + +static const struct fetch_step * v_matchproto_(vbf_state_f) +vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) +{ + struct objcore *oc; + unsigned handling; + vtim_real now; + int i; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); + oc = bo->fetch_objcore; + CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); HTTP_Setup(bo->beresp, bo->ws, bo->vsl, SLT_BerespMethod); @@ -835,6 +855,13 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo) stale_oc = bo->stale_oc; CHECK_OBJ_NOTNULL(stale_oc, OBJCORE_MAGIC); + if (stale_oc->flags & OC_F_DYING) { + (void)VFP_Error(bo->vfc, "Template object invalidated"); + vbf_cleanup(bo); + wrk->stats->fetch_failed++; + return (F_STP_ERROR); + } + stale_boc = HSH_RefBoc(stale_oc); CHECK_OBJ_ORNULL(stale_boc, BOC_MAGIC); if (stale_boc) { diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c index d8630d34774..2a6737c4583 100644 --- a/bin/varnishd/cache/cache_vrt.c +++ b/bin/varnishd/cache/cache_vrt.c @@ -637,6 +637,16 @@ VRT_SetHdr(VRT_CTX, VCL_HEADER hs, const char *pfx, VCL_STRANDS s) /*--------------------------------------------------------------------*/ +void +VPI_retry(VRT_CTX, unsigned is_task) +{ + + CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); + CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC); + ctx->bo->retry_fetch = !is_task; + VRT_handling(ctx, VCL_RET_RETRY); +} + VCL_VOID VRT_handling(VRT_CTX, unsigned hand) { diff --git a/bin/varnishtest/tests/c00129.vtc b/bin/varnishtest/tests/c00129.vtc new file mode 100644 index 00000000000..888f5e39de2 --- /dev/null +++ b/bin/varnishtest/tests/c00129.vtc @@ -0,0 +1,58 @@ +varnishtest "304 on invalidated stale object" + +barrier b1 cond 2 +barrier b2 cond 2 + +server s1 { + rxreq + txresp -hdr {ETag: "foo"} -body corrupted + + rxreq + expect req.http.If-None-Match == {"foo"} + barrier b1 sync + barrier b2 sync + txresp -status 304 -hdr {ETag: "foo"} + + rxreq + expect req.http.If-None-Match == + txresp -hdr {ETag: "foo"} -body valid +} -start + +varnish v1 -vcl+backend { + sub vcl_recv { + if (req.method == "PURGE") { + return (purge); + } + } + sub vcl_backend_response { + set beresp.ttl = 1ms; + set beresp.grace = 0s; + set beresp.keep = 10s; + } + sub vcl_deliver { + set resp.http.obj-hits = obj.hits; + } +} -start + +client c1 { + txreq + rxresp + expect resp.body == corrupted + expect resp.http.obj-hits == 0 + + delay 0.1 + + txreq + rxresp + expect resp.body == valid + expect resp.http.obj-hits == 0 +} -start + +barrier b1 sync +client c2 { + txreq -req PURGE + rxresp +} -run +barrier b2 sync + +client c1 -wait diff --git a/bin/varnishtest/tests/c00130.vtc b/bin/varnishtest/tests/c00130.vtc new file mode 100644 index 00000000000..c75f2462121 --- /dev/null +++ b/bin/varnishtest/tests/c00130.vtc @@ -0,0 +1,59 @@ +varnishtest "Demote condfetch to fetch on invalidated stale object" + +barrier b1 sock 2 +barrier b2 sock 2 + +server s1 { + rxreq + txresp -hdr {ETag: "foo"} -body corrupted + + rxreq + expect req.http.If-None-Match == + txresp -hdr {ETag: "foo"} -body valid +} -start + +varnish v1 -vcl+backend { + import vtc; + sub vcl_recv { + if (req.method == "PURGE") { + return (purge); + } + } + sub vcl_miss { + if (req.http.sync) { + vtc.barrier_sync("${b1_sock}"); + vtc.barrier_sync("${b2_sock}"); + } + } + sub vcl_backend_response { + set beresp.ttl = 1ms; + set beresp.grace = 0s; + set beresp.keep = 10s; + } + sub vcl_deliver { + set resp.http.obj-hits = obj.hits; + } +} -start + +client c1 { + txreq + rxresp + expect resp.body == corrupted + expect resp.http.obj-hits == 0 + + delay 0.1 + + txreq -hdr "sync: true" + rxresp + expect resp.body == valid + expect resp.http.obj-hits == 0 +} -start + +barrier b1 sync +client c2 { + txreq -req PURGE + rxresp +} -run +barrier b2 sync + +client c1 -wait diff --git a/doc/sphinx/reference/vcl_step.rst b/doc/sphinx/reference/vcl_step.rst index ee07f616923..fe1b46830ea 100644 --- a/doc/sphinx/reference/vcl_step.rst +++ b/doc/sphinx/reference/vcl_step.rst @@ -392,6 +392,10 @@ circumstances, be cautious with putting private information there. If you really must, then you need to explicitly set ``beresp.ttl`` to zero in ``vcl_backend_error``. +If a conditional fetch failed to process a 304 response and transitioned +to ``vcl_backend_error``, the backend request is retried for a regular +fetch within the same transaction. + The `vcl_backend_error` subroutine may terminate with calling ``return()`` with one of the following keywords: diff --git a/doc/sphinx/reference/vcl_var.rst b/doc/sphinx/reference/vcl_var.rst index 4697211648b..9bebb64be40 100644 --- a/doc/sphinx/reference/vcl_var.rst +++ b/doc/sphinx/reference/vcl_var.rst @@ -1378,7 +1378,8 @@ beresp.was_304 When ``true`` this indicates that we got a 304 response to our conditional fetch from the backend and turned - that into ``beresp.status = 200`` + that into ``beresp.status = 200``, unless the refresh + attempt failed. obj diff --git a/include/tbl/bereq_flags.h b/include/tbl/bereq_flags.h index 31660035c9d..2a689b6c954 100644 --- a/include/tbl/bereq_flags.h +++ b/include/tbl/bereq_flags.h @@ -34,6 +34,7 @@ /* lower, vcl_r, vcl_w, doc */ BEREQ_FLAG(uncacheable, 0, 0, "") // also beresp BEREQ_FLAG(is_bgfetch, 1, 0, "") +BEREQ_FLAG(retry_fetch, 0, 0, "") #define REQ_BEREQ_FLAG(lower, vcl_r, vcl_w, doc) \ BEREQ_FLAG(lower, vcl_r, vcl_w, doc) #include "tbl/req_bereq_flags.h" diff --git a/include/vcc_interface.h b/include/vcc_interface.h index 529ab9ed17e..649a4da7f32 100644 --- a/include/vcc_interface.h +++ b/include/vcc_interface.h @@ -40,6 +40,7 @@ void VPI_vcl_rel(VRT_CTX, VCL_VCL); void VPI_vcl_select(VRT_CTX, VCL_VCL); void v_noreturn_ VPI_Fail(const char *func, const char *file, int line, const char *cond); +void VPI_retry(VRT_CTX, unsigned is_task); /*********************************************************************** * VPI_count() refers to this structure for coordinates into the VCL source. diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c index f7c2ce29fbb..82aee794090 100644 --- a/lib/libvcc/vcc_action.c +++ b/lib/libvcc/vcc_action.c @@ -344,6 +344,27 @@ vcc_act_return_vcl(struct vcc *tl) /*--------------------------------------------------------------------*/ +static void +vcc_act_return_retry(struct vcc *tl) +{ + unsigned is_task = 0; + + ExpectErr(tl, '('); + vcc_NextToken(tl); + if (vcc_IdIs(tl->t, "task")) + is_task = 1; + else if (!vcc_IdIs(tl->t, "fetch")) { + VSB_printf(tl->sb, "Expected \"task\" or \"fetch\" retry.\n"); + vcc_ErrWhere(tl, tl->t); + } + ERRCHK(tl); + Fb(tl, 1, "VPI_retry(ctx, %u);\n", is_task); + SkipToken(tl, ID); + SkipToken(tl, ')'); +} + +/*--------------------------------------------------------------------*/ + static void v_matchproto_(sym_act_f) vcc_act_return(struct vcc *tl, struct token *t, struct symbol *sym) { @@ -388,6 +409,8 @@ vcc_act_return(struct vcc *tl, struct token *t, struct symbol *sym) vcc_act_return_vcl(tl); else if (hand == VCL_RET_PASS) vcc_act_return_pass(tl); + else if (hand == VCL_RET_RETRY) + vcc_act_return_retry(tl); else if (hand == VCL_RET_FAIL) vcc_act_return_fail(tl); else {