[PATCH] During background update, nginx can't add "Range" header

[Sangdeuk Kwon]

> Hi,

Thank you for your explanation.

I added comments. Please check it one more time.

>>* On 23 Jul 2020, at 08:29, Sangdeuk Kwon <sangdeuk.kwon at quantil.com <http://mailman.nginx.org/mailman/listinfo/nginx-devel>> wrote:
*>> >>* # HG changeset patch
>*>* # User Sangdeuk Kwon <sangdeuk.kwon at quantil.com <http://mailman.nginx.org/mailman/listinfo/nginx-devel> <mailto:sangdeuk.kwon at quantil.com <http://mailman.nginx.org/mailman/listinfo/nginx-devel>>>
*>>* # Date 1595481798 -32400
>*>* #      Thu Jul 23 14:23:18 2020 +0900
>*>* # Node ID 90e5ccf7c229322079ba1b61b4241ed69dfc09b2
>*>* # Parent  4f30f75dbdf33d6fae9e70086e0df5cbab7db027
>*>* During background update, nginx can't add "Range" header
>*>*
>*>* If the configuration is "slice enabled" and "proxy_cache_use_stale updating"
*>>* and "proxy_cache_background_update on",
>*>* nginx can't get "$slice_range" value during background update.
>*>* nginx sends request to upstream without "Range" header.
>*>* The re-fetched content is saved by cache key of absent "$slice_range".
*>>* So, nginx always serves stale content even though after
re-fetching new content.
*>
> This is correct.  The slice module does not work with background updates.
>
>>*
>*>* slice 1k;
>*>* proxy_cache_use_stale updating;
>*>* proxy_cache_background_update on;
>*>* proxy_cache_key "$host$uri[$slice_range]
>*>*
>*>* diff -r 4f30f75dbdf3 -r 90e5ccf7c229 src/http/modules/ngx_http_slice_filter_module.c
*>>* --- a/src/http/modules/ngx_http_slice_filter_module.c   Tue Jul
21 20:34:29 2020 +0300
*>>* +++ b/src/http/modules/ngx_http_slice_filter_module.c   Thu Jul
23 14:23:18 2020 +0900
*>>* @@ -400,9 +400,12 @@
*>>*      ngx_http_slice_loc_conf_t  *slcf;
*>>
             >>*      ctx = ngx_http_get_module_ctx(r,
ngx_http_slice_filter_module);
*>>* +    if (r->background && r != r->main && r->main != r->parent) {
*>>* +        ctx = ngx_http_get_module_ctx(r->parent,
ngx_http_slice_filter_module);
*>>* +    }
*>
> Here you’re trying to get the parent request’s context to access the slice range.
> But slice range is being constantly updated.  So there’s no guarantee that you get
> the right slice range.

> The right solution would be to pass the slice range to the background update
> subrequest as an argument or a part of uri.  But that does not seem to be possible
> within the current background update implementation. Probably there’s a way to save
> and then access the right slice using njs.
>

The posted requests are saved as below order in r->main->posted_requests.
Subrequest of background update is executed next to same slice request
before next slice request.
Background update's r->parent refers same slice request.
So, r->parent's ngx_http_slice_ctx_t has correct "Range" value for
background update.

r: slice #1
r->main->posted_requests: -> background update request (slice #1) ->
next slice request (slice #2)

r: background update request (slice #1) (r->main=slice #1, r->parent=slice #1)
r->main->posted_requests: -> next slice request (slice #2)

r: slice #2
r->main->posted_requests: -> background update request (slice #2) ->
next slice request (slice #3)

r: background update request (slice #2) (r->main=slice #1, r->parent=slice #2)
r->main->posted_requests: -> next slice request (slice #3)

r: slice #3 (if no need background update)
r->main->posted_requests: -> next slice request (slice #4)

r: slice #4
r->main->posted_requests: -> background update request (slice #4) ->
next slice request (slice #5)

r: background update request (slice #4) (r->main=slice #1, r->parent=slice #4)
r->main->posted_requests: -> next slice request (slice #5)



But, in slice #1 case, "Range" value of ctx is already updated to next
value (slice #2) when it's created.
So, background update of slice #1 can't use r->parent's ctx.

When user request has "Range" header(only one slice case), background
update can generate "Range" value by "Range" header.

"r->main == r->parent" condition is for slice #1 and user request has
"Range" header(only one slice case)
"r->main != r->parent" condition is for slice #2, slice #3, and ...

>>                                                                          >>                                                                                 >>*      if (ctx == NULL) {
*>>* -        if (r != r->main || r->headers_out.status) {
*>>* +        if ((r != r->main && !r->background) ||
r->headers_out.status) {
*>>*              v->not_found = 1;
*>>*              return NGX_OK;
*>>*          }
*>
> Here you are creating a new ctx for the background update subrequest,
> which suggests that you want to update the entire response rather that
> only one slice.
>

It's created when slice #1 and user request has "Range" header(only
one slice case)
>>* _______________________________________________
*>>* nginx-devel mailing list
*>>* nginx-devel at nginx.org
<http://mailman.nginx.org/mailman/listinfo/nginx-devel>
*>>* http://mailman.nginx.org/mailman/listinfo/nginx-devel
<http://mailman.nginx.org/mailman/listinfo/nginx-devel>*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20200727/76c2547e/attachment.htm>