[6.0] d6b7866e6 vcl_vrt: Skip VCL execution if the client is gone

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Wed Oct 18 15:27:06 UTC 2023


commit d6b7866e63b634a05268cce12640be25caffc394
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
    
    Conflicts:
            bin/varnishd/cache/cache_vrt_vcl.c
            bin/varnishd/cache/cache_vcl_vrt.c
            bin/varnishd/mgt/mgt_param_bits.c
            include/tbl/params.h

diff --git a/bin/varnishd/VSC_main.vsc b/bin/varnishd/VSC_main.vsc
index d03144367..027245d88 100644
--- a/bin/varnishd/VSC_main.vsc
+++ b/bin/varnishd/VSC_main.vsc
@@ -330,6 +330,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
diff --git a/bin/varnishd/cache/cache_vcl_vrt.c b/bin/varnishd/cache/cache_vcl_vrt.c
index 5f3bfeec0..e35ae59f8 100644
--- a/bin/varnishd/cache/cache_vcl_vrt.c
+++ b/bin/varnishd/cache/cache_vcl_vrt.c
@@ -37,8 +37,10 @@
 #include "cache_varnishd.h"
 
 #include "vcl.h"
+#include "vtim.h"
 
 #include "cache_director.h"
+#include "cache_transport.h"
 #include "cache_vcl.h"
 
 /*--------------------------------------------------------------------*/
@@ -338,6 +340,35 @@ VRT_rel_vcl(VRT_CTX, struct vclref **refp)
  * The workspace argument is where random VCL stuff gets space from.
  */
 
+static int
+req_poll(struct worker *wrk, struct req *req)
+{
+
+	CHECK_OBJ_NOTNULL(req->top, REQ_MAGIC);
+	CHECK_OBJ_NOTNULL(req->top->transport, TRANSPORT_MAGIC);
+
+	/* NB: Since a fail transition leads to vcl_synth, the request may be
+	 * short-circuited twice.
+	 */
+	if (req->req_reset) {
+		wrk->handling = VCL_RET_FAIL;
+		return (-1);
+	}
+
+	if (!FEATURE(FEATURE_VCL_REQ_RESET))
+		return (0);
+	if (req->top->transport->poll == NULL)
+		return (0);
+	if (req->top->transport->poll(req->top) >= 0)
+		return (0);
+
+	VSLb_ts_req(req, "Reset", W_TIM_real(wrk));
+	wrk->stats->req_reset++;
+	wrk->handling = VCL_RET_FAIL;
+	req->req_reset = 1;
+	return (-1);
+}
+
 static void
 vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
     void *specific, unsigned method, vcl_func_f *func)
@@ -351,6 +382,8 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
 		CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 		CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC);
 		CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC);
+		if (req_poll(wrk, req))
+			return;
 		VCL_Req2Ctx(&ctx, req);
 	}
 	if (bo != NULL) {
diff --git a/bin/varnishd/mgt/mgt_param_bits.c b/bin/varnishd/mgt/mgt_param_bits.c
index 7ea66d7e1..2767ede0a 100644
--- a/bin/varnishd/mgt/mgt_param_bits.c
+++ b/bin/varnishd/mgt/mgt_param_bits.c
@@ -220,7 +220,12 @@ tweak_feature(struct vsb *vsb, const struct parspec *par, const char *arg)
 	(void)par;
 
 	if (arg != NULL && arg != JSON_FMT) {
-		if (!strcmp(arg, "none")) {
+		if (!strcmp(arg, "default")) {
+			AZ(bit_tweak(vsb, mgt_param.feature_bits,
+				FEATURE_Reserved,
+				"+vcl_req_reset",
+				feature_tags, "feature bit", "+"));
+		}else if (!strcmp(arg, "none")) {
 			memset(mgt_param.feature_bits,
 			    0, sizeof mgt_param.feature_bits);
 		} else {
@@ -272,6 +277,6 @@ struct parspec VSL_parspec[] = {
 #define FEATURE_BIT(U, l, d, ld) "\n\t" #l "\t" d
 #include "tbl/feature_bits.h"
 #undef FEATURE_BIT
-		, 0, "none", "" },
+		, 0, "default", "" },
 	{ NULL, NULL, NULL }
 };
diff --git a/doc/sphinx/reference/vsl.rst b/doc/sphinx/reference/vsl.rst
index 4d01f5ba8..ec1814ff8 100644
--- a/doc/sphinx/reference/vsl.rst
+++ b/doc/sphinx/reference/vsl.rst
@@ -71,6 +71,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 23f1b01f8..844ecfa32 100644
--- a/include/tbl/feature_bits.h
+++ b/include/tbl/feature_bits.h
@@ -83,6 +83,12 @@ FEATURE_BIT(HTTP_DATE_POSTEL,	http_date_postel,
     "like Date:, Last-Modified:, Expires: etc."
 )
 
+FEATURE_BIT(VCL_REQ_RESET,			vcl_req_reset,
+    "Stop processing client VCL once the client is gone.",
+    "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/req_flags.h b/include/tbl/req_flags.h
index 2c0dbe803..3d3f05fdb 100644
--- a/include/tbl/req_flags.h
+++ b/include/tbl/req_flags.h
@@ -39,6 +39,7 @@ REQ_FLAG(is_hitpass,		1, 0, "")
 REQ_FLAG(waitinglist,		0, 0, "")
 REQ_FLAG(want100cont,		0, 0, "")
 REQ_FLAG(late100cont,		0, 0, "")
+REQ_FLAG(req_reset,		0, 0, "")
 #undef REQ_FLAG
 
 /*lint -restore */


More information about the varnish-commit mailing list