[PATCH] Log 'set beresp.uncacheable = true' occurrences

[Federico Schwindt]

On Sat, Sep 12, 2015 at 12:52 PM, Dridi Boukelmoune <dridi at varni.sh> wrote:

> On Sat, Sep 12, 2015 at 12:55 PM, Federico Schwindt <fgsch at lodoss.net>
> wrote:
> > Since nitpicking is my name:
> >
> >> +                       VSLb(bo->vsl, SLT_Uncacheable, "Illegal Vary
> >> header");
> >
> > Before this there is a SLT_Error with "Illegal 'Vary' header from
> backend,
> > making this a pass.". Repeating the reason here seems redundant.
>
> One is an error, the other is informing you that beresp.uncacheable is
> set to true (and why).
>

Fair enough.


>
> If we instead follow phk's suggestion we could remove said duplication
> by having something like:
> VCL_set beresp.uncacheable true
>
> >> +       if (bo->do_pass) {
> >> +               bo->uncacheable = 1;
> >> +               VSLb(bo->vsl, SLT_Uncacheable, "bereq");
> >> +       }
> >
> > We could argue that this is uncacheable at the time we returned pass.
> Also
> > "bereq" doesn't really tell us anything.
>
> It tells you that beresp.uncacheable is true because of the bereq. So
> obviously either the log record or the docs need to be improved, or
> again, phk's suggestion would make things simpler (for the user).
>

Still doesn't say a thing to me. The pass happens in recv (client side) not
in bereq (backend side).
Explaining obscure things in the doc is not the right way to go. If
anything it should complement an already well explained situation.


>
> > Furthermore, doing it in here will result in this being logged twice in
> some
> > cases.
>
> Yes, but it is logged for different reasons, and actually set twice,
> so I'd rather see it logged twice for consistency.
>

I'd rather move it where it belongs, that is where do_pass is set to 1.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-dev/attachments/20150912/1b43e6bc/attachment.html>