Re: VCL storage discussion summary

[Federico Schwindt]

OK, all makes sense now.

I will work on the PR.

On Wed, Oct 12, 2016 at 9:32 AM, Poul-Henning Kamp 
wrote:

> 
> In message  15...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >In other words, for cache insertions default / no setting means RR and
> NULL
> >means failure?
>
> yes.
>
> >I believe that'd work.
>
> >About this:
> >
> >> If at the end of v_b_r{} beresp->storage is non-NULL, we use that
> stevedore
> >and only that stevedore.
> >
> >If someone sets the wrong storage, because e.g. they had a typo, we will
> >fail the request or fallback to Transient?
>
> beresp->storage is typed and type-checked, so if you have a typo,
> the compiler complains unless you happen to name one of your other
> stevedores with the typo.
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Dridi Boukelmoune writes:

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

We can do that at the next major rev.


-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: 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  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  wrote:
> 
> In message 
> 
> , 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  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

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

In other words, for cache insertions default / no setting means RR and NULL
means failure?

I believe that'd work.

About this:

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

If someone sets the wrong storage, because e.g. they had a typo, we will
fail the request or fallback to Transient?


On Tue, Oct 11, 2016 at 9:12 PM, Poul-Henning Kamp 
wrote:

> 
> In message  mail.gmail.com>
> , Federico Schwindt writes:
>
> >Regardless of the RR, any comments or guidelines to move this forward?
>
> 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).
>
> If at the end of v_b_r{} beresp->storage is non-NULL, we use that
> stevedore and only that stevedore.
>
> If at the end of v_b_r{} beresp->storage is NULL, we take it to
> mean storage failure and we 50x.
>
> 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 belive that gives the semantics you desire in a POLA compliant fashion ?
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Federico Schwindt writes:

>Regardless of the RR, any comments or guidelines to move this forward?

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

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

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

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 belive that gives the semantics you desire in a POLA compliant fashion ?

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

Regardless of the RR, any comments or guidelines to move this forward?

Should we stick to the _hint and gc in the next major release?

On Mon, Oct 10, 2016 at 11:19 PM, Federico Schwindt 
wrote:

> In my experience setups defining multiple storages are few and they use
> them explicitly (in VCL).
>
> While I'm not necessarily advocating this change I think this will be
> closer to how someone would expect it to work.
> Waiting for the next major release and documenting the change might do the
> trick.
>
> On Mon, Oct 10, 2016 at 8:43 PM, Poul-Henning Kamp 
> wrote:
>
>> 
>> In message 

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 

Re: VCL storage discussion summary

[Federico Schwindt]

Why? Is there anyone depending on this feature? When do you want to use it?
Wouldn't be easier to visualise and/or explain what is going where if it's
done explicitly?
Also, if we allow this shouldn't be a way to disable it?

My problem with this is two fold: 1, it's not documented AFAICT; and 2.
people specifying the wrong storage by mistake ends up using another
storage and causing evictions (seen this in a customer).

Since the definition and usage is done separately, it is not that difficult
to get it wrong, specially when you change one but forget to update the
other.

This also means that we cannot say that default means the first
non-Transient storage for cache insertions.



On Mon, Oct 10, 2016 at 8:23 PM, Poul-Henning Kamp 
wrote:

> 
> In message  g5px0xcd6ukqn+a...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >- No more RR on storages by default. If this is wanted, it should be set
> >explicitly somehow (VMOD?)
>
> I agree with the rest, but this on I think would be unwise, we should
> retain the RR behaviour.
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Federico Schwindt writes:

>- No more RR on storages by default. If this is wanted, it should be set
>explicitly somehow (VMOD?)

I agree with the rest, but this on I think would be unwise, we should
retain the RR behaviour.

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

What about something like this:

- NULL or "no setting" means default storage (this is the same as no
backend).
- default means the first non-Transient storage for cache insertions,
Transient for anything else (pass and request bodies).
- No more RR on storages by default. If this is wanted, it should be set
explicitly somehow (VMOD?)

If we failed to allocate using the selected storage we fallback to
Transient, always.
If Transient fails because it's capped and we couldn't make space, we fail
hard.

Comments?

On Mon, Oct 10, 2016 at 6:42 PM, Poul-Henning Kamp 
wrote:

> 
> In message  rtqa686ak...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >This might be obvious but are we considering NULL a "no setting" as well
> or
> >only when a VMOD returns NULL?
> >
> >If it's the latter I agree. The former would be a breaking change.
>
> Well, that's why I'm asking:
>
> If vmod returns NULL does it mean
>
> A) Failure to select stevedore
>
> B) Use default stevedore selection
>
> ?
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Federico Schwindt writes:

>This might be obvious but are we considering NULL a "no setting" as well or
>only when a VMOD returns NULL?
>
>If it's the latter I agree. The former would be a breaking change.

Well, that's why I'm asking:

If vmod returns NULL does it mean

