[PATCH] proxy_cache_max_range_offset could affect the background updating

[Sergey Kandaurov]

> On 7 Dec 2022, at 09:26, Sangdeuk Kwon <sangdeuk.kwon at quantil.com> wrote:
> 
> # HG changeset patch
> # User Sangdeuk Kwon <sangdeuk.kwon at quantil.com>
> # Date 1670390583 -32400
> #      Wed Dec 07 14:23:03 2022 +0900
> # Node ID a1069fbf10ffd806b7c8d6deb3f6546edc7b0427
> # Parent  0b360747c74e3fa7e439e0684a8cf1da2d14d8f6
> proxy_cache_max_range_offset could affect the background updating
> 
> proxy_cache_max_range_offset doesn't care about the upstream of background updating.
> So, nginx drops the new cache file after background updating.
> This behavior is strange because background updating is just to fetch
> new content after serving a stale cache, not to serve it.
> 
> I think the background updating should be not affected by proxy_cache_max_range_offset.
> 

Indeed, such configuration with far ranges results in useless background
(range) subrequests, the response is discarded.

While the behaviour matches the description of proxy_cache_max_range_offset,
I believe the original intention was to avoid latencies in sending output
to the client, imposed by obtaining a new full resource from upstream.
As long as background subrequests don't delay sending output to the client,
it should be ok to keep caching of (full) response enabled, regardless of
whether the proxy_cache_max_range_offset limit is hit.

> Related directives:
> proxy_cache_max_range_offset 10;
> proxy_cache_use_stale updating;
> proxy_cache_background_update on;
> 
> diff -r 0b360747c74e -r a1069fbf10ff src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c      Thu Nov 24 23:08:30 2022 +0400
> +++ b/src/http/ngx_http_upstream.c      Wed Dec 07 14:23:03 2022 +0900
> @@ -986,7 +986,9 @@
>          return rc;
>      }
>  
> -    if (ngx_http_upstream_cache_check_range(r, u) == NGX_DECLINED) {
> +    if (!r->background
> +        && ngx_http_upstream_cache_check_range(r, u) == NGX_DECLINED)
> +    {
>          u->cacheable = 0;
>      }
>  

Given the above, I cannot imagine that testing far ranges
is anyhow usable for background subrequests in practice.
As such, I think the condition can be moved inside of the
ngx_http_upstream_cache_check_range() function.

-- 
Sergey Kandaurov