Mailing List Archive

Reintroduce a req.uncacheable flag
Greetings,

I would like to reopen a discussion started by Guillaume via a pull
request:

https://github.com/varnishcache/varnish-cache/pull/3457

For reasons other than the ones he exposed I have reached the
conclusion that such a flag would be both relevant and useful. Not
req.hash_always_pass as originally suggested but req.uncacheable
(and to Guillaume's credit he did make the suggestion first in the pull
request description).

I didn't take part in the pull request discussion and don't remember
if I took a position but chances are I would have opposed a flag
called hash_always_pass. On the other hand, here are the reasons why I
think req.uncacheable would make sense.

First, we already have a relationship between bereq.uncacheable and
bereq.uncacheable: the former implies the latter. I believe the same
relationship could exist between req and bereq.

Second, since the VCL built-in split it makes even more sense to get
the ability to mark a request as uncacheable. If you wish to keep your
code organized as per the split, but don't want an early return to
prevent a later step to be taken, then a flag instead of a
return(pass) lets the flow of execution continue and guarantees that a
lookup attempt would result in a pass transition.

As we are heading towards a dot-zero release, we could even break the
built-in VCL to replace pass transitions with req flagging. That would
be a breaking change in the sense that we would no longer break the
flow of execution, but in practice that does not change the outcome of
existing VCL not taking advantage of the split (for example upgrading
from 6.0 LTS) and for VCL authors who already adopted the split, I
assume they'd be savvy enough to check their code.

Third, one more reason to bring it back as req.uncacheable instead of
hash_always_pass is consistency across the board.

I'm not diving into implementation details on purpose, my goal with
this thread is to discuss the possibility to reintroduce a new
iteration on #3457 and shine a new light on it from the perspective
of the recent split and the additional opportunity it grants us.

Cheers,
Dridi
_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
Re: Reintroduce a req.uncacheable flag [ In reply to ]
On 5/28/21 17:50, Dridi Boukelmoune wrote:
>
> First, we already have a relationship between bereq.uncacheable and
> bereq.uncacheable: the former implies the latter.

Presumably you meant to write that bereq.uncacheable implies
beresp.uncacheable.

My quick and superficial first reaction to the proposal -- I don't quite
have my head around what you would write in VCL if you do in fact want
to return early out of vcl_recv, and you want to say that the response
will be uncacheable.

You could set req.uncacheable to true, and also say return(pass), but it
feels like saying the same thing twice.

VCL could also have one but not the other: req.uncacheable=false and
return(pass), or req.uncacheable=true and return(lookup). Which one
determines whether a cache lookup is attempted or bypassed?

The combination req.uncacheable=true and return(lookup) in particular
feels like a contradiction. But if the point-oh release eliminates
return(pass) as you considered, isn't return(lookup) the only way to
return early out of vcl_recv, unless you're going to something like pipe
or synth or fail? If so, then is req.uncacheable=true, suggesting no
lookup, and then return(lookup) the only way to accomplish what
return(pass) does now?

I guess this is starting to sound like I'm critical of the idea, which I
didn't intend, because there may be good answers to all of these
questions. Just trying to get it sorted in my head.


Best,
Geoff
--
** * * UPLEX - Nils Goroll Systemoptimierung

Scheffelstraße 32
22301 Hamburg

Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753

http://uplex.de
Re: Reintroduce a req.uncacheable flag [ In reply to ]
Hi Geoff,

It's been a while, I hope you are doing well :)

On Fri, May 28, 2021 at 5:22 PM Geoff Simmons <geoff@uplex.de> wrote:
>
> On 5/28/21 17:50, Dridi Boukelmoune wrote:
> >
> > First, we already have a relationship between bereq.uncacheable and
> > bereq.uncacheable: the former implies the latter.
>
> Presumably you meant to write that bereq.uncacheable implies
> beresp.uncacheable.

Correct, a tragic copy-pasta accident and yet I managed to make a
silly but valid statement anyway \o/

> My quick and superficial first reaction to the proposal -- I don't quite
> have my head around what you would write in VCL if you do in fact want
> to return early out of vcl_recv, and you want to say that the response
> will be uncacheable.
>
> You could set req.uncacheable to true, and also say return(pass), but it
> feels like saying the same thing twice.
>
> VCL could also have one but not the other: req.uncacheable=false and
> return(pass), or req.uncacheable=true and return(lookup). Which one
> determines whether a cache lookup is attempted or bypassed?

I didn't want to jump too soon into implementation details but I have
to dip a toe now :)

Let's ignore bereq.uncacheable since it's read-only and determined
strictly by what happened on the client side. You can decide to make a
beresp uncacheable but you can't undo it. My assumption is that we
would want the same for req.uncacheable where assigning true takes
effect and false does nothing.

> The combination req.uncacheable=true and return(lookup) in particular
> feels like a contradiction. But if the point-oh release eliminates
> return(pass) as you considered, isn't return(lookup) the only way to
> return early out of vcl_recv, unless you're going to something like pipe
> or synth or fail? If so, then is req.uncacheable=true, suggesting no
> lookup, and then return(lookup) the only way to accomplish what
> return(pass) does now?

Consider the backend transitions today:

- return(deliver)
- return(pass[...])

You have an explicit transition and a general one that will be
influenced by beresp.uncacheable.

On the client side that would be the same:

- return(hash) => influenced by req.uncacheable
- return(pass)

> I guess this is starting to sound like I'm critical of the idea, which I
> didn't intend, because there may be good answers to all of these
> questions. Just trying to get it sorted in my head.

Well, critical doesn't imply against, and your reply shows that my
first message was lacking a bit of context.

I'm not suggesting to remove the pass transition on the client side,
but to add a flag to signal that a lookup attempt should be neutered.
I am however suggesting that the built-in VCL replaces return(pass)
transitions with raising the req.uncacheable flag. This would for
example allow you to keep your cookie handling in vcl_req_cookie even
if your POST request was made uncacheable earlier.

Next implementation details, req.uncacheable would be read-write in
vcl_recv and vcl_hash, and read-only everywhere else.

If you return(pass) from vcl_recv, you can expect req.uncaheable to be
true in vcl_pass, vcl_deliver and vcl_synth.

The req.uncacheable flag would be reset to false after a return(restart).

I hope this clarifies things a bit.

Cheers,
Dridi
_______________________________________________
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev