[master] d124132 Synthesize a 503 when we fail to fetch, and pass it on.
Poul-Henning Kamp
phk at varnish-cache.org
Sun Jun 16 15:46:06 CEST 2013
commit d1241323f95e45a30d93b7dde13bcb4d2e5d1246
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Sun Jun 16 13:42:31 2013 +0000
Synthesize a 503 when we fail to fetch, and pass it on.
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index b65c60a..4ef7991 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -486,7 +486,8 @@ vbf_fetch_body(struct worker *wrk, void *priv)
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CAST_OBJ_NOTNULL(bo, priv, BUSYOBJ_MAGIC);
- CHECK_OBJ_NOTNULL(bo->vbc, VBC_MAGIC);
+ htc = &bo->htc;
+ CHECK_OBJ_ORNULL(bo->vbc, VBC_MAGIC);
obj = bo->fetch_obj;
CHECK_OBJ_NOTNULL(obj, OBJECT_MAGIC);
CHECK_OBJ_NOTNULL(obj->http, HTTP_MAGIC);
@@ -500,7 +501,6 @@ vbf_fetch_body(struct worker *wrk, void *priv)
AZ(bo->stats);
bo->stats = &wrk->stats;
- htc = &bo->htc;
if (bo->vfp == NULL)
bo->vfp = &vfp_nop;
@@ -513,7 +513,7 @@ vbf_fetch_body(struct worker *wrk, void *priv)
/* XXX: pick up estimate from objdr ? */
cl = 0;
- cls = 0;
+ cls = bo->should_close;
switch (htc->body_status) {
case BS_NONE:
mklen = 0;
@@ -526,7 +526,7 @@ vbf_fetch_body(struct worker *wrk, void *priv)
bo->vfp->begin(bo, cl);
if (bo->state == BOS_FETCHING && cl > 0)
- cls = vbf_fetch_straight(bo, htc, cl);
+ cls |= vbf_fetch_straight(bo, htc, cl);
mklen = 1;
if (bo->vfp->end(bo))
assert(bo->state == BOS_FAILED);
@@ -534,7 +534,7 @@ vbf_fetch_body(struct worker *wrk, void *priv)
case BS_CHUNKED:
bo->vfp->begin(bo, cl > 0 ? cl : 0);
if (bo->state == BOS_FETCHING)
- cls = vbf_fetch_chunked(bo, htc);
+ cls |= vbf_fetch_chunked(bo, htc);
mklen = 1;
if (bo->vfp->end(bo))
assert(bo->state == BOS_FAILED);
@@ -549,11 +549,10 @@ vbf_fetch_body(struct worker *wrk, void *priv)
assert(bo->state == BOS_FAILED);
break;
case BS_ERROR:
- cls = VBF_Error(bo, "error incompatible Transfer-Encoding");
+ cls |= VBF_Error(bo, "error incompatible Transfer-Encoding");
mklen = 0;
break;
default:
- cls = 0;
mklen = 0;
INCOMPL();
}
@@ -574,18 +573,21 @@ vbf_fetch_body(struct worker *wrk, void *priv)
http_Teardown(bo->bereq);
http_Teardown(bo->beresp);
+ if (bo->vbc != NULL) {
+ if (cls)
+ VDI_CloseFd(&bo->vbc);
+ else
+ VDI_RecycleFd(&bo->vbc);
+ }
+
if (bo->state == BOS_FAILED) {
wrk->stats.fetch_failed++;
- VDI_CloseFd(&bo->vbc);
obj->len = 0;
EXP_Clr(&obj->exp);
EXP_Rearm(obj);
} else {
assert(bo->state == BOS_FETCHING);
- if (cls == 0 && bo->should_close)
- cls = 1;
-
VSLb(bo->vsl, SLT_Length, "%zd", obj->len);
{
@@ -609,12 +611,6 @@ vbf_fetch_body(struct worker *wrk, void *priv)
"Content-Length: %zd", obj->len);
}
- if (cls)
- VDI_CloseFd(&bo->vbc);
- else
- VDI_RecycleFd(&bo->vbc);
-
-
/* XXX: Atomic assignment, needs volatile/membar ? */
bo->state = BOS_FINISHED;
}
@@ -627,15 +623,14 @@ vbf_fetch_body(struct worker *wrk, void *priv)
* Copy req->bereq and run it by VCL::vcl_backend_fetch{}
*/
-static void
-vbf_make_bereq(struct worker *wrk, const struct req *req, struct busyobj *bo)
+static enum fetch_step
+vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo, const struct req *req)
{
int i;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
- CHECK_OBJ_NOTNULL(bo->fetch_objcore, OBJCORE_MAGIC);
AN(bo->director);
AZ(bo->vbc);
@@ -666,18 +661,54 @@ vbf_make_bereq(struct worker *wrk, const struct req *req, struct busyobj *bo)
http_PrintfHeader(bo->bereq,
"X-Varnish: %u", bo->vsl->wid & VSL_IDENTMASK);
+ return (F_STP_FETCHHDR);
}
-
/*--------------------------------------------------------------------
*/
-static void
-vbf_proc_resp(struct worker *wrk, struct busyobj *bo)
+static enum fetch_step
+vbf_stp_fetchhdr(struct worker *wrk, struct busyobj *bo, struct req **reqp)
{
int i;
+ CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+ AN(reqp);
+ CHECK_OBJ_NOTNULL((*reqp), REQ_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+ xxxassert (wrk->handling == VCL_RET_FETCH);
+
+ HTTP_Setup(bo->beresp, bo->ws, bo->vsl, HTTP_Beresp);
+
+ if (!bo->do_pass)
+ *reqp = NULL;
+
+ i = vbf_fetch_hdr(wrk, bo, *reqp);
+ /*
+ * If we recycle a backend connection, there is a finite chance
+ * that the backend closed it before we get a request to it.
+ * Do a single retry in that case.
+ */
+ if (i == 1) {
+ VSC_C_main->backend_retry++;
+ i = vbf_fetch_hdr(wrk, bo, *reqp);
+ }
+
+ if (bo->do_pass)
+ *reqp = NULL;
+
+ if (i) {
+ AZ(bo->vbc);
+ bo->err_code = 503;
+ http_SetH(bo->beresp, HTTP_HDR_PROTO, "HTTP/1.1");
+ http_SetResp(bo->beresp,
+ "HTTP/1.1", 503, "Backend fetch failed");
+ http_SetHeader(bo->beresp, "Content-Length: 0");
+ http_SetHeader(bo->beresp, "Connection: close");
+ } else {
+ AN(bo->vbc);
+ }
+
/*
* These two headers can be spread over multiple actual headers
* and we rely on their content outside of VCL, so collect them
@@ -727,6 +758,7 @@ vbf_proc_resp(struct worker *wrk, struct busyobj *bo)
* no Content-Encoding --> object is not gzip'ed.
* anything else --> do nothing wrt gzip
*
+ * XXX: BS_NONE/cl==0 should avoid gzip/gunzip
*/
/* We do nothing unless the param is set */
@@ -769,17 +801,18 @@ vbf_proc_resp(struct worker *wrk, struct busyobj *bo)
else if (bo->is_gzip)
bo->vfp = &vfp_testgzip;
+ if (wrk->handling != VCL_RET_DELIVER)
+ return (F_STP_NOTYET);
+ return (F_STP_FETCH);
}
/*--------------------------------------------------------------------
*/
static enum fetch_step
-vbf_stp_fetch(struct worker *wrk, struct busyobj *bo, struct req **reqp)
+vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
{
- int i;
struct http *hp, *hp2;
- struct req *req;
char *b;
uint16_t nhttp;
unsigned l;
@@ -789,48 +822,10 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo, struct req **reqp)
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
- AN(reqp);
- req = *reqp;
- CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
- vbf_make_bereq(wrk, req, bo);
- xxxassert (wrk->handling == VCL_RET_FETCH);
-
- HTTP_Setup(bo->beresp, bo->ws, bo->vsl, HTTP_Beresp);
-
- if (!bo->do_pass) {
- AN(req);
- req = NULL;
- *reqp = NULL;
- }
-
- i = vbf_fetch_hdr(wrk, bo, req);
- /*
- * If we recycle a backend connection, there is a finite chance
- * that the backend closed it before we get a request to it.
- * Do a single retry in that case.
- */
- if (i == 1) {
- VSC_C_main->backend_retry++;
- i = vbf_fetch_hdr(wrk, bo, req);
- }
-
- if (bo->do_pass) {
- AN(req);
- req = NULL;
- *reqp = NULL;
- }
- AZ(req);
-
- if (i) {
- wrk->handling = VCL_RET_ERROR;
- bo->err_code = 503;
- } else {
- vbf_proc_resp(wrk, bo);
- if (wrk->handling != VCL_RET_DELIVER)
- VDI_CloseFd(&bo->vbc);
- }
+ if (wrk->handling != VCL_RET_DELIVER)
+ VDI_CloseFd(&bo->vbc);
if (wrk->handling != VCL_RET_DELIVER) {
/* Clean up partial fetch */
@@ -1003,6 +998,12 @@ vbf_stp_abandon(struct worker *wrk, struct busyobj *bo)
*/
static enum fetch_step
+vbf_stp_notyet(void)
+{
+ WRONG("Patience, grashopper, patience...");
+}
+
+static enum fetch_step
vbf_stp_done(void)
{
WRONG("Just plain wrong");
@@ -1033,7 +1034,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
bo = vsh->bo;
THR_SetBusyobj(bo);
- bo->step = F_STP_FETCH;
+ bo->step = F_STP_MKBEREQ;
while (bo->step != F_STP_DONE) {
switch(bo->step) {
diff --git a/bin/varnishtest/tests/b00015.vtc b/bin/varnishtest/tests/b00015.vtc
index ff39f42..3626284 100644
--- a/bin/varnishtest/tests/b00015.vtc
+++ b/bin/varnishtest/tests/b00015.vtc
@@ -15,36 +15,51 @@ client c1 {
expect resp.http.X-varnish == "1001"
} -run
+delay .1
+
client c1 {
txreq -url "/"
rxresp
expect resp.status == 503
- expect resp.http.X-varnish == "1005"
+ expect resp.http.X-varnish == "1004"
} -run
-# Then check that an cacheable error from the backend is
+delay .1
+
+# Then check that a cacheable error from the backend is
+
+varnish v1 -cliok "ban req.url ~ .*"
server s1 {
rxreq
txresp -status 302
} -start
-varnish v1 -vcl+backend { }
+varnish v1 -vcl+backend {
+ sub vcl_backend_response {
+ set beresp.http.ttl = beresp.ttl;
+ set beresp.http.uncacheable = beresp.uncacheable;
+ }
+}
client c1 {
txreq -url "/"
rxresp
expect resp.status == 302
- expect resp.http.X-varnish == "1009"
+ expect resp.http.X-varnish == "1007"
} -run
+delay .1
+
client c1 {
txreq -url "/"
rxresp
expect resp.status == 302
- expect resp.http.X-varnish == "1012 1010"
+ expect resp.http.X-varnish == "1010 1008"
} -run
+delay .1
+
# Then check that a non-cacheable error from the backend can be
server s1 {
@@ -64,12 +79,16 @@ client c1 {
txreq -url "/2"
rxresp
expect resp.status == 502
- expect resp.http.X-varnish == "1014"
+ expect resp.http.X-varnish == "1012"
} -run
+delay .1
+
client c1 {
txreq -url "/2"
rxresp
expect resp.status == 502
- expect resp.http.X-varnish == "1017 1015"
+ expect resp.http.X-varnish == "1015 1013"
} -run
+
+delay .1
diff --git a/bin/varnishtest/tests/c00024.vtc b/bin/varnishtest/tests/c00024.vtc
index 4b1bdac..31ebc8b 100644
--- a/bin/varnishtest/tests/c00024.vtc
+++ b/bin/varnishtest/tests/c00024.vtc
@@ -5,21 +5,10 @@ server s1 {
txresp
} -start
-server s2 {
-} -start
-
-varnish v1 -vcl {
- backend bad {
- .host = "${s2_addr}";
- .port = "${s2_port}";
- }
- backend good {
- .host = "${s1_addr}";
- .port = "${s1_port}";
- }
+varnish v1 -vcl+backend {
sub vcl_recv {
- if (req.restarts > 0) {
- set req.backend = good;
+ if (req.restarts == 0) {
+ return (error(701, "FOO"));
}
}
sub vcl_error {
diff --git a/include/tbl/steps.h b/include/tbl/steps.h
index 32af092..5f9b3b2 100644
--- a/include/tbl/steps.h
+++ b/include/tbl/steps.h
@@ -50,8 +50,11 @@ REQ_STEP(error, ERROR, (wrk, req))
#endif
#ifdef FETCH_STEP
-FETCH_STEP(fetch, FETCH, (wrk, bo, reqp))
+FETCH_STEP(mkbereq, MKBEREQ, (wrk, bo, *reqp))
+FETCH_STEP(fetchhdr, FETCHHDR, (wrk, bo, reqp))
+FETCH_STEP(fetch, FETCH, (wrk, bo))
FETCH_STEP(abandon, ABANDON, (wrk, bo))
+FETCH_STEP(notyet, NOTYET, ())
FETCH_STEP(done, DONE, ())
#endif
More information about the varnish-commit
mailing list