diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 6df7d546d61..172d8fc687a 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -896,9 +896,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo) * * replaces a stale object unless * - abandoning the bereq or - * - leaving vcl_backend_error with return (deliver) and beresp.ttl == 0s or - * - there is a waitinglist on this object because in this case the default ttl - * would be 1s, so we might be looking at the same case as the previous + * - leaving vcl_backend_error with return (deliver) * * We do want the stale replacement to avoid an object pileup with short ttl and * long grace/keep, yet there could exist cases where a cache object is @@ -954,23 +952,9 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo) stale = bo->stale_oc; oc->t_origin = now; - if (!VTAILQ_EMPTY(&oc->objhead->waitinglist)) { - /* - * If there is a waitinglist, it means that there is no - * grace-able object, so cache the error return for a - * short time, so the waiting list can drain, rather than - * each objcore on the waiting list sequentially attempt - * to fetch from the backend. - */ - oc->ttl = 1; - oc->grace = 5; - oc->keep = 5; - stale = NULL; - } else { - oc->ttl = 0; - oc->grace = 0; - oc->keep = 0; - } + oc->ttl = 0; + oc->grace = 0; + oc->keep = 0; synth_body = VSB_new_auto(); AN(synth_body); diff --git a/bin/varnishtest/tests/c00131.vtc b/bin/varnishtest/tests/c00131.vtc new file mode 100644 index 00000000000..c9ce19bd588 --- /dev/null +++ b/bin/varnishtest/tests/c00131.vtc @@ -0,0 +1,71 @@ +varnishtest "waiting list rush from vcl_backend_error" + +barrier b1 sock 2 +barrier b2 sock 2 + +server s1 { + rxreq + send garbage +} -start + +varnish v1 -cliok "param.set debug +syncvsl,+waitinglist" +varnish v1 -vcl+backend { + import vtc; + sub vcl_backend_fetch { + vtc.barrier_sync("${b1_sock}"); + vtc.barrier_sync("${b2_sock}"); + } + sub vcl_backend_error { + set beresp.http.error-vxid = bereq.xid; + } +} -start + +client c1 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +barrier b1 sync + +logexpect l1 -v v1 -q Debug -g raw { + loop 4 { + expect * * Debug "on waiting list" + } +} -start + +client c2 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c3 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c4 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c5 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +logexpect l1 -wait + +barrier b2 sync + +client c1 -wait +client c2 -wait +client c3 -wait +client c4 -wait +client c5 -wait + +varnish v1 -expect n_expired == 1 diff --git a/doc/sphinx/reference/vcl_step.rst b/doc/sphinx/reference/vcl_step.rst index ee07f616923..86fad6312a2 100644 --- a/doc/sphinx/reference/vcl_step.rst +++ b/doc/sphinx/reference/vcl_step.rst @@ -378,19 +378,14 @@ This subroutine is called if we fail the backend fetch or if Returning with :ref:`abandon` does not leave a cache object. -If returning with ``deliver`` and a ``beresp.ttl > 0s``, a synthetic -cache object is generated in VCL, whose body may be constructed using -the ``synthetic()`` function. - -When there is a waiting list on the object, the default ``ttl`` will -be positive (currently one second), set before entering -``vcl_backend_error``. This is to avoid request serialization and -hammering on a potentially failing backend. - -Since these synthetic objects are cached in these special -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 returning with ``deliver`` and ``beresp.uncacheable == false``, a +synthetic cache object is generated in VCL, whose body may be constructed +using the ``synthetic()`` function. + +Since these synthetic objects are cached in these special circumstances, +be cautious with putting private information there. If you really must, +then you need to explicitly set ``beresp.uncacheable`` to ``true`` in +``vcl_backend_error``. The `vcl_backend_error` subroutine may terminate with calling ``return()`` with one of the following keywords: