VCL storage discussion summary

[Dridi Boukelmoune]

Hi,

Apologies for coming back late on this topic. After some discussions
on IRC I realized I should mention that my suggestions were
non-breaking because the next major is supposedly "only" a 5.1 so I
figured we'd maintain VCL compatibility.

I'm not against removing beresp.storage_hint as long as we have an
alternative like STEVEDORE std.storage_hint(STRING[, STEVEDORE]). I
believe Nils is on the same page regarding this point.

On Mon, Oct 10, 2016 at 9:18 PM, Federico Schwindt <fgsch at lodoss.net> wrote:
> What about something like this:
>
> - NULL or "no setting" means default storage (this is the same as no
> backend).

It may be the case for req.backend_hint, but it isn't for
bereq.backend. That would be where I draw the line with a hint: it'd
have a graceful fallback.


On Wed, Oct 12, 2016 at 10:32 AM, Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
> --------
> In message <CAJV_h0YEqgoh9b+qrBVW05CeRezr0Etq5Zkv8y4n19OxT15wLQ at mail.gmail.com>
> , Federico Schwindt writes:
>
>>In other words, for cache insertions default / no setting means RR and NULL
>>means failure?
>
> yes.

How about aligning on backends, and make the first storage (besides
Transient) be the default unless there's a stevedore actually called
`default'? RR storage selection could be moved to std and be made
explicit. That is fine if we rename beresp.storage_hint to just
beresp.storage and back the change with proper docs.


On Tue, Oct 11, 2016 at 10:12 PM, Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
> Having read the thread again, I suggest:
>
>         Before vcl_backend_response{} we point beresp->storage to the next
>         stevedore RR-wise. (Today we do it later, because it is a hint).

Or to a default, see above.

>         If at the end of v_b_r{} beresp->storage is non-NULL, we use that
>         stevedore and only that stevedore.

Agreed.

>         If at the end of v_b_r{} beresp->storage is NULL, we take it to
>         mean storage failure and we 50x.

Agreed.

>         That means you can still salvage inside v_b_r{}:
>
>                 beresp->stevedore = vmod.forklift();
>                 if (!beresp->stevedore) {
>                         beresp->stevedore = Transient;
>                         beresp->uncacheable = True;
>                 }
>
>         For compatibility, if VCL sets beresp->storage_hint, and the string
>         is a stevedore name, we *also* set beresp->storage, but on failure
>         we leave it alone.

I would rather retire beresp.storage_hint as soon as beresp.storage
lands and avoid confusion between both.

> I belive that gives the semantics you desire in a POLA compliant fashion ?

Moving RR selection to std would be even more POLA compliant IMHO.

Cheers,
Dridi