[master] eecd409 Honor first_byte_timeout for recycled backend connections

Dag Haavi Finstad daghf at varnish-software.com
Mon Nov 27 11:51:07 UTC 2017


commit eecd409d13f145765de3aeee4984179e0ae008b5
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Fri Nov 24 16:22:55 2017 +0100

    Honor first_byte_timeout for recycled backend connections
    
    Fixes: #1772

diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index 5f5ebb5..48075d9 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -178,7 +178,8 @@ vbe_dir_finish(const struct director *d, struct worker *wrk,
 	CAST_OBJ_NOTNULL(vtp, bo->htc->priv, VTP_MAGIC);
 	bo->htc->priv = NULL;
 	if (vtp->state != VTP_STATE_USED)
-		assert(bo->htc->doclose == SC_TX_PIPE);
+		assert(bo->htc->doclose == SC_TX_PIPE ||
+			bo->htc->doclose == SC_RX_TIMEOUT);
 	if (bo->htc->doclose != SC_NULL || bp->proxy_header != 0) {
 		VSLb(bo->vsl, SLT_BackendClose, "%d %s", vtp->fd,
 		    bp->director->display_name);
@@ -233,16 +234,24 @@ vbe_dir_gethdrs(const struct director *d, struct worker *wrk,
 
 		i = V1F_SendReq(wrk, bo, &bo->acct.bereq_hdrbytes, 0);
 
-		if (vtp->state != VTP_STATE_USED)
-			VTP_Wait(wrk, vtp);
-
-		assert(vtp->state == VTP_STATE_USED);
+		if (vtp->state != VTP_STATE_USED) {
+			if (VTP_Wait(wrk, vtp, VTIM_real() +
+			    bo->htc->first_byte_timeout) != 0) {
+				bo->htc->doclose = SC_RX_TIMEOUT;
+				VSLb(bo->vsl, SLT_FetchError,
+				     "Timed out reusing backend connection");
+				extrachance = 0;
+			}
+		}
 
-		if (i == 0)
-			i = V1F_FetchRespHdr(bo);
-		if (i == 0) {
-			AN(bo->htc->priv);
-			return (0);
+		if (bo->htc->doclose == SC_NULL) {
+			assert(vtp->state == VTP_STATE_USED);
+			if (i == 0)
+				i = V1F_FetchRespHdr(bo);
+			if (i == 0) {
+				AN(bo->htc->priv);
+				return (0);
+			}
 		}
 
 		/*
@@ -252,7 +261,7 @@ vbe_dir_gethdrs(const struct director *d, struct worker *wrk,
 		 */
 		vbe_dir_finish(d, wrk, bo);
 		AZ(bo->htc);
-		if (i < 0)
+		if (i < 0 || extrachance == 0)
 			break;
 		if (bo->req != NULL &&
 		    bo->req->req_body_status != REQ_BODY_NONE &&
diff --git a/bin/varnishd/cache/cache_tcp_pool.c b/bin/varnishd/cache/cache_tcp_pool.c
index 855eb0c..18adad3 100644
--- a/bin/varnishd/cache/cache_tcp_pool.c
+++ b/bin/varnishd/cache/cache_tcp_pool.c
@@ -31,6 +31,7 @@
 
 #include "config.h"
 
+#include <errno.h>
 #include <stdlib.h>
 
 #include "cache_varnishd.h"
@@ -416,10 +417,11 @@ VTP_Get(struct tcp_pool *tp, double tmo, struct worker *wrk,
 /*--------------------------------------------------------------------
  */
 
-void
-VTP_Wait(struct worker *wrk, struct vtp *vtp)
+int
+VTP_Wait(struct worker *wrk, struct vtp *vtp, double tmo)
 {
 	struct tcp_pool *tp;
+	int r;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(vtp, VTP_MAGIC);
@@ -427,11 +429,21 @@ VTP_Wait(struct worker *wrk, struct vtp *vtp)
 	CHECK_OBJ_NOTNULL(tp, TCP_POOL_MAGIC);
 	assert(vtp->cond == &wrk->cond);
 	Lck_Lock(&tp->mtx);
-	while (vtp->state == VTP_STATE_STOLEN)
-		AZ(Lck_CondWait(&wrk->cond, &tp->mtx, 0));
+	while (vtp->state == VTP_STATE_STOLEN) {
+		r = Lck_CondWait(&wrk->cond, &tp->mtx, tmo);
+		if (r != 0) {
+			if (r == EINTR)
+				continue;
+			assert(r == ETIMEDOUT);
+			Lck_Unlock(&tp->mtx);
+			return (1);
+		}
+	}
 	assert(vtp->state == VTP_STATE_USED);
 	vtp->cond = NULL;
 	Lck_Unlock(&tp->mtx);
+
+	return (0);
 }
 
 /*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/cache/cache_tcp_pool.h b/bin/varnishd/cache/cache_tcp_pool.h
index 4dbc12f..d9d3c54 100644
--- a/bin/varnishd/cache/cache_tcp_pool.h
+++ b/bin/varnishd/cache/cache_tcp_pool.h
@@ -91,7 +91,7 @@ struct vtp *VTP_Get(struct tcp_pool *, double tmo, struct worker *,
 	 * Get a (possibly) recycled connection.
 	 */
 
-void VTP_Wait(struct worker *, struct vtp *);
+int VTP_Wait(struct worker *, struct vtp *, double tmo);
 	/*
 	 * If the connection was recycled (state != VTP_STATE_USED) call this
 	 * function before attempting to receive on the connection.
diff --git a/bin/varnishtest/tests/r01772.vtc b/bin/varnishtest/tests/r01772.vtc
new file mode 100644
index 0000000..93c5e1b
--- /dev/null
+++ b/bin/varnishtest/tests/r01772.vtc
@@ -0,0 +1,24 @@
+varnishtest "#1772: Honor first_byte_timeout on a recycled connection"
+
+server s1 {
+	rxreq
+	expect req.url == "/first"
+	txresp
+
+	rxreq
+	expect req.url == "/second"
+	delay 2
+	txresp
+} -start
+
+varnish v1 -arg "-p first_byte_timeout=1" -vcl+backend {} -start
+
+client c1 {
+	txreq -url "/first"
+	rxresp
+	expect resp.status == 200
+
+	txreq -url "/second"
+	rxresp
+	expect resp.status == 503
+} -run


More information about the varnish-commit mailing list