diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 9346f0980f..aa6a514367 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -354,6 +354,7 @@ struct objcore { uint16_t oa_present; unsigned timer_idx; // XXX 4Gobj limit + unsigned waitinglist_gen; vtim_real last_lru; VTAILQ_ENTRY(objcore) hsh_list; VTAILQ_ENTRY(objcore) lru_list; @@ -471,6 +472,7 @@ struct req { stream_close_t doclose; unsigned restarts; unsigned esi_level; + unsigned waitinglist_gen; /* Delivery mode */ unsigned res_mode; @@ -497,9 +499,6 @@ struct req { struct objcore *body_oc; - /* The busy objhead we sleep on */ - struct objhead *hash_objhead; - /* Built Vary string == workspace reservation */ uint8_t *vary_b; uint8_t *vary_e; diff --git a/bin/varnishd/cache/cache_ban_lurker.c b/bin/varnishd/cache/cache_ban_lurker.c index 17087a8e2b..7ce26ac229 100644 --- a/bin/varnishd/cache/cache_ban_lurker.c +++ b/bin/varnishd/cache/cache_ban_lurker.c @@ -332,7 +332,7 @@ ban_lurker_test_ban(struct worker *wrk, struct ban *bt, if (i) ObjSendEvent(wrk, oc, OEV_BANCHG); } - (void)HSH_DerefObjCore(wrk, &oc, 0); + (void)HSH_DerefObjCore(wrk, &oc); } } diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c index 593bdfe430..d20777f765 100644 --- a/bin/varnishd/cache/cache_busyobj.c +++ b/bin/varnishd/cache/cache_busyobj.c @@ -179,8 +179,7 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo) wrk->stats->ws_backend_overflow++; if (bo->fetch_objcore != NULL) { - (void)HSH_DerefObjCore(wrk, &bo->fetch_objcore, - HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &bo->fetch_objcore); } VRT_Assign_Backend(&bo->director_req, NULL); @@ -195,3 +194,34 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo) vbo_Free(&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); + broadcast = bo->is_bgfetch; + break; + case BOS_STREAM: + if (!bo->do_stream) { + bo->req = NULL; + return; /* keep objcore busy */ + } + /* fall through */ + case BOS_FINISHED: + case BOS_FAILED: + broadcast = 1; + break; + default: + WRONG("unexpected BOC state"); + } + + bo->req = NULL; + ObjSetState(wrk, bo->fetch_objcore, next, broadcast); +} diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c index ff1c041527..fb4d80a849 100644 --- a/bin/varnishd/cache/cache_expire.c +++ b/bin/varnishd/cache/cache_expire.c @@ -213,7 +213,7 @@ EXP_Insert(struct worker *wrk, struct objcore *oc) ObjSendEvent(wrk, oc, OEV_EXPIRE); tmpoc = oc; assert(oc->refcnt >= 2); /* Silence coverity */ - (void)HSH_DerefObjCore(wrk, &oc, 0); + (void)HSH_DerefObjCore(wrk, &oc); AZ(oc); assert(tmpoc->refcnt >= 1); /* Silence coverity */ } @@ -309,7 +309,7 @@ exp_inbox(struct exp_priv *ep, struct objcore *oc, unsigned flags, double now) VXID(ObjGetXID(ep->wrk, oc)), EXP_Ttl(NULL, oc) - now, (intmax_t)oc->hits); ObjSendEvent(ep->wrk, oc, OEV_EXPIRE); - (void)HSH_DerefObjCore(ep->wrk, &oc, 0); + (void)HSH_DerefObjCore(ep->wrk, &oc); return; } @@ -387,7 +387,7 @@ exp_expire(struct exp_priv *ep, vtim_real now) VXID(ObjGetXID(ep->wrk, oc)), EXP_Ttl(NULL, oc) - now, (intmax_t)oc->hits); ObjSendEvent(ep->wrk, oc, OEV_EXPIRE); - (void)HSH_DerefObjCore(ep->wrk, &oc, 0); + (void)HSH_DerefObjCore(ep->wrk, &oc); } return (0); } diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 76a819c2c6..e559e01e08 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -289,14 +289,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); } @@ -538,10 +536,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; @@ -706,11 +702,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_PREP_STREAM); - HSH_Unbusy(wrk, oc); - 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, @@ -745,13 +737,10 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo) if (bo->do_stream) assert(oc->boc->state == BOS_STREAM); - else { + else assert(oc->boc->state == BOS_REQ_DONE); - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); - } - 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", @@ -878,11 +867,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo) ObjSetFlag(bo->wrk, oc, OF_IMSCAND, 0); AZ(ObjCopyAttr(bo->wrk, oc, stale_oc, OA_GZIPBITS)); - if (bo->do_stream) { - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); - ObjSetState(wrk, oc, BOS_STREAM); - } + VBO_SetState(wrk, bo, BOS_STREAM); INIT_OBJ(vop, VBF_OBITER_PRIV_MAGIC); vop->bo = bo; @@ -903,9 +888,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo) * * replaces a stale object unless * - abandoning the bereq or - * - leaving vcl_backend_error with return (deliver) and beresp.ttl == 0s or - * - there is a waitinglist on this object because in this case the default ttl - * would be 1s, so we might be looking at the same case as the previous + * - leaving vcl_backend_error with return (deliver) * * We do want the stale replacement to avoid an object pileup with short ttl and * long grace/keep, yet there could exist cases where a cache object is @@ -928,7 +911,8 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo) CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); oc = bo->fetch_objcore; CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); - AN(oc->flags & OC_F_BUSY); + CHECK_OBJ_NOTNULL(oc->boc, BOC_MAGIC); + assert(oc->boc->state < BOS_STREAM); assert(bo->director_state == DIR_S_NULL); if (wrk->vpi->handling != VCL_RET_ERROR) @@ -960,23 +944,9 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo) stale = bo->stale_oc; oc->t_origin = now; - if (!VTAILQ_EMPTY(&oc->objhead->waitinglist)) { - /* - * If there is a waitinglist, it means that there is no - * grace-able object, so cache the error return for a - * short time, so the waiting list can drain, rather than - * each objcore on the waiting list sequentially attempt - * to fetch from the backend. - */ - oc->ttl = 1; - oc->grace = 5; - oc->keep = 5; - stale = NULL; - } else { - oc->ttl = 0; - oc->grace = 0; - oc->keep = 0; - } + oc->ttl = 0; + oc->grace = 0; + oc->keep = 0; synth_body = VSB_new_auto(); AN(synth_body); @@ -1026,11 +996,10 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo) } AZ(ObjSetU64(wrk, oc, OA_LEN, o)); VSB_destroy(&synth_body); - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); + VBO_SetState(wrk, bo, BOS_STREAM); if (stale != NULL && oc->ttl > 0) HSH_Kill(stale); - ObjSetState(wrk, oc, BOS_FINISHED); + VBO_SetState(wrk, bo, BOS_FINISHED); return (F_STP_DONE); } @@ -1048,10 +1017,8 @@ vbf_stp_fail(struct worker *wrk, struct busyobj *bo) CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); assert(oc->boc->state < BOS_FINISHED); - HSH_Fail(oc); - if (!(oc->flags & OC_F_BUSY)) - HSH_Kill(oc); - ObjSetState(wrk, oc, BOS_FAILED); + VBO_SetState(wrk, bo, BOS_FAILED); + HSH_Kill(oc); return (F_STP_DONE); } @@ -1123,7 +1090,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv) http_Teardown(bo->beresp); // cannot make assumptions about the number of references here #3434 if (bo->bereq_body != NULL) - (void) HSH_DerefObjCore(bo->wrk, &bo->bereq_body, 0); + (void)HSH_DerefObjCore(bo->wrk, &bo->bereq_body); if (oc->boc->state == BOS_FINISHED) { AZ(oc->flags & OC_F_FAILED); @@ -1133,7 +1100,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv) // AZ(oc->boc); // XXX if (bo->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &bo->stale_oc); wrk->vsl = NULL; HSH_DerefBoc(wrk, oc); @@ -1157,7 +1124,6 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(req, REQ_MAGIC); CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); - AN(oc->flags & OC_F_BUSY); CHECK_OBJ_ORNULL(oldoc, OBJCORE_MAGIC); bo = VBO_GetBusyObj(wrk, req); @@ -1166,6 +1132,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, boc = HSH_RefBoc(oc); CHECK_OBJ_NOTNULL(boc, BOC_MAGIC); + assert(boc->state < BOS_STREAM); switch (mode) { case VBF_PASS: @@ -1224,7 +1191,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, "No thread available for bgfetch"); (void)vbf_stp_fail(req->wrk, bo); if (bo->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &bo->stale_oc); HSH_DerefBoc(wrk, oc); SES_Rel(bo->sp); THR_SetBusyobj(NULL); @@ -1237,11 +1204,9 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, (void)VRB_Ignore(req); } else { ObjWaitState(oc, BOS_STREAM); - if (oc->boc->state == BOS_FAILED) { - AN((oc->flags & OC_F_FAILED)); - } else { - AZ(oc->flags & OC_F_BUSY); - } + AZ(oc->flags & OC_F_BUSY); + if (oc->boc->state == BOS_FAILED) + AN(oc->flags & OC_F_FAILED); } } AZ(bo); @@ -1249,5 +1214,5 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, assert(oc->boc == boc); HSH_DerefBoc(wrk, oc); if (mode == VBF_BACKGROUND) - (void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &oc); } diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c index 6446f3beb0..8bbf85a710 100644 --- a/bin/varnishd/cache/cache_hash.c +++ b/bin/varnishd/cache/cache_hash.c @@ -75,12 +75,12 @@ struct rush { static const struct hash_slinger *hash; static struct objhead *private_oh; -static void hsh_rush1(const struct worker *, struct objhead *, - struct rush *, int); +static void hsh_rush1(const struct worker *, struct objcore *, + struct rush *); static void hsh_rush2(struct worker *, struct rush *); static int hsh_deref_objhead(struct worker *wrk, struct objhead **poh); static int hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, - int); + struct objcore *oc); /*---------------------------------------------------------------------*/ @@ -101,6 +101,7 @@ hsh_newobjhead(void) ALLOC_OBJ(oh, OBJHEAD_MAGIC); XXXAN(oh); oh->refcnt = 1; + oh->waitinglist_gen = 1; VTAILQ_INIT(&oh->objcs); VTAILQ_INIT(&oh->waitinglist); Lck_New(&oh->mtx, lck_objhdr); @@ -294,7 +295,7 @@ HSH_Insert(struct worker *wrk, const void *digest, struct objcore *oc, AN(digest); CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); AN(ban); - AN(oc->flags & OC_F_BUSY); + AZ(oc->flags & OC_F_BUSY); AZ(oc->flags & OC_F_PRIVATE); assert(oc->refcnt == 1); INIT_OBJ(&rush, RUSH_MAGIC); @@ -322,9 +323,8 @@ HSH_Insert(struct worker *wrk, const void *digest, struct objcore *oc, Lck_Lock(&oh->mtx); VTAILQ_REMOVE(&oh->objcs, oc, hsh_list); VTAILQ_INSERT_HEAD(&oh->objcs, oc, hsh_list); - oc->flags &= ~OC_F_BUSY; if (!VTAILQ_EMPTY(&oh->waitinglist)) - hsh_rush1(wrk, oh, &rush, HSH_RUSH_POLICY); + hsh_rush1(wrk, oc, &rush); Lck_Unlock(&oh->mtx); hsh_rush2(wrk, &rush); @@ -348,13 +348,49 @@ hsh_insert_busyobj(const struct worker *wrk, struct objhead *oh) wrk->wpriv->nobjcore = NULL; CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); - AN(oc->flags & OC_F_BUSY); + AZ(oc->flags & OC_F_BUSY); + oc->flags |= OC_F_BUSY; oc->refcnt = 1; /* Owned by busyobj */ oc->objhead = oh; VTAILQ_INSERT_TAIL(&oh->objcs, oc, hsh_list); return (oc); } +/*--------------------------------------------------------------------- +*/ + +static unsigned +hsh_rush_match(struct req *req) +{ + struct objhead *oh; + struct objcore *oc; + const uint8_t *vary; + + oc = req->objcore; + CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); + + AZ(oc->flags & OC_F_BUSY); + AZ(oc->flags & OC_F_PRIVATE); + if (oc->flags & (OC_F_WITHDRAWN|OC_F_HFM|OC_F_HFP|OC_F_CANCEL| + OC_F_FAILED)) + return (0); + + if (req->vcf != NULL) /* NB: must operate under oh lock. */ + return (0); + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + + if (req->hash_ignore_vary) + return (1); + if (!ObjHasAttr(req->wrk, oc, OA_VARY)) + return (1); + + vary = ObjGetAttr(req->wrk, oc, OA_VARY, NULL); + AN(vary); + return (VRY_Match(req, vary)); +} + /*--------------------------------------------------------------------- */ @@ -383,6 +419,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(wrk->wpriv, WORKER_PRIV_MAGIC); CHECK_OBJ_NOTNULL(req->http, HTTP_MAGIC); + CHECK_OBJ_ORNULL(req->objcore, OBJCORE_MAGIC); CHECK_OBJ_ORNULL(req->vcf, VCF_MAGIC); AN(hash); @@ -390,15 +427,32 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) if (DO_DEBUG(DBG_HASHEDGE)) hsh_testmagic(req->digest); - if (req->hash_objhead != NULL) { + if (req->objcore != NULL && hsh_rush_match(req)) { + TAKE_OBJ_NOTNULL(oc, &req->objcore, OBJCORE_MAGIC); + *ocp = oc; + oh = oc->objhead; + Lck_Lock(&oh->mtx); + oc->hits++; + boc_progress = oc->boc == NULL ? -1 : oc->boc->fetched_so_far; + AN(hsh_deref_objhead_unlock(wrk, &oh, oc)); + Req_LogHit(wrk, req, oc, boc_progress); + /* NB: since this hit comes from the waiting list instead of + * a regular lookup, grace is not considered. The object is + * fresh in the context of the waiting list, even expired: it + * was successfully just [re]validated by a fetch task. + */ + return (HSH_HIT); + } + + if (req->objcore != NULL) { /* * This req came off the waiting list, and brings an - * oh refcnt with it. + * oh refcnt and an incompatible oc refcnt with it, + * the latter acquired during rush hour. */ - CHECK_OBJ_NOTNULL(req->hash_objhead, OBJHEAD_MAGIC); - oh = req->hash_objhead; + oh = req->objcore->objhead; + (void)HSH_DerefObjCore(wrk, &req->objcore); Lck_Lock(&oh->mtx); - req->hash_objhead = NULL; } else { AN(wrk->wpriv->nobjhead); oh = hash->lookup(wrk, req->digest, &wrk->wpriv->nobjhead); @@ -501,7 +555,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) if (oc != NULL && oc->flags & OC_F_HFP) { xid = VXID(ObjGetXID(wrk, oc)); dttl = EXP_Dttl(req, oc); - AN(hsh_deref_objhead_unlock(wrk, &oh, HSH_RUSH_POLICY)); + AN(hsh_deref_objhead_unlock(wrk, &oh, oc)); wrk->stats->cache_hitpass++; VSLb(req->vsl, SLT_HitPass, "%u %.6f", xid, dttl); return (HSH_HITPASS); @@ -521,7 +575,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) } oc->hits++; boc_progress = oc->boc == NULL ? -1 : oc->boc->fetched_so_far; - AN(hsh_deref_objhead_unlock(wrk, &oh, HSH_RUSH_POLICY)); + AN(hsh_deref_objhead_unlock(wrk, &oh, oc)); Req_LogHit(wrk, req, oc, boc_progress); return (HSH_HIT); } @@ -570,7 +624,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) exp_oc->refcnt++; *ocp = exp_oc; exp_oc->hits++; - AN(hsh_deref_objhead_unlock(wrk, &oh, 0)); + AN(hsh_deref_objhead_unlock(wrk, &oh, NULL)); Req_LogHit(wrk, req, exp_oc, boc_progress); return (HSH_GRACE); } @@ -581,13 +635,14 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) AZ(req->hash_ignore_busy); /* - * The objhead reference transfers to the sess, we get it - * back when the sess comes off the waiting list and - * calls us again + * The objhead reference is held by req while it is parked on the + * waiting list. The oh pointer is taken back from the objcore that + * triggers a rush of req off the waiting list. */ - req->hash_objhead = oh; + assert(oh->refcnt > 1); + req->wrk = NULL; - req->waitinglist = 1; + req->waitinglist_gen = oh->waitinglist_gen; if (DO_DEBUG(DBG_WAITINGLIST)) VSLb(req->vsl, SLT_Debug, "on waiting list <%p>", oh); @@ -603,33 +658,52 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) */ static void -hsh_rush1(const struct worker *wrk, struct objhead *oh, struct rush *r, int max) +hsh_rush1(const struct worker *wrk, struct objcore *oc, struct rush *r) { - int i; + struct objhead *oh; struct req *req; - - if (max == 0) - return; - if (max == HSH_RUSH_POLICY) - max = cache_param->rush_exponent; - assert(max > 0); + int i, max; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + CHECK_OBJ_ORNULL(oc, OBJCORE_MAGIC); CHECK_OBJ_NOTNULL(r, RUSH_MAGIC); VTAILQ_INIT(&r->reqs); + + if (oc == NULL) + return; + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); Lck_AssertHeld(&oh->mtx); + + AZ(oc->flags & OC_F_BUSY); + AZ(oc->flags & OC_F_PRIVATE); + max = cache_param->rush_exponent; + if (oc->flags & (OC_F_WITHDRAWN|OC_F_FAILED)) + max = 1; + assert(max > 0); + + if (oc->waitinglist_gen == 0) { + oc->waitinglist_gen = oh->waitinglist_gen; + oh->waitinglist_gen++; + } + for (i = 0; i < max; i++) { req = VTAILQ_FIRST(&oh->waitinglist); + CHECK_OBJ_ORNULL(req, REQ_MAGIC); if (req == NULL) break; - CHECK_OBJ_NOTNULL(req, REQ_MAGIC); - wrk->stats->busy_wakeup++; + if (req->waitinglist_gen > oc->waitinglist_gen) + continue; + AZ(req->wrk); VTAILQ_REMOVE(&oh->waitinglist, req, w_list); VTAILQ_INSERT_TAIL(&r->reqs, req, w_list); - req->waitinglist = 0; + req->objcore = oc; } + + oc->refcnt += i; + wrk->stats->busy_wakeup += i; } /*--------------------------------------------------------------------- @@ -748,7 +822,7 @@ HSH_Purge(struct worker *wrk, struct objhead *oh, vtim_real ttl_now, EXP_Remove(ocp[i], NULL); else EXP_Reduce(ocp[i], ttl_now, ttl, grace, keep); - (void)HSH_DerefObjCore(wrk, &ocp[i], 0); + (void)HSH_DerefObjCore(wrk, &ocp[i]); AZ(ocp[i]); total++; } @@ -782,24 +856,33 @@ HSH_Purge(struct worker *wrk, struct objhead *oh, vtim_real ttl_now, */ void -HSH_Fail(struct objcore *oc) +HSH_Fail(struct worker *wrk, struct objcore *oc) { struct objhead *oh; + struct rush rush; + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); oh = oc->objhead; CHECK_OBJ(oh, OBJHEAD_MAGIC); + INIT_OBJ(&rush, RUSH_MAGIC); /* - * We have to have either a busy bit, so that HSH_Lookup - * will not consider this oc, or an object hung of the oc - * so that it can consider it. + * We have to have either a busy bit or a pass, so that + * HSH_Lookup will not consider this oc, or an object + * hung of the oc so that it can consider it. */ - assert((oc->flags & OC_F_BUSY) || (oc->stobj->stevedore != NULL)); + assert((oc->flags & (OC_F_BUSY|OC_F_PRIVATE)) || + (oc->stobj->stevedore != NULL)); Lck_Lock(&oh->mtx); oc->flags |= OC_F_FAILED; + if (oc->flags & OC_F_BUSY) { + oc->flags &= ~OC_F_BUSY; + hsh_rush1(wrk, oc, &rush); + } Lck_Unlock(&oh->mtx); + hsh_rush2(wrk, &rush); } /*--------------------------------------------------------------------- @@ -857,6 +940,36 @@ HSH_Cancel(struct worker *wrk, struct objcore *oc, struct boc *boc) ObjSlim(wrk, oc); } +/*--------------------------------------------------------------------- + * Withdraw an objcore that will not proceed with a fetch. + */ + +void +HSH_Withdraw(struct worker *wrk, struct objcore **ocp) +{ + struct objhead *oh; + struct objcore *oc; + struct rush rush; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); + INIT_OBJ(&rush, RUSH_MAGIC); + + oh = oc->objhead; + CHECK_OBJ(oh, OBJHEAD_MAGIC); + + Lck_Lock(&oh->mtx); + AZ(oc->stobj->stevedore); + assert(oc->flags == OC_F_BUSY); + assert(oc->refcnt == 1); + assert(oh->refcnt > 0); + oc->flags = OC_F_WITHDRAWN; + hsh_rush1(wrk, oc, &rush); /* grabs up to 1 oc ref */ + assert(HSH_DerefObjCoreUnlock(wrk, &oc) <= 1); + + hsh_rush2(wrk, &rush); +} + /*--------------------------------------------------------------------- * Unbusy an objcore when the object is completely fetched. */ @@ -873,6 +986,17 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc) oh = oc->objhead; CHECK_OBJ(oh, OBJHEAD_MAGIC); + + /* NB: It is guaranteed that exactly one request is waiting for + * the objcore for pass objects. The other reference is held by + * the current fetch task. + */ + if (oc->flags & OC_F_PRIVATE) { + AZ(oc->flags & OC_F_BUSY); + assert(oc->refcnt == 2); + return; + } + INIT_OBJ(&rush, RUSH_MAGIC); AN(oc->stobj->stevedore); @@ -880,28 +1004,24 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc) assert(oh->refcnt > 0); assert(oc->refcnt > 0); - if (!(oc->flags & OC_F_PRIVATE)) { - BAN_NewObjCore(oc); - AN(oc->ban); - } + BAN_NewObjCore(oc); + AN(oc->ban); /* XXX: pretouch neighbors on oh->objcs to prevent page-on under mtx */ Lck_Lock(&oh->mtx); assert(oh->refcnt > 0); assert(oc->refcnt > 0); - if (!(oc->flags & OC_F_PRIVATE)) - EXP_RefNewObjcore(oc); /* Takes a ref for expiry */ + EXP_RefNewObjcore(oc); /* Takes a ref for expiry */ /* XXX: strictly speaking, we should sort in Date: order. */ VTAILQ_REMOVE(&oh->objcs, oc, hsh_list); VTAILQ_INSERT_HEAD(&oh->objcs, oc, hsh_list); oc->flags &= ~OC_F_BUSY; if (!VTAILQ_EMPTY(&oh->waitinglist)) { assert(oh->refcnt > 1); - hsh_rush1(wrk, oh, &rush, HSH_RUSH_POLICY); + hsh_rush1(wrk, oc, &rush); } Lck_Unlock(&oh->mtx); - EXP_Insert(wrk, oc); /* Does nothing unless EXP_RefNewObjcore was - * called */ + EXP_Insert(wrk, oc); hsh_rush2(wrk, &rush); } @@ -1040,35 +1160,46 @@ HSH_DerefBoc(struct worker *wrk, struct objcore *oc) */ int -HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax) +HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp) { struct objcore *oc; struct objhead *oh; - struct rush rush; - int r; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); assert(oc->refcnt > 0); - INIT_OBJ(&rush, RUSH_MAGIC); oh = oc->objhead; CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); Lck_Lock(&oh->mtx); + return (HSH_DerefObjCoreUnlock(wrk, &oc)); +} + +int +HSH_DerefObjCoreUnlock(struct worker *wrk, struct objcore **ocp) +{ + struct objcore *oc; + struct objhead *oh; + int r; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); + assert(oc->refcnt > 0); + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + + Lck_AssertHeld(&oh->mtx); assert(oh->refcnt > 0); r = --oc->refcnt; if (!r) VTAILQ_REMOVE(&oh->objcs, oc, hsh_list); - if (!VTAILQ_EMPTY(&oh->waitinglist)) { - assert(oh->refcnt > 1); - hsh_rush1(wrk, oh, &rush, rushmax); - } Lck_Unlock(&oh->mtx); - hsh_rush2(wrk, &rush); if (r != 0) return (r); + AZ(oc->flags & OC_F_BUSY); AZ(oc->exp_flags); BAN_DestroyObj(oc); @@ -1085,7 +1216,8 @@ HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax) } static int -hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, int max) +hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, + struct objcore *oc) { struct objhead *oh; struct rush rush; @@ -1107,7 +1239,7 @@ hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, int max) INIT_OBJ(&rush, RUSH_MAGIC); if (!VTAILQ_EMPTY(&oh->waitinglist)) { assert(oh->refcnt > 1); - hsh_rush1(wrk, oh, &rush, max); + hsh_rush1(wrk, oc, &rush); } if (oh->refcnt == 1) @@ -1128,7 +1260,7 @@ hsh_deref_objhead(struct worker *wrk, struct objhead **poh) TAKE_OBJ_NOTNULL(oh, poh, OBJHEAD_MAGIC); Lck_Lock(&oh->mtx); - return (hsh_deref_objhead_unlock(wrk, &oh, 0)); + return (hsh_deref_objhead_unlock(wrk, &oh, NULL)); } void diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index 9ffa373f3b..363f109561 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -86,6 +86,7 @@ #include "cache_varnishd.h" #include "cache_obj.h" +#include "cache_objhead.h" #include "vend.h" #include "storage/storage.h" @@ -141,8 +142,6 @@ ObjNew(const struct worker *wrk) AN(oc); wrk->stats->n_objectcore++; oc->last_lru = NAN; - oc->flags = OC_F_BUSY; - oc->boc = obj_newboc(); return (oc); @@ -292,8 +291,8 @@ ObjWaitExtend(const struct worker *wrk, const struct objcore *oc, uint64_t l, */ void -ObjSetState(struct worker *wrk, const 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; @@ -302,7 +301,6 @@ ObjSetState(struct worker *wrk, const struct objcore *oc, assert(next > oc->boc->state); CHECK_OBJ_ORNULL(oc->stobj->stevedore, STEVEDORE_MAGIC); - assert(next != BOS_STREAM || oc->boc->state == BOS_PREP_STREAM); assert(next != BOS_FINISHED || (oc->oa_present & (1 << OA_LEN))); if (oc->stobj->stevedore != NULL) { @@ -311,9 +309,15 @@ ObjSetState(struct worker *wrk, const struct objcore *oc, om->objsetstate(wrk, oc, next); } + if (next == BOS_FAILED) + HSH_Fail(wrk, oc); + else if (oc->boc->state < BOS_STREAM && next >= BOS_STREAM) + HSH_Unbusy(wrk, oc); + Lck_Lock(&oc->boc->mtx); oc->boc->state = next; - PTOK(pthread_cond_broadcast(&oc->boc->cond)); + if (broadcast) + PTOK(pthread_cond_broadcast(&oc->boc->cond)); Lck_Unlock(&oc->boc->mtx); } diff --git a/bin/varnishd/cache/cache_objhead.h b/bin/varnishd/cache/cache_objhead.h index 2edfb5a883..162aaeb7fa 100644 --- a/bin/varnishd/cache/cache_objhead.h +++ b/bin/varnishd/cache/cache_objhead.h @@ -40,6 +40,7 @@ struct objhead { struct lock mtx; VTAILQ_HEAD(,objcore) objcs; uint8_t digest[DIGEST_LEN]; + unsigned waitinglist_gen; VTAILQ_HEAD(, req) waitinglist; /*---------------------------------------------------- @@ -65,20 +66,19 @@ enum lookup_e { HSH_BUSY, }; -void HSH_Fail(struct objcore *); void HSH_Kill(struct objcore *); void HSH_Replace(struct objcore *, const struct objcore *); void HSH_Insert(struct worker *, const void *hash, struct objcore *, struct ban *); +void HSH_Withdraw(struct worker *, struct objcore **); +void HSH_Fail(struct worker *, struct objcore *); void HSH_Unbusy(struct worker *, struct objcore *); int HSH_Snipe(const struct worker *, struct objcore *); struct boc *HSH_RefBoc(const struct objcore *); void HSH_DerefBoc(struct worker *wrk, struct objcore *); void HSH_DeleteObjHead(const struct worker *, struct objhead *); - -int HSH_DerefObjCore(struct worker *, struct objcore **, int rushmax); -#define HSH_RUSH_POLICY -1 - +int HSH_DerefObjCore(struct worker *, struct objcore **); +int HSH_DerefObjCoreUnlock(struct worker *, struct objcore **); enum lookup_e HSH_Lookup(struct req *, struct objcore **, struct objcore **); void HSH_Ref(struct objcore *o); void HSH_AddString(struct req *, void *ctx, const char *str); diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c index 9a4cd54d7e..cd19421636 100644 --- a/bin/varnishd/cache/cache_req_body.c +++ b/bin/varnishd/cache/cache_req_body.c @@ -80,7 +80,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (STV_NewObject(req->wrk, req->body_oc, stv, 0) == 0) { req->req_body_status = BS_ERROR; HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); (void)VFP_Error(vfc, "Object allocation failed:" " Ran out of space in %s", stv->vclname); return (-1); @@ -96,7 +96,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) (void)VFP_Error(vfc, "req.body filters failed"); req->req_body_status = BS_ERROR; HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); return (-1); } @@ -104,7 +104,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (VFP_Open(ctx, vfc) < 0) { req->req_body_status = BS_ERROR; HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); return (-1); } @@ -152,7 +152,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) VSLb_ts_req(req, "ReqBody", VTIM_real()); if (func != NULL) { HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); if (vfps == VFP_END && r == 0 && (flush & OBJ_ITER_END) == 0) r = func(priv, flush | OBJ_ITER_END, NULL, 0); if (vfps != VFP_END) { @@ -168,7 +168,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (vfps != VFP_END) { req->req_body_status = BS_ERROR; - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); return (-1); } @@ -289,7 +289,7 @@ VRB_Free(struct req *req) if (req->body_oc == NULL) return; - r = HSH_DerefObjCore(req->wrk, &req->body_oc, 0); + r = HSH_DerefObjCore(req->wrk, &req->body_oc); // each busyobj may have gained a reference assert (r >= 0); diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index bbcb3824f7..8a592428fc 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -222,7 +222,7 @@ cnt_deliver(struct worker *wrk, struct req *req) ObjTouch(req->wrk, req->objcore, req->t_prev); if (Resp_Setup_Deliver(req)) { - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); req->err_code = 500; req->req_step = R_STP_SYNTH; return (REQ_FSM_MORE); @@ -240,7 +240,7 @@ cnt_deliver(struct worker *wrk, struct req *req) if (wrk->vpi->handling != VCL_RET_DELIVER) { HSH_Cancel(wrk, req->objcore, NULL); - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); switch (wrk->vpi->handling) { @@ -414,7 +414,7 @@ cnt_synth(struct worker *wrk, struct req *req) VSLb(req->vsl, SLT_Error, "Could not get storage"); req->doclose = SC_OVERLOAD; VSLb_ts_req(req, "Resp", W_TIM_real(wrk)); - (void)HSH_DerefObjCore(wrk, &req->objcore, 1); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); return (REQ_FSM_DONE); } @@ -530,7 +530,7 @@ cnt_finish(struct worker *wrk, struct req *req) req->boc = NULL; } - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); req->vdp_filter_list = NULL; @@ -557,7 +557,7 @@ cnt_fetch(struct worker *wrk, struct req *req) if (req->objcore->flags & OC_F_FAILED) { req->err_code = 503; req->req_step = R_STP_SYNTH; - (void)HSH_DerefObjCore(wrk, &req->objcore, 1); + (void)HSH_DerefObjCore(wrk, &req->objcore); AZ(req->objcore); return (REQ_FSM_MORE); } @@ -576,20 +576,23 @@ cnt_lookup(struct worker *wrk, struct req *req) { struct objcore *oc, *busy; enum lookup_e lr; - int had_objhead = 0; + int had_objcore = 0; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(req, REQ_MAGIC); - AZ(req->objcore); AZ(req->stale_oc); AN(req->vcl); VRY_Prep(req); - AZ(req->objcore); - if (req->hash_objhead) - had_objhead = 1; + if (req->waitinglist_gen) { + CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC); + req->waitinglist_gen = 0; + had_objcore = 1; + } else + AZ(req->objcore); + wrk->strangelove = 0; lr = HSH_Lookup(req, &oc, &busy); if (lr == HSH_BUSY) { @@ -605,7 +608,7 @@ cnt_lookup(struct worker *wrk, struct req *req) if ((unsigned)wrk->strangelove >= cache_param->vary_notice) VSLb(req->vsl, SLT_Notice, "vsl: High number of variants (%d)", wrk->strangelove); - if (had_objhead) + if (had_objcore) VSLb_ts_req(req, "Waitinglist", W_TIM_real(wrk)); if (req->vcf != NULL) { @@ -685,10 +688,10 @@ cnt_lookup(struct worker *wrk, struct req *req) } /* Drop our object, we won't need it */ - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); if (busy != NULL) { - (void)HSH_DerefObjCore(wrk, &busy, 0); + HSH_Withdraw(wrk, &busy); VRY_Clear(req); } @@ -715,7 +718,7 @@ cnt_miss(struct worker *wrk, struct req *req) wrk->stats->cache_miss++; VBF_Fetch(wrk, req, req->objcore, req->stale_oc, VBF_NORMAL); if (req->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &req->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &req->stale_oc); req->req_step = R_STP_FETCH; return (REQ_FSM_MORE); case VCL_RET_FAIL: @@ -735,8 +738,8 @@ cnt_miss(struct worker *wrk, struct req *req) } VRY_Clear(req); if (req->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &req->stale_oc, 0); - AZ(HSH_DerefObjCore(wrk, &req->objcore, 1)); + (void)HSH_DerefObjCore(wrk, &req->stale_oc); + HSH_Withdraw(wrk, &req->objcore); return (REQ_FSM_MORE); } @@ -1108,7 +1111,7 @@ cnt_purge(struct worker *wrk, struct req *req) (void)HSH_Purge(wrk, boc->objhead, req->t_req, 0, 0, 0); - AZ(HSH_DerefObjCore(wrk, &boc, 1)); + HSH_Withdraw(wrk, &boc); VCL_purge_method(req->vcl, wrk, req, NULL, NULL); switch (wrk->vpi->handling) { diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index a0e4987fab..7c32e45bcc 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 *); @@ -337,8 +339,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 *, const 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_vary.c b/bin/varnishd/cache/cache_vary.c index fa9053966e..7a3962bdc2 100644 --- a/bin/varnishd/cache/cache_vary.c +++ b/bin/varnishd/cache/cache_vary.c @@ -224,8 +224,7 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2) void VRY_Prep(struct req *req) { - if (req->hash_objhead == NULL) { - /* Not a waiting list return */ + if (req->waitinglist_gen == 0) { AZ(req->vary_b); AZ(req->vary_e); (void)WS_ReserveAll(req->ws); diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c index 5d5be780d9..cb7768a55b 100644 --- a/bin/varnishd/cache/cache_vrt_var.c +++ b/bin/varnishd/cache/cache_vrt_var.c @@ -641,15 +641,13 @@ VRT_u_bereq_body(VRT_CTX) CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC); if (ctx->bo->bereq_body != NULL) { - (void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body, 0); + (void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body); http_Unset(ctx->bo->bereq, H_Content_Length); } 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); http_Unset(ctx->bo->bereq, H_Content_Length); } } diff --git a/bin/varnishd/storage/storage_lru.c b/bin/varnishd/storage/storage_lru.c index 5e046e7946..ec0369c817 100644 --- a/bin/varnishd/storage/storage_lru.c +++ b/bin/varnishd/storage/storage_lru.c @@ -205,6 +205,6 @@ LRU_NukeOne(struct worker *wrk, struct lru *lru) ObjSlim(wrk, oc); VSLb(wrk->vsl, SLT_ExpKill, "LRU xid=%ju", VXID(ObjGetXID(wrk, oc))); - (void)HSH_DerefObjCore(wrk, &oc, 0); // Ref from HSH_Snipe + (void)HSH_DerefObjCore(wrk, &oc); // Ref from HSH_Snipe return (1); } diff --git a/bin/varnishd/storage/storage_persistent_silo.c b/bin/varnishd/storage/storage_persistent_silo.c index 81c24b4367..eb96acd3ad 100644 --- a/bin/varnishd/storage/storage_persistent_silo.c +++ b/bin/varnishd/storage/storage_persistent_silo.c @@ -178,7 +178,7 @@ smp_load_seg(struct worker *wrk, const struct smp_sc *sc, HSH_Insert(wrk, so->hash, oc, ban); AN(oc->ban); HSH_DerefBoc(wrk, oc); // XXX Keep it an stream resurrection? - (void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &oc); wrk->stats->n_vampireobject++; } Pool_Sumstat(wrk); diff --git a/bin/varnishtest/tests/c00125.vtc b/bin/varnishtest/tests/c00125.vtc new file mode 100644 index 0000000000..8fbca4f46b --- /dev/null +++ b/bin/varnishtest/tests/c00125.vtc @@ -0,0 +1,156 @@ +varnishtest "successful expired waiting list hit" + +barrier b1 cond 2 +barrier b2 cond 2 +barrier b3 cond 2 +barrier b4 cond 2 + + +server s1 { + rxreq + expect req.http.user-agent == c1 + expect req.http.bgfetch == false + barrier b1 sync + barrier b2 sync + txresp -hdr "Cache-Control: max-age=60" -hdr "Age: 120" + + rxreq + expect req.http.user-agent == c3 + expect req.http.bgfetch == true + txresp + + # The no-cache case only works with a complicit VCL, for now. + rxreq + expect req.http.user-agent == c4 + expect req.http.bgfetch == false + barrier b3 sync + barrier b4 sync + txresp -hdr "Cache-Control: no-cache" + + rxreq + expect req.http.user-agent == c6 + expect req.http.bgfetch == false + txresp -hdr "Cache-Control: no-cache" +} -start + +varnish v1 -cliok "param.set default_grace 1h" +varnish v1 -cliok "param.set thread_pools 1" +varnish v1 -cliok "param.set debug +syncvsl,+waitinglist" +varnish v1 -vcl+backend { + sub vcl_backend_fetch { + set bereq.http.bgfetch = bereq.is_bgfetch; + } + sub vcl_beresp_stale { + # We just validated a stale object, do not mark it as + # uncacheable. The object remains available for grace + # hits and background fetches. + return; + } + sub vcl_beresp_control { + if (beresp.http.cache-control == "no-cache") { + # Keep beresp.uncacheable clear. + return; + } + } + sub vcl_deliver { + set resp.http.obj-hits = obj.hits; + set resp.http.obj-ttl = obj.ttl; + } +} -start + +client c1 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1001 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl < 0 +} -start + +barrier b1 sync + +client c2 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1004 1002" + expect resp.http.obj-hits == 1 + expect resp.http.obj-ttl < 0 +} -start + +varnish v1 -expect busy_sleep == 1 +barrier b2 sync + +client c1 -wait +client c2 -wait + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 1 +varnish v1 -expect cache_hit == 1 +varnish v1 -expect cache_hit_grace == 0 +varnish v1 -expect s_bgfetch == 0 + +client c3 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1006 1002" + expect resp.http.obj-hits == 2 + expect resp.http.obj-ttl < 0 +} -run + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 1 +varnish v1 -expect cache_hit == 2 +varnish v1 -expect cache_hit_grace == 1 +varnish v1 -expect s_bgfetch == 1 + +# The only way for a plain no-cache to be hit is to have a non-zero keep. +varnish v1 -cliok "param.set default_ttl 0" +varnish v1 -cliok "param.set default_grace 0" +varnish v1 -cliok "param.set default_keep 1h" + +client c4 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1009 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl <= 0 +} -start + +barrier b3 sync + +client c5 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1012 1010" + expect resp.http.obj-hits == 1 + expect resp.http.obj-ttl <= 0 +} -start + +varnish v1 -expect busy_sleep == 2 +barrier b4 sync + +client c4 -wait +client c5 -wait + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 2 +varnish v1 -expect cache_hit == 3 +varnish v1 -expect cache_hit_grace == 1 +varnish v1 -expect s_bgfetch == 1 + +# No hit when not on the waiting list +client c6 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1014 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl <= 0 +} -run diff --git a/bin/varnishtest/tests/c00131.vtc b/bin/varnishtest/tests/c00131.vtc new file mode 100644 index 0000000000..c9ce19bd58 --- /dev/null +++ b/bin/varnishtest/tests/c00131.vtc @@ -0,0 +1,71 @@ +varnishtest "waiting list rush from vcl_backend_error" + +barrier b1 sock 2 +barrier b2 sock 2 + +server s1 { + rxreq + send garbage +} -start + +varnish v1 -cliok "param.set debug +syncvsl,+waitinglist" +varnish v1 -vcl+backend { + import vtc; + sub vcl_backend_fetch { + vtc.barrier_sync("${b1_sock}"); + vtc.barrier_sync("${b2_sock}"); + } + sub vcl_backend_error { + set beresp.http.error-vxid = bereq.xid; + } +} -start + +client c1 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +barrier b1 sync + +logexpect l1 -v v1 -q Debug -g raw { + loop 4 { + expect * * Debug "on waiting list" + } +} -start + +client c2 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c3 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c4 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +client c5 { + txreq + rxresp + expect resp.http.error-vxid == 1002 +} -start + +logexpect l1 -wait + +barrier b2 sync + +client c1 -wait +client c2 -wait +client c3 -wait +client c4 -wait +client c5 -wait + +varnish v1 -expect n_expired == 1 diff --git a/bin/varnishtest/tests/c00133.vtc b/bin/varnishtest/tests/c00133.vtc new file mode 100644 index 0000000000..cf0cb8c198 --- /dev/null +++ b/bin/varnishtest/tests/c00133.vtc @@ -0,0 +1,64 @@ +varnishtest "subsequent rush on rush miss" + +barrier b1 cond 2 +barrier b2 cond 2 +barrier b3 cond 2 +barrier b4 cond 2 + +server s1 { + rxreq + expect req.http.user-agent == c1 + barrier b1 sync + barrier b2 sync + txresp -hdr "Vary: accept" + + rxreq + expect req.http.user-agent == c2 + txresp -hdr "Vary: accept" +} -start + +varnish v1 -cliok "param.set thread_pools 1" +varnish v1 -cliok "param.set rush_exponent 1" +varnish v1 -cliok "param.set debug +syncvsl,+waitinglist" +varnish v1 -vcl+backend "" -start + +client c1 { + txreq + rxresp + expect resp.http.x-varnish == 1001 +} -start + +barrier b1 sync + +logexpect l1 -v v1 -q Debug -g raw { + expect * * Debug "on waiting list" +} -start + +client c2 { + txreq -hdr "accept: nothing" + rxresp + expect resp.http.x-varnish == 1004 +} -start + +logexpect l1 -wait -start + +client c3 { + txreq + rxresp + expect resp.http.x-varnish == "1006 1002" +} -start + +logexpect l1 -wait + +barrier b2 sync + +client c1 -wait +client c2 -wait +client c3 -wait + +varnish v1 -expect cache_miss == 2 +varnish v1 -expect cache_hit == 1 +varnish v1 -expect cache_hit_grace == 0 +varnish v1 -expect s_bgfetch == 0 +varnish v1 -expect busy_sleep == 2 +varnish v1 -expect busy_wakeup == 2 diff --git a/bin/varnishtest/tests/r02422.vtc b/bin/varnishtest/tests/r02422.vtc index 9c956f5666..7a51689d26 100644 --- a/bin/varnishtest/tests/r02422.vtc +++ b/bin/varnishtest/tests/r02422.vtc @@ -1,14 +1,24 @@ varnishtest "long polling and low latency using req.ttl" +# synchronizes the setup of all clients +barrier b1 cond 2 + +# makes c1..cN send their requests simultaneously-ish +barrier b2 cond 7 + +# synchronizes c4 with c5 and c6 +barrier b3 cond 3 + server s1 { # s1 uses the etag as a version rxreq - txresp -hdr "Etag: 5" + txresp -hdr {Etag: "5"} rxreq + barrier b3 sync # wait until the new version is ready delay 1 - txresp -hdr "Etag: 6" + txresp -hdr {Etag: "6"} } -start varnish v1 -cliok "param.set default_grace 0" @@ -30,12 +40,6 @@ varnish v1 -vcl+backend { } } -start -# synchronizes the setup of all clients -barrier b1 cond 2 - -# makes c1..cN send their requests simultaneously-ish -barrier b2 cond 7 - client c0 { # wait for all clients to be ready barrier b1 sync @@ -43,7 +47,7 @@ client c0 { txreq rxresp expect resp.status == 200 - expect resp.http.ETag == 5 + expect resp.http.ETag == {"5"} expect resp.http.Hit == false # let all other clients send their requests barrier b2 sync @@ -55,37 +59,37 @@ client c1 { txreq rxresp expect resp.status == 200 - expect resp.http.ETag == 5 + expect resp.http.ETag == {"5"} expect resp.http.Hit == true } -start client c2 { # late client, immediate hit barrier b2 sync - txreq -hdr "If-None-Match: 2" + txreq -hdr {If-None-Match: "2"} rxresp expect resp.status == 200 - expect resp.http.ETag == 5 + expect resp.http.ETag == {"5"} expect resp.http.Hit == true } -start client c3 { # late client, immediate hit barrier b2 sync - txreq -hdr "If-None-Match: 4" + txreq -hdr {If-None-Match: "4"} rxresp expect resp.status == 200 - expect resp.http.ETag == 5 + expect resp.http.ETag == {"5"} expect resp.http.Hit == true } -start client c4 { # up-to-date client, long polling barrier b2 sync - txreq -hdr "If-None-Match: 5" + txreq -hdr {If-None-Match: "5"} rxresp expect resp.status == 200 - expect resp.http.ETag == 6 + expect resp.http.ETag == {"6"} expect resp.http.Hit == false } -start @@ -93,11 +97,11 @@ client c5 { # up-to-date client, long polling barrier b2 sync # wait to make sure c4 gets the miss - delay 0.2 - txreq -hdr "If-None-Match: 5" + barrier b3 sync + txreq -hdr {If-None-Match: "5"} rxresp expect resp.status == 200 - expect resp.http.ETag == 6 + expect resp.http.ETag == {"6"} expect resp.http.Hit == true } -start @@ -105,11 +109,11 @@ client c6 { # up-to-date client, long polling barrier b2 sync # wait to make sure c4 gets the miss - delay 0.2 - txreq -hdr "If-None-Match: 5" + barrier b3 sync + txreq -hdr {If-None-Match: "5"} rxresp expect resp.status == 200 - expect resp.http.ETag == 6 + expect resp.http.ETag == {"6"} expect resp.http.Hit == true } -start diff --git a/bin/varnishtest/vtc.c b/bin/varnishtest/vtc.c index 80f04b5011..a5cd68ee2d 100644 --- a/bin/varnishtest/vtc.c +++ b/bin/varnishtest/vtc.c @@ -373,8 +373,11 @@ parse_string(struct vtclog *vl, void *priv, const char *spec) e = strchr(buf, '\0'); AN(e); for (p = buf; p < e; p++) { - if (vtc_error || vtc_stop) + if (vtc_error || vtc_stop) { + vtc_log(vl, 1, "Aborting execution, test %s", + vtc_error ? "failed" : "ended"); break; + } /* Start of line */ if (isspace(*p)) continue; @@ -484,7 +487,7 @@ parse_string(struct vtclog *vl, void *priv, const char *spec) if (!strcmp(token_s[0], "loop")) { n = strtoul(token_s[1], NULL, 0); - for (m = 0; m < n; m++) { + for (m = 0; m < n && !vtc_error && !vtc_stop; m++) { vtc_log(vl, 4, "Loop #%u", m); parse_string(vl, priv, token_s[2]); } diff --git a/bin/varnishtest/vtc_server.c b/bin/varnishtest/vtc_server.c index 45913aaa7d..5de00896a9 100644 --- a/bin/varnishtest/vtc_server.c +++ b/bin/varnishtest/vtc_server.c @@ -265,7 +265,7 @@ server_disc(void *priv, struct vtclog *vl, int *fdp) struct server *s; CAST_OBJ_NOTNULL(s, priv, SERVER_MAGIC); - vtc_log(vl, 3, "shutting fd %d", *fdp); + vtc_log(vl, 3, "shutting fd %d (server run)", *fdp); j = shutdown(*fdp, SHUT_WR); if (!vtc_stop && !VTCP_Check(j)) vtc_fatal(vl, "Shutdown failed: %s", strerror(errno)); @@ -322,7 +322,7 @@ server_dispatch_wrk(void *priv) vtc_log(vl, 3, "start with fd %d", fd); fd = sess_process(vl, s->vsp, s->spec, fd, &s->sock, s->listen); - vtc_log(vl, 3, "shutting fd %d", fd); + vtc_log(vl, 3, "shutting fd %d (server dispatch)", fd); j = shutdown(fd, SHUT_WR); if (!VTCP_Check(j)) vtc_fatal(vl, "Shutdown failed: %s", strerror(errno)); diff --git a/doc/sphinx/reference/vcl_step.rst b/doc/sphinx/reference/vcl_step.rst index ee07f61692..86fad6312a 100644 --- a/doc/sphinx/reference/vcl_step.rst +++ b/doc/sphinx/reference/vcl_step.rst @@ -378,19 +378,14 @@ This subroutine is called if we fail the backend fetch or if Returning with :ref:`abandon` does not leave a cache object. -If returning with ``deliver`` and a ``beresp.ttl > 0s``, a synthetic -cache object is generated in VCL, whose body may be constructed using -the ``synthetic()`` function. - -When there is a waiting list on the object, the default ``ttl`` will -be positive (currently one second), set before entering -``vcl_backend_error``. This is to avoid request serialization and -hammering on a potentially failing backend. - -Since these synthetic objects are cached in these special -circumstances, be cautious with putting private information there. If -you really must, then you need to explicitly set ``beresp.ttl`` to -zero in ``vcl_backend_error``. +If returning with ``deliver`` and ``beresp.uncacheable == false``, a +synthetic cache object is generated in VCL, whose body may be constructed +using the ``synthetic()`` function. + +Since these synthetic objects are cached in these special circumstances, +be cautious with putting private information there. If you really must, +then you need to explicitly set ``beresp.uncacheable`` to ``true`` in +``vcl_backend_error``. The `vcl_backend_error` subroutine may terminate with calling ``return()`` with one of the following keywords: diff --git a/include/tbl/boc_state.h b/include/tbl/boc_state.h index 44984de81b..2e04136606 100644 --- a/include/tbl/boc_state.h +++ b/include/tbl/boc_state.h @@ -32,7 +32,6 @@ BOC_STATE(INVALID, invalid) /* don't touch (yet) */ BOC_STATE(REQ_DONE, req_done) /* bereq.* can be examined */ -BOC_STATE(PREP_STREAM, prep_stream) /* Prepare for streaming */ BOC_STATE(STREAM, stream) /* beresp.* can be examined */ BOC_STATE(FINISHED, finished) /* object is complete */ BOC_STATE(FAILED, failed) /* something went wrong */ diff --git a/include/tbl/oc_flags.h b/include/tbl/oc_flags.h index 768962fdae..0866cc38f0 100644 --- a/include/tbl/oc_flags.h +++ b/include/tbl/oc_flags.h @@ -30,7 +30,8 @@ /*lint -save -e525 -e539 */ -OC_FLAG(BUSY, busy, (1<<1)) //lint !e835 +OC_FLAG(WITHDRAWN, withdrawn, (1<<0)) //lint !e835 +OC_FLAG(BUSY, busy, (1<<1)) OC_FLAG(HFM, hfm, (1<<2)) OC_FLAG(HFP, hfp, (1<<3)) OC_FLAG(CANCEL, cancel, (1<<4)) diff --git a/include/tbl/params.h b/include/tbl/params.h index 7c093baa95..0667414580 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -858,7 +858,7 @@ PARAM_SIMPLE( PARAM_SIMPLE( /* name */ rush_exponent, /* type */ uint, - /* min */ "2", + /* min */ "1", /* max */ NULL, /* def */ "3", /* units */ "requests per request", @@ -866,7 +866,9 @@ PARAM_SIMPLE( "How many parked request we start for each completed request on " "the object.\n" "NB: Even with the implicit delay of delivery, this parameter " - "controls an exponential increase in number of worker threads.", + "controls an exponential increase in number of worker threads. " + "A value of 1 will instead serialize requests resumption and is " + "only useful for testing purposes.", /* flags */ EXPERIMENTAL ) diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h index 6a9e6acbb1..15a1b430e6 100644 --- a/include/tbl/req_flags.h +++ b/include/tbl/req_flags.h @@ -37,7 +37,6 @@ REQ_FLAG(hash_ignore_busy, 1, 1, "") REQ_FLAG(hash_ignore_vary, 1, 1, "") REQ_FLAG(hash_always_miss, 1, 1, "") REQ_FLAG(is_hit, 0, 0, "") -REQ_FLAG(waitinglist, 0, 0, "") REQ_FLAG(want100cont, 0, 0, "") REQ_FLAG(late100cont, 0, 0, "") REQ_FLAG(req_reset, 0, 0, "")