Updating r->cache in ngx_http_upstream_process_cache_control()

[Maxim Dounin]

Hello!

On Mon, Jul 29, 2024 at 10:00:29AM -0700, Maksim Yevmenkin wrote:

> Hello!
> 
> [...]
> 
> thank you for the response.
> 
> > Parsing cache validity time from cached responses is used at least
> > when sending revalidated responses (NGX_HTTP_CACHE_REVALIDATED).
> >
> > For cached responses with other cache statuses
> > (NGX_HTTP_CACHE_HIT, NGX_HTTP_CACHE_STALE) this probably can be
> > avoided, at least with current code, but it is generally safe, and
> > avoiding parsing would be at most minor optimization.  Further,
> > such an optimization might not be trivial to maintain, since cache
> > statuses might be added or changed over time.
> 
> Just to clarify, I'm not suggesting that parsing is an issue.
> 
> My concern is updating r->cache->valid_sec (and other variables) while
> serving a cached object. As the cached object is served (HIT),
> r->cache->valid_sec is advancing compared to node->valid_sec.

Updating r->cache->valid_sec for cached responses is safe, as this 
field is not used (as a cache node validity time) after parsing 
cached response headers.

And for some cached responses (REVALIDATED) it is actually used as 
an updated cached response validity time.

> > In general, though, updating r->cache->valid_sec from headers
> > processing functions as currently implemented is wrong, and can
> 
> Understood, I just wanted to clarify your response about it being
> "generally safe."
> 
> > lead to incorrect results when switching to other upstream servers
> > per proxy_next_upstream after parsing some headers.  It should be
> > either explicitly cleared in ngx_http_upstream_reinit(), or the
> > parsed value should be kept in u->headers_in till actually used
> > (so it will be cleared by existing generic code in
> > ngx_http_upstream_reinit()).
> 
> Could you please help me understand how ngx_http_upstream_reinit()
> would be beneficial in the case of a cache HIT?

The issue with updating r->cache->valid_sec from headers 
processing functions as described in the quoted paragraph is 
unrelated to cached responses, but instead manifests itself when 
receiving responses from upstream servers.

-- 
Maxim Dounin
http://mdounin.ru/