condfetch: Test retrying a bereq on a dying stale object

The built-in vcl_refresh_valid returns a 503 error if the stale object used for
revalidation gets invalidated while the backend fetch is in progress.

One alternative is to not dismiss the stale object, but that can lead to
invalidated objects being re-instantiated through concurrent refreshes (#4082).

This test case covers the other alternative, which is to retry the fetch. We
could use

       sub vcl_refresh_valid {
               if (!obj_stale.is_valid) {
                       # core code removes conditional headers INM/IMS
                       return (retry);
               }
       }

but more realisticly, real world VCL will retry in v_b_e, so we take that route
and keep the built-in vcl_refresh_valid {}

The plain retry works because core code detects the invalidated stale object and
removes the conditional headers.

Committer edit: Removed code change from original commit, edited test case and,
rewrote commit message. See
#4400 (comment)

Author: dridi

bin/varnishtest/tests/c00129.vtc

@@ -0,0 +1,68 @@
+varnishtest "304 on invalidated stale object"
+
+barrier b1 cond 2
+barrier b2 cond 2
+
+server s1 {
+	rxreq
+	txresp -hdr {ETag: "foo"} -body corrupted
+
+	rxreq
+	expect req.http.If-None-Match == {"foo"}
+	barrier b1 sync
+	barrier b2 sync
+	txresp -status 304 -hdr {ETag: "foo"}
+
+	close
+	accept
+
+	rxreq
+	expect req.http.If-None-Match == <undef>
+	txresp -hdr {ETag: "foo"} -body valid
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		if (req.method == "PURGE") {
+			return (purge);
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.ttl = 1ms;
+		set beresp.grace = 0s;
+		set beresp.keep = 10s;
+	}
+	# rather than modifying vcl_refresh_valid, we retry in v_b_e, which is
+	# more generic and likely to be found in real-world configs.
+	sub vcl_backend_error {
+		if (bereq.retries == 0) {
+			return (retry);
+		}
+	}
+	sub vcl_deliver {
+		set resp.http.obj-hits = obj.hits;
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.body == corrupted
+	expect resp.http.obj-hits == 0
+
+	delay 0.1
+
+	txreq
+	rxresp
+	expect resp.body == valid
+	expect resp.http.obj-hits == 0
+} -start
+
+barrier b1 sync
+client c2 {
+	txreq -req PURGE
+	rxresp
+} -run
+barrier b2 sync
+
+client c1 -wait