[master] 8633ce1 Fix an off-by-one in the max_restarts check

Nils Goroll nils.goroll at uplex.de
Mon Oct 2 13:41:07 UTC 2017


commit 8633ce15f63370e619b89365421ae1c750505e6c
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Oct 2 14:47:43 2017 +0200

    Fix an off-by-one in the max_restarts check
    
    max_restarts is the number of allowed restarts
    
    Related to #2405

diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 987f3d0..e36e3aa 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -727,7 +727,7 @@ cnt_restart(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
 	req->director_hint = NULL;
-	if (++req->restarts >= cache_param->max_restarts) {
+	if (++req->restarts > cache_param->max_restarts) {
 		VSLb(req->vsl, SLT_VCL_Error, "Too many restarts");
 		req->err_code = 503;
 		req->req_step = R_STP_SYNTH;
diff --git a/bin/varnishtest/tests/b00019.vtc b/bin/varnishtest/tests/b00019.vtc
index 33d3dbc..5336478 100644
--- a/bin/varnishtest/tests/b00019.vtc
+++ b/bin/varnishtest/tests/b00019.vtc
@@ -26,15 +26,17 @@ varnish v1 -vcl+backend {
 	}
 
 	sub vcl_synth {
-		if (req.restarts == 2) {
+		# when we end up here, we have _exceeded_ the number
+		# allowed restarts
+		if (req.restarts == 3) {
 			set resp.status = 200;
-			set resp.reason = "restart=2";
-		} elsif (req.restarts > 2) {
+			set resp.reason = "restart=3";
+		} elsif (req.restarts > 3) {
 			set resp.status = 501;
-			set resp.reason = "restart>2";
-		} elsif (req.restarts < 2) {
+			set resp.reason = "restart>3";
+		} elsif (req.restarts < 3) {
 			set resp.status = 500;
-			set resp.reason = "restart<2";
+			set resp.reason = "restart<3";
 		}
 	}
 } -start
diff --git a/doc/changes.rst b/doc/changes.rst
index 9b546b1..c1e45b8 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -2,6 +2,10 @@
 Varnish Cache Trunk (ongoing)
 =============================
 
+* Fixed implementation of the ``max_restarts`` limit: It used to be one
+  less than the number of allowed restarts, it now is the number of
+  ``return(restart)``s per request.
+
 ================================
 Varnish Cache 5.2.0 (2017-09-15)
 ================================


More information about the varnish-commit mailing list