[PATCH] skip apply "ngx_http_upstream_process_accel_expires" when a request responeded by cache

[Sokichi Takada]

Hello!

Thanks for the response.

> The cache validity time as obtained from cached responses is used
> while processing 304 responses during cache revalidation, so the
> suggested change looks wrong to me.

When processing 304 responses during cache revalidation,
ngx_http_upstream_process_accel_expires is called twice.

1st time, in "ngx_http_upstream_process_header" function.
src/http/ngx_http_upstream.c: 2387         rc = u->process_header(r);
#  process_header = ngx_http_upstream_process_accel_expires

This calculates the exact valid_sec value by x-accel-expires of upstream's
response,  It doesn't use a value of the cache data.
Also, the r->cached is 0 at this time.

2nd time, in "ngx_http_upstream_process_header" ->
"ngx_http_upstream_test_next" -> "ngx_http_upstream_cache_send" function.
src/http/ngx_http_upstream.c: 2423         if
(ngx_http_upstream_test_next(r, u) == NGX_OK)    ->
src/http/ngx_http_upstream.c: 2535         rc =
ngx_http_upstream_cache_send(r, u);                  ->
src/http/ngx_http_upstream.c: 1073     rc = u->process_header(r);

This calculates unnecessary valid_sec value by x-accel-expires of the cache
data.
Also, this calculation can skip by this patch since the r->cached is 1 at
this time.

Please look into ngx_http_upstream_test_next.
It keeps the valid_sec value of 1st time into "valid" variable at "2523
    valid = r->cache->valid_sec;".
After that, it "calls ngx_http_upstream_cache_send", then updates cache
data with the "valid" variable at "2560             r->cache->valid_sec =
valid;".

--
TAKADA Sokichi







On Mon, Jun 22, 2020 at 6:11 AM Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Fri, Jun 19, 2020 at 09:41:37AM +0900, TAKADA Sokichi wrote:
>
> > # HG changeset patch
> > # User TAKADA Sokichi <takada at jocdn.co.jp>
> > # Date 1592383243 -32400
> > #      Wed Jun 17 17:40:43 2020 +0900
> > # Node ID d5bbe3ad1f1491125485786aacc5d219c81801d1
> > # Parent  8cf31489b479b689b7ff4a9601ce24c914d0394c
> > skip apply "ngx_http_upstream_process_accel_expires" when a request
> responeded by cache
> >
> > "ngx_http_upstream_process_accel_expires" is calculate and set
> valid_date value when a request respond by a cache, even though it will
> never use in the response.
> > This patch to skip unnecessary process.
> >
> > On the other hands, this has good side effect to can get TRUE valid_date
> value of the cache file in "header filter" and "log" hook handler.
> > i.e. I would like to get the TRUE valid_date of cache in
> header_filter_by_lua or log_by_lua handlers.
> >
> > diff -r 8cf31489b479 -r d5bbe3ad1f14 src/http/ngx_http_upstream.c
> > --- a/src/http/ngx_http_upstream.c    Mon Jun 15 17:35:26 2020 -0400
> > +++ b/src/http/ngx_http_upstream.c    Wed Jun 17 17:40:43 2020 +0900
> > @@ -4776,6 +4776,10 @@
> >          return NGX_OK;
> >      }
> >
> > +    if (r->cached == 1) {
> > +        return NGX_OK;
> > +    }
> > +
> >      len = h->value.len;
> >      p = h->value.data;
>
> Thanks for the patch.
>
> The cache validity time as obtained from cached responses is used
> while processing 304 responses during cache revalidation, so the
> suggested change looks wrong to me.
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>


-- 
-------------------------------------
Sokichi TAKADA <takada at jocdn.co.jp>
JOCDN Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20200622/84df8141/attachment.htm>