Vadim Fedorenko via nginx-devel
April 22, 2022 03:08PM
Hi Maxim!

Thanks for the feedback. As you might have already seen, I sent another version
of this patch which addresses some of style and readability issues as well as
X-Accel-Expires issue in v3.

Regarding RFC issue. RFC 7234 Section 5.1 explicitly states that Expires MUST be
ignored if reponse contains Cache-Control header with max-age or s-maxage
directives. That's why I said that Nginx server configured to cache responses
and to rely on headers to determine the freshness violates RFC. And this is true
for a subset of absolutely legitimate responses, and it was done intended to
actually optimize the internal processing but not because of configuration
optimizations.

Hope you will have some time to review version 3 of my patch even though it
might not clearly apply on top of the patchset you sent couple of days ago.

Thanks,
Vadim

On 22.04.2022 19:23, Maxim Dounin wrote:
> Hello!
>
> On Tue, Apr 19, 2022 at 10:59:12PM +0100, Vadim Fedorenko wrote:
>
>> On 19.04.2022 16:01, Maxim Dounin wrote:
>>>
>>> On Sun, Apr 17, 2022 at 08:50:25PM +0100, Vadim Fedorenko via nginx-devel wrote:
>>>
>>>> On 17.04.2022 02:55, Maxim Dounin wrote:
>>>>
>>>>> 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.
>>>
>>> That's basically why the ticket is still not fixed: there is no
>>> easy fix, and the issue is rather minor, especially compared to
>>> the complexity of the changes required to fix it.
>>>
>> Hmm.. I would argue on the minority of the issue, especially when we talk about
>> absence of compatibility with RFC and actual discussions in the ticket. Anyway.
>
> While current behaviour might be suboptimal in some use cases, 34it
> does not violate RFC, as caching is not something required to
> happen. So we are talking about optimizing some use cases here,
> and the same optimization can be easily obtained by adjusting
> headers in the backend response.
>
>>> IMHO, the most promising approach would be to move
>>> Cache-Control/Exires/X-Accel-Expires handling elsewhere, and
>>> handle them somewhere in ngx_http_upstream_process_headers(),
>>> probably along with ngx_http_file_cache_valid() as well.
>>>
>> Ok, cool, I updated my patch, now it's based on the work from Yugo that was left
>> without comments and is following the way you suggest. Take a look at v2, please.
>
> Apart multiple style issues, the patch fails to address the
> X-Accel-Expires vs. Cache-Control order issue as outlined in the
> ticket, that is, the response with:
>
> : X-Accel-Expires: 10
> : Cache-Control: max-age=100, stale-while-revalidate=1000
>
> results in different behaviour than the one with:
>
> : Cache-Control: max-age=100, stale-while-revalidate=1000
> : X-Accel-Expires: 10
>
> It is also seems to be completely unreadable. (Yugo's patch tried
> to address this, though had multiple style and readability issues
> as well.)
>
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 1097 April 14, 2022 07:04PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 158 April 16, 2022 09:56PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 187 April 17, 2022 03:52PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

yugo-horie 231 April 17, 2022 09:16PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 213 April 18, 2022 06:22AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

yugo-horie 200 April 18, 2022 07:04PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 275 April 19, 2022 11:02AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 140 April 22, 2022 02:24PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 248 April 22, 2022 03:08PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 161 April 24, 2022 12:56AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 147 April 24, 2022 11:44AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

yugo-horie 189 April 25, 2022 08:28AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 194 April 25, 2022 05:04PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Vadim Fedorenko via nginx-devel 165 April 25, 2022 05:08PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov 142 June 06, 2022 09:44AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov 119 June 06, 2022 10:42AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 137 June 06, 2022 05:10PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov 163 June 07, 2022 12:02PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 175 June 07, 2022 09:00PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov 158 June 10, 2022 07:40AM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Maxim Dounin 129 June 11, 2022 06:54PM

Re: [PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov 167 June 13, 2022 07:42AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 290
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 466 on July 09, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready