On Mon, Jan 11, 2016 at 3:54 PM, Poul-Henning Kamp <p...@freebsd.org> wrote:
>
> commit f4895da5b08b83c2190ff96e2a6364e4699e962f
> Author: Poul-Henning Kamp <p...@freebsd.org>
> Date: Mon Jan 11 14:53:48 2016 +0000
>
> Split VCL event sender functions out individually.
>
> Dridi: Not XXX comments
Please find answers below XXX comments.
> static int
> -vcl_setup_event(VRT_CTX, enum vcl_event_e ev)
> +vcl_event_warm(VRT_CTX)
> {
> -
> CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
> AN(ctx->handling);
> AN(ctx->vcl);
> - assert(ev == VCL_EVENT_LOAD || ev == VCL_EVENT_WARM ||
> - ev == VCL_EVENT_USE);
> -
> - if (ev == VCL_EVENT_LOAD)
> - AN(ctx->msg);
> + // AN/AZ(ctx->msg); // XXX: Dridi: which is it ?
> + return (ctx->vcl->conf->event_vcl(ctx, VCL_EVENT_WARM));
> +}
Currently only the LOAD event can fail so I check the availability of
ctx->msg only in this case, so it's AN.
> static void
> -vcl_failsafe_event(VRT_CTX, enum vcl_event_e ev)
> +vcl_event_cold(VRT_CTX)
> {
> + CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
> + AN(ctx->handling);
> + AN(ctx->vcl);
> + AZ(ctx->msg);
> + AZ(ctx->vcl->conf->event_vcl(ctx, VCL_EVENT_COLD));
> +}
During the VDD15Q4 discussions you asked me to use the WRONG macro to
provide a human-friendly message instead of using AZ.
> +static void
> +vcl_event_discard(VRT_CTX)
> +{
> CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
> AN(ctx->handling);
> AN(ctx->vcl);
> - assert(ev == VCL_EVENT_COLD || ev == VCL_EVENT_DISCARD);
> + // AN/AZ(ctx->msg); // XXX: Dridi: which is it ?
> + AZ(ctx->vcl->conf->event_vcl(ctx, VCL_EVENT_DISCARD));
> +}
I did not test ctx->msg in the previous vcl_failsafe_event function
because VMODs are not supposed to either fail or talk back to the CLI
for these events.
I'd say AZ but again, I'm not sure all code paths to cold/dispatch
event handling *don't* allocate a vsb.
> /* The VCL must first reach a stable cold state */
> else if (vcl->temp != VCL_TEMP_COOLING) {
> vcl->temp = VCL_TEMP_WARM;
> - (void)vcl_setup_event(ctx, VCL_EVENT_WARM);
> + vcl_event_warm(ctx); // XXX: Dridi: what if it
> fails?
> vcl_BackendEvent(vcl, VCL_EVENT_WARM);
> }
> break;
It doesn't currently fail. This is handled in the next patch from the
pending review we didn't have time to finish.
As I said in today's email, this commit is only part of the prep work
for fixing VCL temperature in 4.1 and master. It only breaks down code
before the changes that will actually fix known issues and changes in
behavior decided during the last two VDDs. The single decided change I
didn't apply in the patch-set is the altogether removal of the USE
event because this is meant for 4.1.x too.
With regards to splitting the event dispatching into 5 different
functions, I didn't see much value since I essentially see two kinds
of events: the setup ones, and the ones that must not fail.
Best,
Dridi
_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
