[master] c186423b5 Don't report send timeouts as REM_CLOSE errors

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Jan 13 17:49:05 UTC 2020


commit c186423b5edde99d7fc487cd4b785a7ee73dfdd0
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Fri Jan 3 14:58:46 2020 +0100

    Don't report send timeouts as REM_CLOSE errors
    
    V1L_Close() is now in charge of returning a specific enum sess_close
    instead of an error flag where SC_NULL implies that everything went
    well.
    
    This otherwise maintains the status quo regarding prior handling of
    HTTP/1 write errors. The calling code falls back to what it used to
    default to when an error occurred somewhere else.

diff --git a/bin/varnishd/http1/cache_http1.h b/bin/varnishd/http1/cache_http1.h
index 47c9fce5c..9d83a782a 100644
--- a/bin/varnishd/http1/cache_http1.h
+++ b/bin/varnishd/http1/cache_http1.h
@@ -60,6 +60,6 @@ void V1L_Chunked(const struct worker *w);
 void V1L_EndChunk(const struct worker *w);
 void V1L_Open(struct worker *, struct ws *, int *fd, struct vsl_log *,
     vtim_real deadline, unsigned niov);
-unsigned V1L_Flush(const struct worker *w);
-unsigned V1L_Close(struct worker *w, uint64_t *cnt);
+enum sess_close V1L_Flush(const struct worker *w);
+enum sess_close V1L_Close(struct worker *w, uint64_t *cnt);
 size_t V1L_Write(const struct worker *w, const void *ptr, ssize_t len);
diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c
index ff21ede7f..f7e7b270d 100644
--- a/bin/varnishd/http1/cache_http1_deliver.c
+++ b/bin/varnishd/http1/cache_http1_deliver.c
@@ -85,7 +85,7 @@ void v_matchproto_(vtr_deliver_f)
 V1D_Deliver(struct req *req, struct boc *boc, int sendbody)
 {
 	int err = 0, chunked = 0;
-	unsigned u;
+	enum sess_close sc;
 	uint64_t hdrbytes, bytes;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -152,7 +152,7 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody)
 			V1L_EndChunk(req->wrk);
 	}
 
-	u = V1L_Close(req->wrk, &bytes);
+	sc = V1L_Close(req->wrk, &bytes);
 	AZ(req->wrk->v1l);
 
 	/* Bytes accounting */
@@ -163,7 +163,9 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody)
 		req->acct.resp_bodybytes += bytes - hdrbytes;
 	}
 
-	if ((u || err) && req->sp->fd >= 0)
-		Req_Fail(req, SC_REM_CLOSE);
+	if (sc == SC_NULL && err && req->sp->fd >= 0)
+		sc = SC_REM_CLOSE;
+	if (sc != SC_NULL)
+		Req_Fail(req, sc);
 	VDP_close(req);
 }
diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c
index 12d5e7c2b..b2882ab3a 100644
--- a/bin/varnishd/http1/cache_http1_fetch.c
+++ b/bin/varnishd/http1/cache_http1_fetch.c
@@ -72,7 +72,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
     uint64_t *ctr_bodybytes, int onlycached)
 {
 	struct http *hp;
-	int j;
+	enum sess_close sc;
 	ssize_t i;
 	uint64_t bytes, hdrbytes;
 	struct http_conn *htc;
@@ -130,7 +130,7 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
 			V1L_EndChunk(wrk);
 	}
 
-	j = V1L_Close(wrk, &bytes);
+	sc = V1L_Close(wrk, &bytes);
 
 	/* Bytes accounting */
 	if (bytes < hdrbytes)
@@ -140,11 +140,14 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes,
 		*ctr_bodybytes += bytes - hdrbytes;
 	}
 
