Skip to content

Commit

Permalink
vcl_vrt: Skip VCL execution if the client is gone
Browse files Browse the repository at this point in the history
Once a client is reportedly gone, processing its VCL task(s) is just a
waste of resources. The execution of client-facing VCL is intercepted
and an artificial return(fail) is returned in that scenario.

Thanks to the introduction of the universal return(fail) proper error
handling and resource tear down is already in place, which makes this
change safe modulus unknown bugs. This adds a circuit breaker anywhere
in the client state machine where there is VCL execution.

A new Reset time stamp is logged to convey when a task does not complete
because the client is gone. This is a good complement to the walk away
feature and its original circuit breaker for the waiting list, but this
has not been integrated yet.

While the request is technically failed, it won't increase the vcl_fail
counter, and a new req_reset counter is incremented. This new behavior
is guarded by a new vcl_req_reset feature flag, enabled by default.

Refs #3835
Refs 61a15cb
Refs e5efc2c
Refs ba54dc9
Refs 6f50a00
Refs b881699
  • Loading branch information
dridi committed Oct 18, 2023
1 parent ff8ae1f commit 6ad7be6
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 1 deletion.
37 changes: 37 additions & 0 deletions bin/varnishd/cache/cache_vrt_vcl.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "vbm.h"

#include "cache_director.h"
#include "cache_transport.h"
#include "cache_vcl.h"
#include "vcc_interface.h"

Expand Down Expand Up @@ -521,6 +522,40 @@ VRT_VCL_Allow_Discard(struct vclref **refp)
FREE_OBJ(ref);
}

/*--------------------------------------------------------------------
*/

static int
req_poll(struct worker *wrk, struct req *req)
{
struct req *top;

/* NB: Since a fail transition leads to vcl_synth, the request may be
* short-circuited twice.
*/
if (req->req_reset) {
wrk->vpi->handling = VCL_RET_FAIL;
return (-1);
}

top = req->top->topreq;
CHECK_OBJ_NOTNULL(top, REQ_MAGIC);
CHECK_OBJ_NOTNULL(top->transport, TRANSPORT_MAGIC);

if (!FEATURE(FEATURE_VCL_REQ_RESET))
return (0);
if (top->transport->poll == NULL)
return (0);
if (top->transport->poll(top) >= 0)
return (0);

VSLb_ts_req(req, "Reset", W_TIM_real(wrk));
wrk->stats->req_reset++;
wrk->vpi->handling = VCL_RET_FAIL;
req->req_reset = 1;
return (-1);
}

/*--------------------------------------------------------------------
* Method functions to call into VCL programs.
*
Expand Down Expand Up @@ -552,6 +587,8 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC);
CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
if (req_poll(wrk, req))
return;
VCL_Req2Ctx(&ctx, req);
}
assert(ctx.now != 0);
Expand Down
4 changes: 4 additions & 0 deletions doc/sphinx/reference/vsl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ Resp
Restart
Client request is being restarted.

Reset
The client closed its connection or reset its stream. Request
processing is interrupted and considered failed.

Pipe handling timestamps
~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
5 changes: 5 additions & 0 deletions include/tbl/feature_bits.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ FEATURE_BIT(TRACE, trace,
"Required for tracing vcl_init / vcl_fini"
)

FEATURE_BIT(VCL_REQ_RESET, vcl_req_reset,
"Stop processing client VCL once the client is gone. "
"When this happens MAIN.req_reset is incremented."
)

#undef FEATURE_BIT

/*lint -restore */
4 changes: 3 additions & 1 deletion include/tbl/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -1824,7 +1824,9 @@ PARAM_PRE
PARAM_BITS(
/* name */ feature,
/* fld */ feature_bits,
/* def */ "+validate_headers",
/* def */
"+validate_headers,"
"+vcl_req_reset",
/* descr */
"Enable/Disable various minor features.\n"
"\tdefault\tSet default value\n"
Expand Down
1 change: 1 addition & 0 deletions include/tbl/req_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ REQ_FLAG(is_hit, 0, 0, "")
REQ_FLAG(waitinglist, 0, 0, "")
REQ_FLAG(want100cont, 0, 0, "")
REQ_FLAG(late100cont, 0, 0, "")
REQ_FLAG(req_reset, 0, 0, "")
#define REQ_BEREQ_FLAG(lower, vcl_r, vcl_w, doc) \
REQ_FLAG(lower, vcl_r, vcl_w, doc)
#include "tbl/req_bereq_flags.h"
Expand Down
7 changes: 7 additions & 0 deletions lib/libvsc/VSC_main.vsc
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,13 @@
Number of times an HTTP/2 stream was refused because the queue was
too long already. See also parameter thread_queue_limit.

.. varnish_vsc:: req_reset
:group: wrk
:oneliner: Requests reset

Number of times a client left before the VCL processing of its
requests completed.

.. varnish_vsc:: n_object
:type: gauge
:group: wrk
Expand Down

0 comments on commit 6ad7be6

Please sign in to comment.