[PATCH 1 of 3] Correctly calculate and set Age header

Maxim Dounin mdounin at mdounin.ru
Thu Jul 18 00:26:16 UTC 2024


Hello!

On Mon, Jul 01, 2024 at 10:45:59PM +0900, Hiroaki Nakamura wrote:

> Hello
> 
> 2024年6月27日(木) 9:32 Maxim Dounin <mdounin at mdounin.ru>:
> > [...]
> > The commit message certainly needs more work.  Still, you may want
> > to postpone it, as well as code modifications, before various
> > conceptual questions are cleaned up.
> 
> I have updated the commit log again.
> 
> > > > > @@ -75,6 +77,8 @@
> > > > >      time_t                           error_sec;
> > > > >      time_t                           last_modified;
> > > > >      time_t                           date;
> > > > > +    time_t                           response_time;
> > [...]
> > The "date" field is set when the cache item is created, and
> > can be updated on cache item revalidation (with
> > "proxy_cache_revalidate on").  It is not updated when sending a
> > cached response.
> 
> Thank you, I had misunderstood this.
> I take this into consideration and I found that we can keep
> ngx_http_file_cache_header_t as is if we ignore
> corrected_initial_age concept and just use the Age header from the
> upstream as is (see below).

[...]

> > Note well that the "would be great to dig further" comment still
> > applies.

Thanks for yet another version of the patch.  In its current form 
it starts to look reasonable.

I've spent some time looking into this, including looking through 
previous attempts to implement some forms of Age support, notably:

Florent Le Coz in 2013, cache validity based on Age
https://mailman.nginx.org/pipermail/nginx-devel/2013-November/004532.html

Piotr Sikora in 2014, $upstream_cache_age variable
https://mailman.nginx.org/pipermail/nginx-devel/2014-October/006025.html

Damien Tournoud in 2015, $upstream_cache_age variable
https://mailman.nginx.org/pipermail/nginx-devel/2015-June/007002.html

Overall, there seems to be two major parts of what is expected to 
be done here:

1. Adjusting cache validity times based on the Age header in the 
upstream server response.

2. Returning cached responses with updated/added Age header.

These two parts probably can be implemented separately, with 
minimal interdependencies.

Further, I tend to think that, while (1) probably can be the 
default, it might not be a good idea to do (2) by default, as in 
most setups [free]nginx is used as an origin server, even if 
caching is enabled.  In such configurations administrators often 
use "Cache-Control: max-age=N" for client-side caching, and 
control caching in [free]nginx separately, such as with 
proxy_cache_valid.  Adding the Age header in such configurations 
might result in unexpectedly non-working client-side caching.

In particular, (2) can be implemented as a variable with relevant 
information to be used in the "add_header" directive if one needs 
to implement Age updating.  Alternative approach would be to add 
per-module directives to control (2), such as "proxy_cache_age on|off", 
and add the Age header in the upstream module, though a variable 
looks easier.

Below are two patches that implement (1) adjusting cache validity 
base on the Age header and (2) a variable to set the Age header, 
and yet another patch to implement tests.  Please take a look.


# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1721259797 -10800
#      Thu Jul 18 02:43:17 2024 +0300
# Node ID 0843408f7af487d59800edcd773eddab33eade05
# Parent  2a847df382320a54c32abd9e64412ac611fcff0c
Upstream: using the "Age" header when caching responses.

As long as the "Age" header is present and not ignored, it is now
respected when caching responses per "Cache-Control: max-age", reducing
cache validity time.

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -136,6 +136,8 @@ static ngx_int_t
     ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_upstream_process_vary(ngx_http_request_t *r,
     ngx_table_elt_t *h, ngx_uint_t offset);
+static ngx_int_t ngx_http_upstream_process_age(ngx_http_request_t *r,
+    ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_upstream_copy_header_line(ngx_http_request_t *r,
     ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t
@@ -293,6 +295,10 @@ static ngx_http_upstream_header_t  ngx_h
                  ngx_http_upstream_copy_multi_header_lines,
                  offsetof(ngx_http_headers_out_t, link), 0 },
 
+    { ngx_string("Age"),
+                 ngx_http_upstream_process_age, 0,
+                 ngx_http_upstream_copy_header_line, 0, 0 },
+
     { ngx_string("X-Accel-Expires"),
                  ngx_http_upstream_process_accel_expires, 0,
                  ngx_http_upstream_copy_header_line, 0, 0 },
@@ -475,6 +481,7 @@ ngx_conf_bitmask_t  ngx_http_upstream_ig
     { ngx_string("Cache-Control"), NGX_HTTP_UPSTREAM_IGN_CACHE_CONTROL },
     { ngx_string("Set-Cookie"), NGX_HTTP_UPSTREAM_IGN_SET_COOKIE },
     { ngx_string("Vary"), NGX_HTTP_UPSTREAM_IGN_VARY },
+    { ngx_string("Age"), NGX_HTTP_UPSTREAM_IGN_AGE },
     { ngx_null_string, 0 }
 };
 
@@ -4939,13 +4946,14 @@ ngx_http_upstream_process_cache_control(
             return NGX_OK;
         }
 
-        if (n == 0) {
+        if (n <= u->headers_in.age_n) {
             u->headers_in.no_cache = 1;
             return NGX_OK;
         }
 
-        r->cache->valid_sec = ngx_time() + n;
+        r->cache->valid_sec = ngx_time() + n - u->headers_in.age_n;
         u->headers_in.expired = 0;
+        u->headers_in.max_age = 1;
     }
 
 extensions:
@@ -5109,6 +5117,7 @@ ngx_http_upstream_process_accel_expires(
             r->cache->valid_sec = ngx_time() + n;
             u->headers_in.no_cache = 0;
             u->headers_in.expired = 0;
+            u->headers_in.max_age = 0;
             return NGX_OK;
         }
     }
@@ -5122,6 +5131,7 @@ ngx_http_upstream_process_accel_expires(
         r->cache->valid_sec = n;
         u->headers_in.no_cache = 0;
         u->headers_in.expired = 0;
+        u->headers_in.max_age = 0;
     }
     }
 #endif
@@ -5373,6 +5383,61 @@ ngx_http_upstream_process_vary(ngx_http_
 
 
 static ngx_int_t
+ngx_http_upstream_process_age(ngx_http_request_t *r,
+    ngx_table_elt_t *h, ngx_uint_t offset)
+{
+    ngx_http_upstream_t  *u;
+
+    u = r->upstream;
+
+    if (u->headers_in.age) {
+        ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
+                      "upstream sent duplicate header line: \"%V: %V\", "
+                      "previous value: \"%V: %V\", ignored",
+                      &h->key, &h->value,
+                      &u->headers_in.age->key,
+                      &u->headers_in.age->value);
+        h->hash = 0;
+        return NGX_OK;
+    }
+
+    u->headers_in.age = h;
+    h->next = NULL;
+
+#if (NGX_HTTP_CACHE)
+    {
+    ngx_int_t  n;
+
+    if (u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_AGE) {
+        return NGX_OK;
+    }
+
+    if (r->cache == NULL || !u->cacheable) {
+        return NGX_OK;
+    }
+
+    n = ngx_atoi(h->value.data, h->value.len);
+
+    if (n != NGX_ERROR) {
+        u->headers_in.age_n = n;
+
+        if (u->headers_in.max_age) {
+            if (n >= r->cache->valid_sec - ngx_time()) {
+                u->headers_in.no_cache = 1;
+                return NGX_OK;
+            }
+
+            r->cache->valid_sec -= n;
+        }
+    }
+    }
+#endif
+
+    return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_upstream_copy_header_line(ngx_http_request_t *r, ngx_table_elt_t *h,
     ngx_uint_t offset)
 {
diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h
--- a/src/http/ngx_http_upstream.h
+++ b/src/http/ngx_http_upstream.h
@@ -54,6 +54,7 @@
 #define NGX_HTTP_UPSTREAM_IGN_XA_BUFFERING   0x00000080
 #define NGX_HTTP_UPSTREAM_IGN_XA_CHARSET     0x00000100
 #define NGX_HTTP_UPSTREAM_IGN_VARY           0x00000200
+#define NGX_HTTP_UPSTREAM_IGN_AGE            0x00000400
 
 
 typedef struct {
@@ -287,14 +288,17 @@ typedef struct {
 
     ngx_table_elt_t                 *cache_control;
     ngx_table_elt_t                 *set_cookie;
+    ngx_table_elt_t                 *age;
 
     off_t                            content_length_n;
     time_t                           last_modified_time;
+    time_t                           age_n;
 
     unsigned                         connection_close:1;
     unsigned                         chunked:1;
     unsigned                         no_cache:1;
     unsigned                         expired:1;
+    unsigned                         max_age:1;
 } ngx_http_upstream_headers_in_t;
 
 
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1721260119 -10800
#      Thu Jul 18 02:48:39 2024 +0300
# Node ID ea9e8bb70744f245e4d381f13766ffb4470515b3
# Parent  0843408f7af487d59800edcd773eddab33eade05
Upstream: $upstream_cache_age variable.

The variable reflects response age, including the time spent in the
cache and the upstream response age as obtained from the "Age" header.
If the response wasn't cached, the variable reflects the "Age" header
of the upstream response.

If the intended use case is to cache responses as per HTTP/1.1 caching
model, the $upstream_cache_age variable can be used to provide the "Age"
header with the "add_header" directive, such as:

    add_header Age $upstream_cache_age;

This now removes the "Age" header if it was present.

Further, the "expires" directives now removes the "Age" header if it
was present in the response, as the "expires" directive assumes zero
age when it adds "Expires" and "Cache-Control" headers.

diff --git a/src/http/modules/ngx_http_headers_filter_module.c b/src/http/modules/ngx_http_headers_filter_module.c
--- a/src/http/modules/ngx_http_headers_filter_module.c
+++ b/src/http/modules/ngx_http_headers_filter_module.c
@@ -93,6 +93,10 @@ static ngx_http_set_header_t  ngx_http_s
                  offsetof(ngx_http_headers_out_t, etag),
                  ngx_http_set_response_header },
 
+    { ngx_string("Age"),
+                 offsetof(ngx_http_headers_out_t, age),
+                 ngx_http_set_response_header },
+
     { ngx_null_string, 0, NULL }
 };
 
@@ -352,6 +356,11 @@ ngx_http_set_expires(ngx_http_request_t 
         }
     }
 