-	if (j != 0 || i < 0) {
+	if (sc == SC_NULL && i < 0)
+		sc = SC_TX_ERROR;
+
+	if (sc != SC_NULL) {
 		VSLb(bo->vsl, SLT_FetchError, "backend write error: %d (%s)",
 		    errno, vstrerror(errno));
 		VSLb_ts_busyobj(bo, "Bereq", W_TIM_real(wrk));
-		htc->doclose = SC_TX_ERROR;
+		htc->doclose = sc;
 		return (-1);
 	}
 	VSLb_ts_busyobj(bo, "Bereq", W_TIM_real(wrk));
diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index 0731b4a75..5ea8335e0 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -53,7 +53,7 @@ struct v1l {
 	unsigned		magic;
 #define V1L_MAGIC		0x2f2142e5
 	int			*wfd;
-	unsigned		werr;	/* valid after V1L_Flush() */
+	enum sess_close		werr;	/* valid after V1L_Flush() */
 	struct iovec		*iov;
 	unsigned		siov;
 	unsigned		niov;
@@ -122,15 +122,15 @@ V1L_Open(struct worker *wrk, struct ws *ws, int *fd, struct vsl_log *vsl,
 		WS_Release(ws, u * sizeof(struct iovec));
 }
 
-unsigned
+enum sess_close
 V1L_Close(struct worker *wrk, uint64_t *cnt)
 {
 	struct v1l *v1l;
-	unsigned u;
+	enum sess_close sc;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	AN(cnt);
-	u = V1L_Flush(wrk);
+	sc = V1L_Flush(wrk);
 	v1l = wrk->v1l;
 	wrk->v1l = NULL;
 	CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC);
@@ -139,7 +139,7 @@ V1L_Close(struct worker *wrk, uint64_t *cnt)
 		WS_Release(v1l->ws, 0);
 	WS_Reset(v1l->ws, v1l->res);
 	ZERO_OBJ(v1l, sizeof *v1l);
-	return (u);
+	return (sc);
 }
 
 static void
@@ -166,7 +166,7 @@ v1l_prune(struct v1l *v1l, size_t bytes)
 	AZ(v1l->liov);
 }
 
-unsigned
+enum sess_close
 V1L_Flush(const struct worker *wrk)
 {
 	ssize_t i;
@@ -180,7 +180,7 @@ V1L_Flush(const struct worker *wrk)
 
 	assert(v1l->niov <= v1l->siov);
 
-	if (*v1l->wfd >= 0 && v1l->liov > 0 && v1l->werr == 0) {
+	if (*v1l->wfd >= 0 && v1l->liov > 0 && v1l->werr == SC_NULL) {
 		if (v1l->ciov < v1l->siov && v1l->cliov > 0) {
 			/* Add chunk head & tail */
 			bprintf(cbuf, "00%zx\r\n", v1l->cliov);
@@ -230,10 +230,14 @@ V1L_Flush(const struct worker *wrk)
 				v1l->cnt += i;
 		}
 		if (i <= 0) {
-			v1l->werr++;
 			VSLb(v1l->vsl, SLT_Debug,
 			    "Write error, retval = %zd, len = %zd, errno = %s",
 			    i, v1l->liov, vstrerror(errno));
+			AZ(v1l->werr);
+			if (errno == EPIPE)
+				v1l->werr = SC_REM_CLOSE;
+			else
+				v1l->werr = SC_TX_ERROR;
 		}
 	}
 	v1l->liov = 0;
diff --git a/bin/varnishtest/tests/s00010.vtc b/bin/varnishtest/tests/s00010.vtc
index b591c0b6c..391761967 100644
--- a/bin/varnishtest/tests/s00010.vtc
+++ b/bin/varnishtest/tests/s00010.vtc
@@ -33,7 +33,8 @@ varnish v1 -vcl+backend {
 } -start
 
 logexpect l1 -v v1 -g raw {
-	expect * 1001 Debug "Hit total send timeout"
+	expect * 1001 Debug	"Hit total send timeout"
+	expect * 1000 SessClose	TX_ERROR
 } -start
 
 client c1 -rcvbuf 256 {


More information about the varnish-commit mailing list