Skip to content

Commit

Permalink
give all requests a struct reqtop
Browse files Browse the repository at this point in the history
Dynamically creating it through Req_MakeTop() would further complicate
rollbacks.

The memory overhead is basically identical to embedding struct reqtop
into struct req, except that, for ESI, we have the (struct req).top
point to the top request's struct reqtop.

With this commit, tests/r02849.vtc and tests/r03003.vtc are failing as
excpected. While this may impose issues with git bisect, I still think
that this extra commit helps clarity.
  • Loading branch information
nigoroll committed Nov 13, 2019
1 parent a8e3449 commit 244705b
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 38 deletions.
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ struct req {
struct vcf *vcf;
};

#define IS_TOPREQ(req) ((req)->top == NULL || (req)->top->topreq == (req))
#define IS_TOPREQ(req) ((req)->top->topreq == (req))

/*--------------------------------------------------------------------
* Struct sess is a high memory-load structure because sessions typically
Expand Down
11 changes: 1 addition & 10 deletions bin/varnishd/cache/cache_esi_deliver.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ ved_include(struct req *preq, const char *src, const char *host,

req->esi_level = preq->esi_level + 1;

memset(req->top, 0, sizeof *req->top);
req->top = preq->top;

HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);
Expand Down Expand Up @@ -264,16 +265,6 @@ ved_vdp_esi_init(struct req *req, void **priv)
*priv = ecx;
RFC2616_Weaken_Etag(req->resp);

if (IS_TOPREQ(req)) {
Req_MakeTop(req);
if (req->top == NULL) {
VSLb(req->vsl, SLT_Error,
"(top)request workspace overflow");
Req_Fail(req, SC_OVERLOAD);
return (-1);
}
}

req->res_mode |= RES_ESI;
if (req->resp_len != 0)
req->resp_len = -1;
Expand Down
25 changes: 5 additions & 20 deletions bin/varnishd/cache/cache_req.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ Req_New(const struct worker *wrk, struct sess *sp)
INIT_OBJ(req->htc, HTTP_CONN_MAGIC);
p = (void*)PRNDUP(p + sizeof(*req->htc));

req->top = (void*)p;
INIT_OBJ(req->top, REQTOP_MAGIC);
req->top->topreq = req;
p = (void*)PRNDUP(p + sizeof(*req->top));

assert(p < e);

WS_Init(req->ws, "req", p, e - p);
Expand Down Expand Up @@ -170,7 +175,6 @@ Req_Release(struct req *req)
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
MPL_AssertSane(req);
VSL_Flush(req->vsl, 0);
req->top = NULL;
MPL_Free(pp->mpl_req, req);
}

Expand All @@ -189,7 +193,6 @@ Req_Rollback(struct req *req)
if (WS_Overflowed(req->ws))
req->wrk->stats->ws_client_overflow++;
WS_Reset(req->ws, req->ws_req);
req->top = NULL;
}

/*----------------------------------------------------------------------
Expand Down Expand Up @@ -235,7 +238,6 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
req->hash_ignore_busy = 0;
req->esi_level = 0;
req->is_hit = 0;
req->top = 0;

if (WS_Overflowed(req->ws))
wrk->stats->ws_client_overflow++;
Expand All @@ -254,20 +256,3 @@ Req_Fail(struct req *req, enum sess_close reason)
AN(req->transport->req_fail);
req->transport->req_fail(req, reason);
}

/*----------------------------------------------------------------------
*/

void
Req_MakeTop(struct req *req)
{

CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
if (req->top != NULL)
return;
req->top = WS_Alloc(req->ws, sizeof *req->top);
if (req->top != NULL) {
INIT_OBJ(req->top, REQTOP_MAGIC);
req->top->topreq = req;
}
}
1 change: 0 additions & 1 deletion bin/varnishd/cache/cache_varnishd.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ void Req_Rollback(struct req *req);
void Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req);
void Req_Fail(struct req *req, enum sess_close reason);
void Req_AcctLogCharge(struct VSC_main_wrk *, struct req *);
void Req_MakeTop(struct req *req);

/* cache_req_body.c */
int VRB_Ignore(struct req *);
Expand Down
7 changes: 2 additions & 5 deletions bin/varnishd/cache/cache_vcl.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,8 @@ VCL_Req2Ctx(struct vrt_ctx *ctx, struct req *req)
ctx->vcl = req->vcl;
ctx->vsl = req->vsl;
ctx->http_req = req->http;
CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
if (req->top != NULL)
ctx->http_req_top = req->top->topreq->http;
else
ctx->http_req_top = req->http;
CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
ctx->http_req_top = req->top->topreq->http;
ctx->http_resp = req->resp;
ctx->req = req;
ctx->sp = req->sp;
Expand Down
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_vrt_vcl.c
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
CHECK_OBJ(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC);
CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
VCL_Req2Ctx(&ctx, req);
}
if (bo != NULL) {
Expand Down

0 comments on commit 244705b

Please sign in to comment.