From 2812e186afa0fdf2f06efa82e21838097ea94189 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune Date: Wed, 11 Oct 2023 19:51:58 +0200 Subject: [PATCH] vcl_vrt: Skip VCL execution if the client is gone 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 61a15cbffe1141c13b87e30d48ce1402f84433bf Refs e5efc2c8dc0d003e5f0fa1a30b598f5949112897 Refs ba54dc919076b1ddb85434d886ce82a6553d926b Refs 6f50a00f80c7f74b2a8b18bb80593f58b74816fd Refs b8816994cfab58261d8000ea8e6941cb5de640fa --- bin/varnishd/cache/cache_vrt_vcl.c | 37 ++++++++++++++++++++++++++++++ doc/sphinx/reference/vsl.rst | 4 ++++ include/tbl/feature_bits.h | 5 ++++ include/tbl/params.h | 4 +++- include/tbl/req_flags.h | 1 + lib/libvsc/VSC_main.vsc | 7 ++++++ 6 files changed, 57 insertions(+), 1 deletion(-) diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c index c98274e9d57..85b594b7d86 100644 --- a/bin/varnishd/cache/cache_vrt_vcl.c +++ b/bin/varnishd/cache/cache_vrt_vcl.c @@ -42,6 +42,7 @@ #include "vbm.h" #include "cache_director.h" +#include "cache_transport.h" #include "cache_vcl.h" #include "vcc_interface.h" @@ -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. * @@ -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); diff --git a/doc/sphinx/reference/vsl.rst b/doc/sphinx/reference/vsl.rst index e8ea7a36d89..d8b440a088b 100644 --- a/doc/sphinx/reference/vsl.rst +++ b/doc/sphinx/reference/vsl.rst @@ -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 ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/include/tbl/feature_bits.h b/include/tbl/feature_bits.h index 1ff2a361fbc..808efe9174c 100644 --- a/include/tbl/feature_bits.h +++ b/include/tbl/feature_bits.h @@ -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 */ diff --git a/include/tbl/params.h b/include/tbl/params.h index 83054eef3de..ebabc45f21c 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1782,7 +1782,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" diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h index 88eaff6391c..6a9e6acbb18 100644 --- a/include/tbl/req_flags.h +++ b/include/tbl/req_flags.h @@ -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" diff --git a/lib/libvsc/VSC_main.vsc b/lib/libvsc/VSC_main.vsc index a4049fccb25..fc55075164d 100644 --- a/lib/libvsc/VSC_main.vsc +++ b/lib/libvsc/VSC_main.vsc @@ -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