(supersedes "Finish the director...")

- close the backend connection if required
- finish the director
- reset bo attributes
- reset VFPs
- signal bo readers that we had a failure if required

Fixes #1638
Fixes #1641

>From a222f13fc1977f19effe52086ca2bc30dfa2dbd1 Mon Sep 17 00:00:00 2001
From: Nils Goroll <[email protected]>
Date: Mon, 1 Dec 2014 10:15:30 +0100
Subject: [PATCH] Need to do more work when retrying

- close the backend connection if required
- finish the director
- reset bo attributes
- reset VFPs
- signal bo readers that we had a failure if required

Fixes #1638
Fixes #1641
---
 bin/varnishd/cache/cache_fetch.c | 44 ++++++++++++++++++++++++++++++++----
 bin/varnishtest/tests/r01638.vtc | 49 ++++++++++++++++++++++++++++++++++++++++
 bin/varnishtest/tests/r01641.vtc | 49 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 4 deletions(-)
 create mode 100644 bin/varnishtest/tests/r01638.vtc
 create mode 100644 bin/varnishtest/tests/r01641.vtc

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index c44992b..4a6084b 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -213,17 +213,46 @@ vbf_stp_mkbereq(const struct worker *wrk, struct busyobj *bo)
 
 /*--------------------------------------------------------------------
  * Start a new VSL transaction and try again
+ * Prepare the busyobj and fetch processors
  */
 
 static enum fetch_step
 vbf_stp_retry(struct worker *wrk, struct busyobj *bo)
 {
+	struct vfp_ctx *vfc;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+	vfc = bo->vfc;
+	CHECK_OBJ_NOTNULL(vfc, VFP_CTX_MAGIC);
 
 	VSLb_ts_busyobj(bo, "Retry", W_TIM_real(wrk));
 
+	/* VDI_Finish must have been called before */
+	assert(bo->director_state == DIR_S_NULL);
+	bo->doclose = SC_NULL;
+
+	/* reset other bo attributes - See VBO_GetBusyObj */
+	bo->storage_hint = NULL;
+	bo->do_esi = 0;
+	bo->do_stream = 1;
+
+	/* reset fetch processors */
+	vfc->failed = 0;
+	VFP_Close(vfc);
+	VFP_Setup(vfc);
+
+	/*
+	 * reset bo state, transition through failed in case anyone has started
+	 * streaming the body
+	 */
+	if (bo->state > BOS_REQ_DONE) {
+		if (bo->state < BOS_FAILED)
+			VBO_setstate(bo, BOS_FAILED);
+		bo->state = BOS_INVALID;
+		VBO_setstate(bo, BOS_REQ_DONE);
+	}
+
 	// XXX: BereqEnd + BereqAcct ?
 	VSL_ChgId(bo->vsl, "bereq", "retry", VXID_Get(wrk, VSL_BACKENDMARKER));
 	VSLb_ts_busyobj(bo, "Start", bo->t_prev);
@@ -405,11 +434,12 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 
 	if (wrk->handling == VCL_RET_RETRY) {
 		bo->doclose = SC_RESP_CLOSE;
-		VDI_Finish(bo->director_resp, bo->wrk, bo);
-		bo->doclose = SC_NULL;
-		bo->retries++;
-		if (bo->retries <= cache_param->max_retries)
+		if (bo->director_state != DIR_S_NULL)
+			VDI_Finish(bo->director_resp, bo->wrk, bo);
+
+		if (bo->retries++ < cache_param->max_retries)
 			return (F_STP_RETRY);
+
 		VSLb(bo->vsl, SLT_VCL_Error,
 		    "Too many retries, delivering 503");
 		assert(bo->director_state == DIR_S_NULL);
@@ -768,8 +798,14 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	if (wrk->handling == VCL_RET_RETRY) {
 		VSB_delete(bo->synth_body);
 		bo->synth_body = NULL;
+
+		bo->doclose = SC_RESP_CLOSE;
+		if (bo->director_state != DIR_S_NULL)
+			VDI_Finish(bo->director_resp, bo->wrk, bo);
+
 		if (bo->retries++ < cache_param->max_retries)
 			return (F_STP_RETRY);
+
 		return (F_STP_FAIL);
 	}
 
diff --git a/bin/varnishtest/tests/r01638.vtc b/bin/varnishtest/tests/r01638.vtc
new file mode 100644
index 0000000..2ddebbb
--- /dev/null
+++ b/bin/varnishtest/tests/r01638.vtc
@@ -0,0 +1,49 @@
+varnishtest "Test retry of straight insufficient bytes pass-fetch : do_stream = false"
+
+server s1 {
+	rxreq
+	txresp -hdr "Content-Length: 10000" -nolen -bodylen 5000
+}
+
+server s2 {
+	rxreq
+	txresp -bodylen 5
+}
+
+server s1 -start
+server s2 -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		return (pass);
+	}
+	sub vcl_backend_fetch {
+		set bereq.between_bytes_timeout = 10s;
+		if (bereq.retries == 0) {
+			set bereq.backend = s1;
+		} else {
+			set bereq.backend = s2;
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.do_stream = false;
+	}
+	sub vcl_backend_error {
+		if (bereq.retries == 0) {
+			return (retry);
+		} else {
+			return (deliver);
+		}
+	}
+} -start
+
+client c1 {
+	timeout 10
+	txreq -url "/"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 5
+} -run
+
+server s1 -wait
+server s2 -wait
diff --git a/bin/varnishtest/tests/r01641.vtc b/bin/varnishtest/tests/r01641.vtc
new file mode 100644
index 0000000..1dddc85
--- /dev/null
+++ b/bin/varnishtest/tests/r01641.vtc
@@ -0,0 +1,49 @@
+varnishtest "Test retry of straight insufficient bytes pass-fetch : do_esi = true"
+
+server s1 {
+	rxreq
+	txresp -hdr "Content-Length: 10000" -nolen -bodylen 5000
+}
+
+server s2 {
+	rxreq
+	txresp -bodylen 5
+}
+
+server s1 -start
+server s2 -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		return (pass);
+	}
+	sub vcl_backend_fetch {
+		set bereq.between_bytes_timeout = 10s;
+		if (bereq.retries == 0) {
+			set bereq.backend = s1;
+		} else {
+			set bereq.backend = s2;
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.do_esi = true;
+	}
+	sub vcl_backend_error {
+		if (bereq.retries == 0) {
+			return (retry);
+		} else {
+			return (deliver);
+		}
+	}
+} -start
+
+client c1 {
+	timeout 10
+	txreq -url "/"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 5
+} -run
+
+server s1 -wait
+server s2 -wait
-- 
2.1.1

_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to