A) Failure to select stevedore

B) Use default stevedore selection

?

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

This might be obvious but are we considering NULL a "no setting" as well or
only when a VMOD returns NULL?

If it's the latter I agree. The former would be a breaking change.

On Mon, Oct 10, 2016 at 6:33 PM, Poul-Henning Kamp 
wrote:

> 
> In message  gg88cel9uv-k7...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >Doesn't that depend on whether is a cache insertion or pass?
> >
> >Are we considering changing that?
>
> If we have NULL for stevedore and it is an insert, I would consider
> making it pass+Transient, to limit the amount of non-placed storage
> we need.
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Federico Schwindt writes:

>Doesn't that depend on whether is a cache insertion or pass?
>
>Are we considering changing that?

If we have NULL for stevedore and it is an insert, I would consider
making it pass+Transient, to limit the amount of non-placed storage
we need.

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

Doesn't that depend on whether is a cache insertion or pass?

Are we considering changing that?



On Mon, Oct 10, 2016 at 6:08 PM, Poul-Henning Kamp 
wrote:

> 
> In message  cy_rr2xju87ui...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >Assuming this will depend on whether this is the client or the backend
> side
> >(request vs response), yes,
> >
> >for the request body Transient being the default makes sense (current
> >behaviour).
>
> You left out what will make sense on backend side ?
>
> I don't see an alternative to Transient, and I might even advocate pass...
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

Assuming this will depend on whether this is the client or the backend side
(request vs response), yes,
for the request body Transient being the default makes sense (current
behaviour).




On Mon, Oct 10, 2016 at 5:55 PM, Poul-Henning Kamp 
wrote:

> 
> In message  ctatgu2qwftqv...@mail.gmail.com>
> , Federico Schwindt writes:
>
> >If setting or using a storage cannot fail I'd say kill _hint (eventually)
> >and just use storage.
> >
> >If there is a chance of failing, I'd keep the _hint. I'd avoid having
> both.
> >Having backend_hint and backend is confusing enough already.
>
> I agree, and I'd like to kill backend_hint as well.
>
> That said, in both cases we need to deal with "no setting" and "vmod
> returning NULL".
>
> For backend, it's the default backend.
>
> Is it Transient for storage ?
>
> If so, I think we have a decision...
>
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Poul-Henning Kamp]

In message 
, Federico Schwindt writes:

>If setting or using a storage cannot fail I'd say kill _hint (eventually)
>and just use storage.
>
>If there is a chance of failing, I'd keep the _hint. I'd avoid having both.
>Having backend_hint and backend is confusing enough already.

I agree, and I'd like to kill backend_hint as well.

That said, in both cases we need to deal with "no setting" and "vmod
returning NULL".

For backend, it's the default backend.

Is it Transient for storage ?

If so, I think we have a decision...


-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
p...@freebsd.org | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VCL storage discussion summary

[Federico Schwindt]

[ And now replying to all.. ]

Hi,

If setting or using a storage cannot fail I'd say kill _hint (eventually)
and just use storage.

If there is a chance of failing, I'd keep the _hint. I'd avoid having both.
Having backend_hint and backend is confusing enough already.

Regardless of the outcome, I think we should be more explicit on how tell
tell Varnish any storage (or the next storage for the matter) is OK.
Right now this happens under the hood and I've seen people setting the
wrong storage by mistake (typos), not realising it and asking why Transient
or some other storage is full.

Best.

On Mon, Oct 10, 2016 at 1:30 PM, Dridi Boukelmoune  wrote:

> Hello,
>
> Following discussions on storage/stevedore literals vs hints in VCL
> I've tried to sum up the current situation and suggest a direction to
> take.
>
> Currently we have:
>
> 1) a STEVEDORE type for VCL (storage namespace)
> 2) all storages resolved at boot time (typesafe)
> 3) beresp.storage_hint takes a STRING
> 4) no/wrong hint means round-robin among storages
>
> Today during the bugwash, following two pull requests introducing a
> storage hint for the request body (instead of systematically using
> Transient) we discussed the possibility of removing the hint part. The
> reason being 1) in the list above.
>
> After some testing on master, it doesn't seem to be enforced:
>
> sub vcl_backend_response {
> set beresp.storage_hint = "some random junk";
> set beresp.storage_hint = beresp.http.x-storage;
> }
>
> However, implicit stevedore conversion to string exists:
>
> sub vcl_backend_response {
> set beresp.storage_hint = storage.Transient;
> }
>
> Suggestions:
>
> A) Keep the _hint to allow backend- or vmod-driven _loose_ storage
>selection.
>
> B) Maybe introduce besresp.storage to avoid conversions to and from
>STRING, but allow NULL to behave the same as 4) for the _hint.
>
> C) Be consistent when storage selection is introduced for the request body.
>
> Cheers,
> Dridi
>
> ___
> varnish-dev mailing list
> varnish-dev@varnish-cache.org
> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev