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/
