Skip to content

Commit

Permalink
Don't report send timeouts as REM_CLOSE errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Dridi committed Jan 13, 2020
1 parent f82b5f1 commit c186423
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 19 deletions.
4 changes: 2 additions & 2 deletions bin/varnishd/http1/cache_http1.h
Expand Up @@ -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);
10 changes: 6 additions & 4 deletions bin/varnishd/http1/cache_http1_deliver.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand All @@ -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);
}
11 changes: 7 additions & 4 deletions bin/varnishd/http1/cache_http1_fetch.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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));
Expand Down
20 changes: 12 additions & 8 deletions bin/varnishd/http1/cache_http1_line.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion bin/varnishtest/tests/s00010.vtc
Expand Up @@ -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 {
Expand Down

0 comments on commit c186423

Please sign in to comment.