Hi, Vadim.

Thanks for your cooperation!

> @@ -5067,6 +5091,7 @@
> || (h->value.len == 1 && h->value.data[0] == '*'))
> {
> u->cacheable = 0;
> + u->cacheable_reason |= NGX_HTTP_CACHEABLE_VARY;
> }

> I don't see any reasons to store information about "Vary" header.

It should not cache only the Vary asterisk. it doesn't need to store this
information so we consider that your approach (bitmask) is correct.

> And we can call
> "ngx_http_upstream_cache_validate_regardless_order" only when cacheable
is still
> enabled after parsing all other headers.

Our approach is a bit different, u->cacheable should be determined after
completing all headers parsing. So we consider that it could be better not
to set here and there during parsing headers particularly in case of
u->cacheable reverts 1 to 0 if there is, it could be misread easily.

> I will prepare an improved version if you don't mind.

Sure! Can we pick your brain for this issue?

Thanks,
Yugo Horie


On Mon, Apr 18, 2022 at 19:19 Vadim Fedorenko <vadim.fedorenko@cdnnow.ru>
wrote:

> Hi Yugo!
>
> Looks like another solution with the same concept - store additional
> information
> about several headers. The second patch you proposed looks a bit
> overcomplicated
> inside this additional function. I don't see any reasons to store
> information
> about "Vary" header. AFAIU, weed special information about 3 headers:
> "Expires",
> "Cache-Control" and "X-Accel-Expires". And we can call
> "ngx_http_upstream_cache_validate_regardless_order" only when cacheable is
> still
> enabled after parsing all other headers. That approach will simplify the
> logic
> inside ngx_http_upstream_cache_validate_regardless_order. I will prepare an
> improved version if you don't mind.
>
> Thanks,
> Vadim
>
> On 18.04.2022 02:14, Yugo Horie wrote:
> > We're also interested in this issue because the CDN provider like us is
> > concerned about the problems with the headers that determine the cache
> freshness.
> > It is often a serious problem used in Nginx as the origin server or the
> > intermediate cache server that has such problems.
> > In those days, we also proposed a patch to resolve it.
> >
> https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/
> > <
> https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/
> >
> >
> > But it seems to have declined. We sincerely wish to resolve it and
> intend to do
> > anything.
> >
> > Thanks,
> > Yugo Horie
> >
> > 2022年4月18日(月) 4:51 Vadim Fedorenko via nginx-devel <
> nginx-devel@nginx.org
> > <mailto:nginx-devel@nginx.org>>:
> >
> > Hi Maxim!
> >
> > On 17.04.2022 02:55, Maxim Dounin wrote:
> > > Hello!
> > >
> > >
> > > [...]
> > >
> > > Thanks for the patch.
> > >
> > > I'm quite sceptical about attempts to fix this by introducing
> > > various flags and reverting cacheable status back to 1. This is
> > > not how it should be fixed.
> > >
> > Yeah, the solution might be a bit complicated, but I couldn't find
> > another way without breaking concept of independent header parsing.
> > Could you please suggest something if you think that current approach
> > is wrong? The ticket that this patch tries to fix is 6 years old and
> > still has discussions going on without any solution.
> >
> > Thanks,
> > Vadim
> > _______________________________________________
> > nginx-devel mailing list -- nginx-devel@nginx.org <mailto:
> nginx-devel@nginx.org>
> > To unsubscribe send an email to nginx-devel-leave@nginx.org
> > <mailto:nginx-devel-leave@nginx.org>
> >
>
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org