[master] c2406f4 For restarts, keep all req attributes except req.restarts and req.xid

Nils Goroll nils.goroll at uplex.de
Tue Nov 7 11:29:05 UTC 2017


commit c2406f4ff586b7f00efb86d06a250ec249f6f53d
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Oct 2 14:06:08 2017 +0200

    For restarts, keep all req attributes except req.restarts and req.xid
    
    Fixes #2405 for restarts
    Merges #2447
    
    remaining TODOs (but not the scope of this ticket):
    
    * Keep bereq attributes for bereq
    * improved rollback

diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index f0f56cc..e97ce19 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -722,7 +722,6 @@ cnt_restart(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
-	req->director_hint = NULL;
 	if (++req->restarts > cache_param->max_restarts) {
 		VSLb(req->vsl, SLT_VCL_Error, "Too many restarts");
 		req->err_code = 503;
@@ -736,7 +735,6 @@ cnt_restart(struct worker *wrk, struct req *req)
 		req->err_code = 0;
 		req->req_step = R_STP_RECV;
 	}
-	req->is_hit = 0;
 	return (REQ_FSM_MORE);
 }
 
@@ -783,22 +781,23 @@ cnt_recv(struct worker *wrk, struct req *req)
 		} else {
 			http_PrintfHeader(req->http, "X-Forwarded-For: %s", ci);
 		}
-	}
+		http_CollectHdr(req->http, H_Cache_Control);
 
-	/* By default we use the first backend */
-	AZ(req->director_hint);
-	req->director_hint = VCL_DefaultDirector(req->vcl);
-	AN(req->director_hint);
+		/* By default we use the first backend */
+		AZ(req->director_hint);
+		req->director_hint = VCL_DefaultDirector(req->vcl);
+		AN(req->director_hint);
+
+		req->d_ttl = -1;
+		req->disable_esi = 0;
+		req->hash_always_miss = 0;
+		req->hash_ignore_busy = 0;
+		req->client_identity = NULL;
+		req->storage = NULL;
+	}
 
 	req->vdc->retval = 0;
-	req->d_ttl = -1;
-	req->disable_esi = 0;
-	req->hash_always_miss = 0;
-	req->hash_ignore_busy = 0;
-	req->client_identity = NULL;
-	req->storage = NULL;
-
-	http_CollectHdr(req->http, H_Cache_Control);
+	req->is_hit = 0;
 
 	if (req->req_body_status == REQ_BODY_FAIL) {
 		req->doclose = SC_OVERLOAD;
diff --git a/bin/varnishtest/tests/c00009.vtc b/bin/varnishtest/tests/c00009.vtc
index 7931e81..6ff3d97 100644
--- a/bin/varnishtest/tests/c00009.vtc
+++ b/bin/varnishtest/tests/c00009.vtc
@@ -12,8 +12,20 @@ server s2 {
 	txresp -body "foobar"
 } -start
 
-varnish v1 -vcl+backend {
+varnish v1 -arg "-smysteve=malloc,1m" -vcl+backend {
 	sub vcl_recv {
+		if (req.restarts == 0) {
+			set req.url = "/foo";
+			set req.method = "POST";
+			set req.proto = "HTTP/1.2";
+			set req.http.preserveme = "1";
+			set req.storage = storage.mysteve;
+			set req.ttl = 42m;
+			set req.esi = false;
+			set req.backend_hint = s2;
+			set req.hash_ignore_busy = true;
+			set req.hash_always_miss = true;
+		}
 		set req.http.restarts = req.restarts;
 	}
 	sub vcl_backend_fetch {
@@ -35,14 +47,36 @@ varnish v1 -vcl+backend {
 		if (resp.status != 200) {
 			return (restart);
 		}
+		set resp.http.method = req.method;
+		set resp.http.url = req.url;
+		set resp.http.proto = req.proto;
+		set resp.http.preserveme = req.http.preserveme;
+		set resp.http.restarts = req.restarts;
+		set resp.http.storage = req.storage;
+		set resp.http.ttl = req.ttl;
+		set resp.http.esi = req.esi;
+		set resp.http.backend_hint = req.backend_hint;
+		set resp.http.hash-ignore-busy = req.hash_ignore_busy;
+		set resp.http.hash-always-miss = req.hash_always_miss;
 	}
 } -start
 
 client c1 {
-	txreq -url "/foo"
+	txreq -url "/"
 	rxresp
 	expect resp.status == 200
 	expect resp.bodylen == 6
+	expect resp.http.method == POST
+	expect resp.http.url == /foo
+	expect resp.http.proto == HTTP/1.2
+	expect resp.http.preserveme == 1
+	expect resp.http.restarts == 1
+	expect resp.http.storage == storage.mysteve
+	expect resp.http.ttl == 2520.000
+	expect resp.http.esi == false
+	expect resp.http.backend_hint == s2
+	expect resp.http.hash-ignore-busy == true
+	expect resp.http.hash-always-miss == true
 }
 
 client c1 -run
diff --git a/doc/changes.rst b/doc/changes.rst
index 971d269..f96778a 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -6,6 +6,10 @@ Varnish Cache Trunk (ongoing)
   less than the number of allowed restarts, it now is the number of
   ``return(restart)`` calls per request.
 
+* Fix behaviour of restarts to how it was originally intended:
+  Restarts now leave all the request properties in place except for
+  ``req.restarts`` and ``req.xid``, which need to change by design.
+
 ================================
 Varnish Cache 5.2.0 (2017-09-15)
 ================================
diff --git a/doc/sphinx/users-guide/vcl-built-in-subs.rst b/doc/sphinx/users-guide/vcl-built-in-subs.rst
index 14a740d..68c28ae 100644
--- a/doc/sphinx/users-guide/vcl-built-in-subs.rst
+++ b/doc/sphinx/users-guide/vcl-built-in-subs.rst
@@ -59,6 +59,10 @@ common return keywords
     parameter, control is passed to :ref:`vcl_synth` as for
     ``return(synth(503, "Too many restarts"))``
 
+    For a restart, all modifications to ``req`` attributes are
+    preserved except for ``req.restarts`` and ``req.xid``, which need
+    to change by design.
+
 -----------
 client side
 -----------
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index bfd4251..a5d0bd1 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -330,8 +330,6 @@ sp_variables = [
 		or the director otherwise.
 		When used in string context, returns the name of the director
 		or backend, respectively.
-		Note: backend_hint gets reset to the default backend by
-		restarts!
 		"""
 	),
 	('req.hash_ignore_busy',


More information about the varnish-commit mailing list