[master] 90c221829 fetch: Operate BOC state changes on busyobj
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Wed Aug 27 15:23:06 UTC 2025
commit 90c2218299df075648d8b9dc5117f45cff05ec24
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Tue Oct 22 15:33:23 2024 +0200
fetch: Operate BOC state changes on busyobj
This centralizes the rules regarding BOC state changes and whether to
inform clients of those changes. For example, BOS_REQ_DONE is only
needed by grace hits, saving one broadcast on the BOC condvar on misses
and straight passes.
It also formalizes when a busyobj will release its client request.
diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index c8814a7bb..9bda40bd5 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -169,3 +169,42 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo)
MPL_Free(vbopool, bo);
}
+
+void
+VBO_SetState(struct worker *wrk, struct busyobj *bo, enum boc_state_e next)
+{
+ unsigned broadcast;
+
+ CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+ CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+
+ switch (next) {
+ case BOS_REQ_DONE:
+ AN(bo->req);
+ bo->req = NULL;
+ broadcast = bo->is_bgfetch;
+ break;
+ case BOS_STREAM:
+ AN(bo->do_stream);
+ AZ(bo->req);
+ broadcast = 1;
+ break;
+ case BOS_FINISHED:
+ case BOS_FAILED:
+ /* We can't assert that either state already released its
+ * request because a fetch may fail before reaching the
+ * BOS_REQ_DONE state. Failing can also mean executing
+ * vcl_backend_error and reaching BOS_FINISHED from there.
+ * One can legitemately return(retry) from there and proceed
+ * again with a usable req if a return(error) transition led
+ * to vcl_backend_error instead of a failed fetch attempt.
+ */
+ bo->req = NULL;
+ broadcast = 1;
+ break;
+ default:
+ WRONG("unexpected BOC state");
+ }
+
+ ObjSetState(wrk, bo->fetch_objcore, next, broadcast);
+}
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index f4f9ee4a1..fe957f64b 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -292,14 +292,12 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
HTTP_Clone(bo->bereq, bo->bereq0);
if (bo->req->req_body_status->avail == 0) {
- bo->req = NULL;
- ObjSetState(bo->wrk, oc, BOS_REQ_DONE);
+ VBO_SetState(bo->wrk, bo, BOS_REQ_DONE);
} else if (bo->req->req_body_status == BS_CACHED) {
AN(bo->req->body_oc);
bo->bereq_body = bo->req->body_oc;
HSH_Ref(bo->bereq_body);
- bo->req = NULL;
- ObjSetState(bo->wrk, oc, BOS_REQ_DONE);
+ VBO_SetState(bo->wrk, bo, BOS_REQ_DONE);
}
return (F_STP_STARTFETCH);
}
@@ -544,10 +542,8 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
VSLb_ts_busyobj(bo, "Process", W_TIM_real(wrk));
assert(oc->boc->state <= BOS_REQ_DONE);
- if (oc->boc->state != BOS_REQ_DONE) {
- bo->req = NULL;
- ObjSetState(wrk, oc, BOS_REQ_DONE);
- }
+ if (oc->boc->state != BOS_REQ_DONE)
+ VBO_SetState(wrk, bo, BOS_REQ_DONE);
if (bo->do_esi)
bo->do_stream = 0;
@@ -713,7 +709,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
assert(oc->boc->state == BOS_REQ_DONE);
if (bo->do_stream)
- ObjSetState(wrk, oc, BOS_STREAM);
+ VBO_SetState(wrk, bo, BOS_STREAM);
VSLb(bo->vsl, SLT_Fetch_Body, "%u %s %s",
bo->htc->body_status->nbr, bo->htc->body_status->name,
@@ -751,7 +747,7 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo)
else
assert(oc->boc->state == BOS_REQ_DONE);
- ObjSetState(wrk, oc, BOS_FINISHED);
+ VBO_SetState(wrk, bo, BOS_FINISHED);
VSLb_ts_busyobj(bo, "BerespBody", W_TIM_real(wrk));
if (bo->stale_oc != NULL) {
VSL(SLT_ExpKill, NO_VXID, "VBF_Superseded x=%ju n=%ju",
@@ -879,7 +875,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
AZ(ObjCopyAttr(bo->wrk, oc, stale_oc, OA_GZIPBITS));
if (bo->do_stream)
- ObjSetState(wrk, oc, BOS_STREAM);
+ VBO_SetState(wrk, bo, BOS_STREAM);
INIT_OBJ(vop, VBF_OBITER_PRIV_MAGIC);
vop->bo = bo;
@@ -1015,7 +1011,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
VSB_destroy(&synth_body);
if (stale != NULL && oc->ttl > 0)
HSH_Kill(stale);
- ObjSetState(wrk, oc, BOS_FINISHED);
+ VBO_SetState(wrk, bo, BOS_FINISHED);
return (F_STP_DONE);
}
@@ -1033,7 +1029,7 @@ vbf_stp_fail(struct worker *wrk, struct busyobj *bo)
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
assert(oc->boc->state < BOS_FINISHED);
- ObjSetState(wrk, oc, BOS_FAILED);
+ VBO_SetState(wrk, bo, BOS_FAILED);
HSH_Kill(oc);
return (F_STP_DONE);
}
diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index 172e29a8b..804c226aa 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -491,7 +491,8 @@ ObjVAICancel(struct worker *wrk, struct boc *boc, struct vai_qe *qe)
*/
void
-ObjSetState(struct worker *wrk, struct objcore *oc, enum boc_state_e next)
+ObjSetState(struct worker *wrk, struct objcore *oc, enum boc_state_e next,
+ unsigned broadcast)
{
const struct obj_methods *om;
@@ -515,7 +516,8 @@ ObjSetState(struct worker *wrk, struct objcore *oc, enum boc_state_e next)
Lck_Lock(&oc->boc->mtx);
oc->boc->state = next;
- obj_boc_notify(oc->boc);
+ if (broadcast)
+ obj_boc_notify(oc->boc);
Lck_Unlock(&oc->boc->mtx);
}
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 1e00b4004..56ce1c14b 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -175,6 +175,8 @@ vtim_real BAN_Time(const struct ban *ban);
/* cache_busyobj.c */
struct busyobj *VBO_GetBusyObj(const struct worker *, const struct req *);
void VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **busyobj);
+void VBO_SetState(struct worker *wrk, struct busyobj *bo,
+ enum boc_state_e next);
/* cache_director.c */
int VDI_GetHdr(struct busyobj *);
@@ -342,7 +344,8 @@ int ObjGetSpace(struct worker *, struct objcore *, ssize_t *sz, uint8_t **ptr);
void ObjExtend(struct worker *, struct objcore *, ssize_t l, int final);
uint64_t ObjWaitExtend(const struct worker *, const struct objcore *,
uint64_t l, enum boc_state_e *statep);
-void ObjSetState(struct worker *, struct objcore *, enum boc_state_e next);
+void ObjSetState(struct worker *, struct objcore *, enum boc_state_e next,
+ unsigned broadcast);
void ObjWaitState(const struct objcore *, enum boc_state_e want);
void ObjTouch(struct worker *, struct objcore *, vtim_real now);
void ObjFreeObj(struct worker *, struct objcore *);
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index 4fcdf52c6..3415ff83c 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -648,9 +648,8 @@ VRT_u_bereq_body(VRT_CTX)
if (ctx->bo->req != NULL) {
CHECK_OBJ(ctx->bo->req, REQ_MAGIC);
- ctx->bo->req = NULL;
- ObjSetState(ctx->bo->wrk,
- ctx->bo->fetch_objcore, BOS_REQ_DONE);
+ VBO_SetState(ctx->bo->wrk, ctx->bo, BOS_REQ_DONE);
+ AZ(ctx->bo->req);
}
}
More information about the varnish-commit
mailing list