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

Dridi Boukelmoune dridi at varni.sh
Sat Sep 12 13:52:35 CEST 2015


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).

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).

> 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.



More information about the varnish-dev mailing list