[4.0] 25b38f9 Add the internal bo->failed state we need to allow retries separate from the external BOS_FAILED state.
Poul-Henning Kamp
phk at FreeBSD.org
Thu Mar 13 10:24:27 CET 2014
commit 25b38f92702e74d0014c36033b24755c79dddada
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Tue Feb 25 10:29:24 2014 +0000
Add the internal bo->failed state we need to allow retries separate
from the external BOS_FAILED state.
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 5db6e22..7c6079e 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -500,6 +500,9 @@ oc_getlru(const struct objcore *oc)
* streaming delivery will make use of.
*/
+/*
+ * The macro-states we expose outside the fetch code
+ */
enum busyobj_state_e {
BOS_INVALID = 0, /* don't touch (yet) */
BOS_REQ_DONE, /* beresp.* can be examined */
@@ -514,7 +517,6 @@ struct busyobj {
struct lock mtx;
pthread_cond_t cond;
char *end;
- enum fetch_step step;
/*
* All fields from refcount and down are zeroed when the busyobj
@@ -532,6 +534,7 @@ struct busyobj {
intptr_t vfps_priv[N_VFPS];
int vfp_nxt;
+ int failed;
enum busyobj_state_e state;
struct ws ws[1];
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index b64b2bb..0c621df 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -494,7 +494,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
if (vbf_beresp2obj(bo)) {
(void)VFP_Error(bo, "Could not get storage");
VDI_CloseFd(&bo->vbc);
- return (F_STP_DONE);
+ return (F_STP_ERROR);
}
assert(WRW_IsReleased(wrk));
@@ -526,7 +526,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
VSLb(bo->vsl, SLT_Fetch_Body, "%u(%s)",
bo->htc.body_status, body_status_2str(bo->htc.body_status));
- if (bo->state == BOS_FAILED) {
+ if (bo->failed) {
wrk->stats.fetch_failed++;
} else {
if (bo->do_stream)
@@ -553,16 +553,29 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
}
}
- if (!bo->do_stream && bo->state != BOS_FAILED)
- HSH_Unbusy(&wrk->stats, obj->objcore);
+ if (!bo->do_stream) {
+ if (bo->failed) {
+ if (bo->fetch_obj != NULL) {
+ oc_freeobj(bo->fetch_objcore);
+ bo->fetch_obj = NULL;
+ bo->stats->n_object--;
+ }
+ return (F_STP_ERROR);
+ } else {
+ HSH_Unbusy(&wrk->stats, obj->objcore);
+ VBO_setstate(bo, BOS_FINISHED);
+ }
+ } else if (bo->failed) {
+ HSH_Fail(bo->fetch_objcore);
+ VBO_setstate(bo, BOS_FAILED);
+ } else {
+ VBO_setstate(bo, BOS_FINISHED);
+ }
HSH_Complete(obj->objcore);
assert(bo->refcount >= 1);
- if (bo->state != BOS_FAILED)
- VBO_setstate(bo, BOS_FINISHED);
-
return (F_STP_DONE);
}
@@ -650,14 +663,16 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
if (st->len == st->space)
st = NULL;
}
- } while (bo->state < BOS_FAILED &&
- (ois == OIS_DATA || ois == OIS_STREAM));
+ } while (!bo->failed && (ois == OIS_DATA || ois == OIS_STREAM));
ObjIterEnd(&oi);
- if (bo->state != BOS_FAILED) {
+ if (!bo->failed) {
assert(al == bo->ims_obj->len);
assert(obj->len == al);
VBO_setstate(bo, BOS_FINISHED);
EXP_Rearm(bo->ims_obj, bo->ims_obj->exp.t_origin, 0, 0, 0);
+ } else {
+ HSH_Fail(bo->fetch_objcore);
+ VBO_setstate(bo, BOS_FAILED);
}
HSH_Complete(obj->objcore);
return (F_STP_DONE);
@@ -673,7 +688,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
- AN(bo->fetch_objcore->flags &= OC_F_BUSY);
+ AN(bo->fetch_objcore->flags & OC_F_BUSY);
// XXX: reset all beresp flags ?
@@ -697,7 +712,9 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
http_PrintfHeader(bo->beresp, "X-XXXPHK: yes");
if (vbf_beresp2obj(bo)) {
- INCOMPL();
+ VBO_setstate(bo, BOS_FAILED);
+ HSH_Fail(bo->fetch_objcore);
+ return (F_STP_DONE);
}
HSH_Unbusy(&wrk->stats, bo->fetch_obj->objcore);
@@ -739,7 +756,6 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
while (stp != F_STP_DONE) {
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
- bo->step = stp;
switch(stp) {
#define FETCH_STEP(l, U, arg) \
case F_STP_##U: \
diff --git a/bin/varnishd/cache/cache_fetch_proc.c b/bin/varnishd/cache/cache_fetch_proc.c
index b21ca15..8861e1d 100644
--- a/bin/varnishd/cache/cache_fetch_proc.c
+++ b/bin/varnishd/cache/cache_fetch_proc.c
@@ -62,12 +62,11 @@ VFP_Error(struct busyobj *bo, const char *fmt, ...)
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
assert(bo->state >= BOS_REQ_DONE);
- if (bo->state < BOS_FAILED) {
+ if (!bo->failed) {
va_start(ap, fmt);
VSLbv(bo->vsl, SLT_FetchError, fmt, ap);
va_end(ap);
- HSH_Fail(bo->fetch_objcore);
- VBO_setstate(bo, BOS_FAILED);
+ bo->failed = 1;
}
return (VFP_ERROR);
}
@@ -214,7 +213,7 @@ VFP_Fetch_Body(struct busyobj *bo, ssize_t est)
bo->should_close = 1;
break;
}
- assert(bo->state != BOS_FAILED);
+ AZ(bo->failed);
if (st == NULL) {
st = VFP_GetStorage(bo, est);
est = 0;
@@ -228,7 +227,7 @@ VFP_Fetch_Body(struct busyobj *bo, ssize_t est)
CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC);
assert(st == VTAILQ_LAST(&bo->fetch_obj->store, storagehead));
l = st->space - st->len;
- assert(bo->state != BOS_FAILED);
+ AZ(bo->failed);
vfps = VFP_Suck(bo, st->ptr + st->len, &l);
if (l > 0 && vfps != VFP_ERROR) {
assert(!VTAILQ_EMPTY(&bo->fetch_obj->store));
@@ -239,7 +238,7 @@ VFP_Fetch_Body(struct busyobj *bo, ssize_t est)
} while (vfps == VFP_OK);
if (vfps == VFP_ERROR) {
- assert(bo->state == BOS_FAILED);
+ AN(bo->failed);
(void)VFP_Error(bo, "Fetch Pipeline failed to process");
bo->should_close = 1;
}
diff --git a/bin/varnishtest/tests/r01284.vtc b/bin/varnishtest/tests/r01284.vtc
index 6299926..123ddb2 100644
--- a/bin/varnishtest/tests/r01284.vtc
+++ b/bin/varnishtest/tests/r01284.vtc
@@ -19,6 +19,10 @@ varnish v1 \
}
} -start
+varnish v1 -cliok "param.set debug +syncvsl"
+
+delay .1
+
client c1 {
# Fill transient
txreq -url "/obj1"
@@ -26,6 +30,8 @@ client c1 {
expect resp.status == 200
} -run
+delay .1
+
varnish v1 -expect SMA.Transient.g_bytes > 1048000
varnish v1 -expect SMA.Transient.g_space < 200
@@ -36,8 +42,8 @@ client c1 {
delay 1
} -run
-# Two failures, one for obj2 and one for the attempts at sending error
-varnish v1 -expect SMA.Transient.c_fail == 2
+# Three failures, one for obj2, one for vcl_backend_error{} and for vcl_error{}
+varnish v1 -expect SMA.Transient.c_fail == 3
client c1 {
# Check that Varnish is still alive
More information about the varnish-commit
mailing list