+    if (r->headers_out.age) {
+        r->headers_out.age->hash = 0;
+        r->headers_out.age = NULL;
+    }
+
     e = r->headers_out.expires;
 
     if (e == NULL) {
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -278,6 +278,7 @@ typedef struct {
 
     ngx_table_elt_t                  *cache_control;
     ngx_table_elt_t                  *link;
+    ngx_table_elt_t                  *age;
 
     ngx_str_t                        *override_charset;
 
diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -31,6 +31,8 @@ static ngx_int_t ngx_http_upstream_cache
     ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r,
     ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_upstream_cache_age(ngx_http_request_t *r,
+    ngx_http_variable_value_t *v, uintptr_t data);
 #endif
 
 static void ngx_http_upstream_init_request(ngx_http_request_t *r);
@@ -297,7 +299,8 @@ static ngx_http_upstream_header_t  ngx_h
 
     { ngx_string("Age"),
                  ngx_http_upstream_process_age, 0,
-                 ngx_http_upstream_copy_header_line, 0, 0 },
+                 ngx_http_upstream_copy_header_line,
+                 offsetof(ngx_http_headers_out_t, age), 0 },
 
     { ngx_string("X-Accel-Expires"),
                  ngx_http_upstream_process_accel_expires, 0,
@@ -436,6 +439,10 @@ static ngx_http_variable_t  ngx_http_ups
       ngx_http_upstream_cache_etag, 0,
       NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
 
+    { ngx_string("upstream_cache_age"), NULL,
+      ngx_http_upstream_cache_age, 0,
+      NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
+
 #endif
 
     { ngx_string("upstream_http_"), NULL, ngx_http_upstream_header_variable,
@@ -6211,6 +6218,60 @@ ngx_http_upstream_cache_etag(ngx_http_re
     return NGX_OK;
 }
 
+
+static ngx_int_t
+ngx_http_upstream_cache_age(ngx_http_request_t *r,
+    ngx_http_variable_value_t *v, uintptr_t data)
+{
+    u_char  *p;
+    time_t   now, age;
+
+    if (r->upstream == NULL) {
+        v->not_found = 1;
+        return NGX_OK;
+    }
+
+    if (!r->cached
+        || r->cache == NULL
+        || r->upstream->cache_status == NGX_HTTP_CACHE_REVALIDATED)
+    {
+        if (r->upstream->headers_in.age == NULL) {
+            v->not_found = 1;
+            return NGX_OK;
+        }
+
+        v->valid = 1;
+        v->no_cacheable = 0;
+        v->not_found = 0;
+        v->len = r->upstream->headers_in.age->value.len;
+        v->data = r->upstream->headers_in.age->value.data;
+
+        return NGX_OK;
+    }
+
+    p = ngx_pnalloc(r->pool, NGX_TIME_T_LEN);
+    if (p == NULL) {
+        return NGX_ERROR;
+    }
+
+    now = ngx_time();
+    age = now - r->cache->date;
+
+    if (r->cache->date > now) {
+        age = 0;
+    }
+
+    age += r->upstream->headers_in.age_n;
+
+    v->len = ngx_sprintf(p, "%T", age) - p;
+    v->valid = 1;
+    v->no_cacheable = 0;
+    v->not_found = 0;
+    v->data = p;
+
+    return NGX_OK;
+}
+
 #endif
 
 

Tests:

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1721260388 -10800
#      Thu Jul 18 02:53:08 2024 +0300
# Node ID bae8b56d95f6810e146fb6e681229189526973f5
# Parent  85d88cd5091c6691d58de81dec544c901f33b6d7
Tests: proxy cache Age header handling tests.

diff --git a/proxy_cache_age.t b/proxy_cache_age.t
new file mode 100644
--- /dev/null
+++ b/proxy_cache_age.t
@@ -0,0 +1,145 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for proxy cache Age header handling.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http proxy cache/)->plan(13);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+    %%TEST_GLOBALS_HTTP%%
+
+    proxy_cache_path %%TESTDIR%%/cache keys_zone=NAME:1m;
+    proxy_cache_key $request_uri;
+
+    server {
+        listen       127.0.0.1:8080;
+        server_name  localhost;
+
+        add_header  X-Cache-Status  $upstream_cache_status;
+        add_header  Age  $upstream_cache_age;
+
+        location / {
+            proxy_pass  http://127.0.0.1:8081;
+            proxy_cache NAME;
+        }
+
+        location /revalidate {
+            proxy_pass  http://127.0.0.1:8081;
+            proxy_cache NAME;
+            proxy_cache_revalidate on;
+        }
+
+        location /ignore/ {
+            proxy_pass  http://127.0.0.1:8081/;
+            proxy_cache NAME;
+
+            proxy_ignore_headers Age;
+        }
+    }
+
+    server {
+        listen       127.0.0.1:8081;
+        server_name  localhost;
+
+        location /fresh {
+            add_header Cache-Control max-age=100;
+            add_header Age 90;
+        }
+
+        location /stale {
+            add_header Cache-Control max-age=100;
+            add_header Age 110;
+        }
+
+        location /before {
+            add_header Age 110;
+            add_header Cache-Control max-age=100;
+        }
+
+        location /noage {
+            add_header Cache-Control max-age=100;
+        }
+
+        location /revalidate {
+            add_header Cache-Control max-age=1;
+        }
+    }
+}
+
+EOF
+
+$t->write_file('fresh', 'SEE-THIS');
+$t->write_file('stale', 'SEE-THIS');
+$t->write_file('before', 'SEE-THIS');
+$t->write_file('noage', 'SEE-THIS');
+$t->write_file('revalidate', 'SEE-THIS');
+
+$t->try_run('no age support');
+
+###############################################################################
+
+# responses with Age header cached
+
+like(get('/fresh'), qr/HIT/, 'fresh cached');
+like(get('/stale'), qr/MISS/, 'stale not cached');
+like(get('/before'), qr/MISS/, 'stale age first not cached');
+like(get('/noage'), qr/HIT/, 'noage cached');
+like(get('/revalidate'), qr/HIT/, 'revalidate cached');
+
+# the same with the Age header ignored
+
+like(get('/ignore/fresh'), qr/HIT/, 'fresh ignore');
+like(get('/ignore/stale'), qr/HIT/, 'stale ignore');
+like(get('/ignore/before'), qr/HIT/, 'stale age first ignore');
+like(get('/ignore/noage'), qr/HIT/, 'noage ignore');
+
+# age header updated on cached responses
+
+sleep(2);
+
+like(http_get('/fresh'), qr/(?<!Age:.{,200})Age: 9[1-5](?!.*Age:)/ms,
+	'cached age updated');
+like(http_get('/stale'), qr/(?<!Age:.{,200})Age: 110(?!.*Age:)/ms,
+	'not cached age preserved');
+like(http_get('/noage'), qr/(?<!Age:.{,200})Age: [1-5](?!.*Age:)/ms,
+	'noage age added');
+
+like(http_get('/revalidate'), qr/REVALIDATED(?!.*Age:)/ms,
+	'revalidate age not added');
+
+###############################################################################
+
+sub get {
+	my ($uri) = @_;
+	http_get($uri);
+	http_get($uri);
+}
+
+###############################################################################

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


More information about the nginx-devel mailing list