stale negative cache is not updated by fresh negative response

[fdiary]

Hello,

ATS does not update existing cache of 'negative' response even if ATS fetches a fresh equally-negative response from the backend.

CONFIG proxy.config.http.negative_revalidating_enabled INT 1
CONFIG proxy.config.http.negative_revalidating_lifetime INT 3600

If the backend replies 502 result having Cache-Control: max-age=60, public, it is well cached in ATS, that is good. When it becomes stale, i.e. more than 60 seconds later, ATS fetchs a new version from the backend, which, if it is equally negative to the cached version, will be discarded and the stale entry will be sent to the client. That means :

• The client cannot get a newer 'negative' response.
• Once the negative cache becomes stale, all incoming requests will involve requests to the backend.

It is of course good 'not updating positive stale cache by negative fresh response', but there is no reason 'not to update negative stale cache by negative fresh response'.

[background]

My idea is switching between 'normal' backend and 'maintenance mode' backend using HAProxy behind ATS. Usually normal backend replies 200 status with Cache-Control: max-age=300, public. When we switch to the 'maintenance mode' backend, it replies a beautiful maintenance information page having 502 status with Cache-Control: max-age=60, public for whatever HTML requests. By using this short max-age cache, we would like to avoid heavy load on the maintenance mode backend.

Thanks to 502 status of 'maintenance mode' backend, ATS tries to return a stale cache of 'positive' response, if exists, until long-enough negative_revalidating_lifetime reaches, i.e. we can provide a normal site as much as possible. And when negative_revalidating_lifetime reaches, or positive cache does not exist (for explicit no-store requests etc.), the client will get a beautiful maintenance information page.

But because of the behaviour describing here, the client cannot get the updated maitenance information page during maintenance mode and also the maintenance mode backend needs to respond for each request in vain.

Here is an example timeline.

00:00 ATS fetches 200 response from the normal backend and replies it
00:01 (we switch to the maintenance mode backend)
00:03 ATS replies a normal cache
00:10 cache is already stale, but ATS still replies a normal cache thanks to negative_revalidating_lifetime
01:15 ATS fetches from the maintenance mode backend, then caches it and replies it
01:16 (we update the maintenance page on the maintenance backend)
01:17 'negative' cache is already stale, thus ATS fetches again from the maintenance mode backend,
      but replies the old existing stale 'negative' cache without updating it
01:20 'negative' cache is always stale, thus ATS fetches again from the maintenance mode backend.

If ATS supports stale-if-error, our responses would be:

• from 'normal' backend: Cache-Control: max-age=...,stale-if-error=<7 days>
• from 'maintenance' backend: Cache-Control: max-age: 60 (without stale-if-error)

but it is not supported for now, thus we are trying this approach with using negative_revalidating_lifetime.

I verified this behaviour on ATS version 7.1.1 and 8.0.2.

Thanks in advance !
Kazuhiko
/cc @vpelletier

[fdiary]

I would like to propose the following patch that perfectly solves this issue.

--- trafficserver-8.1.1/proxy/http/HttpTransact.cc.orig 2020-12-01 00:30:26.000000000 +0100
+++ trafficserver-8.1.1/proxy/http/HttpTransact.cc      2021-01-11 11:35:41.946893735 +0100
@@ -5752,6 +5752,7 @@
 HttpTransact::is_stale_cache_response_returnable(State *s)
 {
   HTTPHdr *cached_response = s->cache_info.object_read->response_get();
+  HTTPStatus cache_response_code = cached_response->status_get();
 
   // First check if client allows cached response
   // Note does_client_permit_lookup was set to
@@ -5760,6 +5761,12 @@
   if (!s->cache_info.directives.does_client_permit_lookup) {
     return false;
   }
+  // We don't serve stale negative cache.
+  if (cache_response_code == HTTP_STATUS_INTERNAL_SERVER_ERROR || cache_response_code == HTTP_STATUS_GATEWAY_TIMEOUT ||
+      cache_response_code == HTTP_STATUS_BAD_GATEWAY || cache_response_code == HTTP_STATUS_SERVICE_UNAVAILABLE) {
+    TxnDebug("http_trans", "[is_stale_cache_response_returnable] stale negative cache");
+    return false;
+  }
   // Spec says that we can not serve a stale document with a
   //   "must-revalidate header"
   // How about "s-maxage" and "no-cache" directives?

[zwoop]

I think this was fixed in #7401 ? If so, we should close this.

[bneradt]

I think this was fixed in #7401 ? If so, we should close this.

Thanks for looking into this. I believe, however, that the issue described here is orthogonal to #7401. #7401 fixed an issue that was introduced recently (a few weeks ago) via #7361 , but the issue described here is seen in 8.0.2 (per Kazuhiko's previous comment):

I verified this behaviour on ATS version 7.1.1 and 8.0.2.

8.0.2 is a couple years old:
https://github.com/apache/trafficserver/releases/tag/8.0.2

[fdiary]

Thanks for comments.
I just tried ATS 8.1.1 and confirmed that the issue still existed but could be fixed by my patch above.

[masaori335]

@fdiary could you open a PR with your patch on the master branch?

[fdiary]

@masaori335 Sure. I just opened a PR : #7422

[masaori335]

Thanks for the PR. Let me clarify a bit.

When it becomes stale, i.e. more than 60 seconds later, ATS fetchs a new version from the backend, which, if it is equally negative to the cached version, will be discarded and the stale entry will be sent to the client.

Is this because proxy.config.http.negative_revalidating_lifetime is not used at is_stale_cache_response_returnable() as you pointed out #7425?
I tried to repro the scenario. And seems like the stale negative response is updated if proxy.config.http.cache.max_stale_age is short enough.

[fdiary]

And seems like the stale negative response is updated if proxy.config.http.cache.max_stale_age is short enough.

Indeed, is_stale_cache_response_returnable refers max_stale_age but not negative_revalidating_lifetime. But if we have short max_stale_age, good 'positive' cache will be also unavailable shortly, that is not good.

So regardless which value is_stale_cache_response_returnable refers, I believe that the proposal here is still meaningful to fix this issue, i.e. to provide a good 'positive' cache for long period but to make updating 'negative' cache possible and also to reduce requests to the 'negative response' backend.

[masaori335]

I agree. It'd be useful if we can change the period of serving stale contents by the cached status code.