[7.4] 2c5b14ac2 vcl_vrt: Skip VCL execution if the client is gone

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Oct 23 17:18:09 UTC 2023


commit 2c5b14ac2f2983ad1aecf5cc60563fe24cf94406
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Oct 11 19:51:58 2023 +0200

    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

diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c
index c98274e9d..85b594b7d 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 e8ea7a36d..d8b440a08 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 1ff2a361f..808efe917 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 39cedbb13..ae622e53d 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -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"
diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h
index 88eaff639..6a9e6acbb 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 1f63b45a4..13c35d9be 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


More information about the varnish-commit mailing list