[Patch] proxy cache for 304 Not Modified
MagicBear
magicbearmo at gmail.com
Wed Sep 21 20:33:20 UTC 2011
2011/9/22 Maxim Dounin <mdounin at mdounin.ru>:
> Hello!
>
> On Tue, Sep 20, 2011 at 12:47:56AM +0800, MagicBear wrote:
>
[...]
>
> I don't think that using validity time from 200 is correct either.
> What happens if response wasn't 200 (e.g. 206)? What happens if
> response was cached up to time specified in Cache-Control
> header?
>
> You may want to actually use ngx_http_file_cache_valid() with
> original response status code (i.e. after
> ngx_http_upstream_cache_send()), and only if valid_sec wasn't set
> by original response header parsing.
>
> This may also require resetting valid_sec to 0 before
> ngx_http_file_cache_valid() (not sure, needs checking).
>
> But see below, I still want to see rationale based on RFC2616.
>
I have checked for this, valid_sec will automate call the
ngx_http_upstream_process_cache_control for updating valid_sec when
server has return the Cache-Control headers, now I has change it to
when the server hasn't return the Cache-Control, it will call
ngx_http_file_cache_valid for getting original headers code valid sec
to modified.
[...]
> There are still multiple places where spaces aren't used where
> they should be. In particular, exactly this line in new patch
> still have no space after ",".
>
I think I must be post a wrong patch file, just I check the code, I
have found this has been modified, I am sorry for that.
> As I already outlined above, this will require open file cache
> interface changes.
>
But change all of this I think may be have a problem at this usage:
A server use for proxy and have script running.
When the cache has update by script, and nginx has open it with write,
at windows os may be have some problem. (NOT confirmed). So last time
I keep it at original method.
[...]
>
> I don't see any answers here.
I have check for this, the upstream will call
ngx_http_upstream_process_cache_control automate to change the
valid_sec.
[...]
>
> And this.
>
> Addtionally, space is missing after "," in offsetof().
>
> You may also want to line up continuation lines with arguments in
> first line. Also to improve readability it makes sense to use
> separate variable for return code.
>
> Resulting code will look like this:
>
> rc = ngx_write_file(&file, (u_char *) &r->cache->valid_sec,
> sizeof(r->cache->valid_sec),
> offsetof(ngx_http_file_cache_header_t, valid_sec));
>
> if (rc == NGX_ERROR) {
> ...
> }
>
>> + {
>> + ngx_log_error(NGX_LOG_EMERG, r->connection->log, ngx_errno,
>> + "write proxy_cache \"%s\" failed",
>> r->cache->file.name.data);
>
> Again, more than 80 chars.
>
This time's patch has fixed this, thanks for your advise.
>> + return;
>> + }
>> +
>> + if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
>> + ngx_log_error(NGX_LOG_ALERT, r->connection->log, ngx_errno,
>> + ngx_close_file_n " \"%s\" failed",
>> r->cache->file.name.data);
>
> And here.
>
> Setting cache_status to NGX_HTTP_CACHE_EXPIRED useless, it's
> already set.
>
Before I have change the cache_status to another status for checking
when I was update, and before publish I have change it to
NGX_HTTP_CACHE_EXPIRED, but I forgot it has already has this status.
[...]
> Please line up "sizeof" with "r->pool". Code in
> ngx_http_variable_sent_last_modified() uses much longer lines and
> deviates from this rule to preserve line length under 80 chars.
>
This patch has changed.
[...]
>
> Additional question to consider:
>
> What happens with the cache file created by the If-Modified-Since
> request? It should be freed properly, and I don't immediately see
> where it happens in the patch.
>
I have open the file only when cache without CREATE flags, so when the
cache not exists, It will direct return without create a empty cache
file.
> Maxim Dounin
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
This time I have provide two version of this patch to test.
One is update the file using open the original cache.
Full file: http://m-b.cc/share/patch-nginx-proxy-304-2.txt
diff -ruNp a/src/http/modules/ngx_http_proxy_module.c
nginx-1.1.3/src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c 2011-09-16
02:13:16.274428192 +0800
+++ nginx-1.1.3/src/http/modules/ngx_http_proxy_module.c 2011-09-22
03:37:22.374306318 +0800
@@ -543,7 +543,7 @@ static ngx_keyval_t ngx_http_proxy_cach
{ ngx_string("Connection"), ngx_string("close") },
{ ngx_string("Keep-Alive"), ngx_string("") },
{ ngx_string("Expect"), ngx_string("") },
- { ngx_string("If-Modified-Since"), ngx_string("") },
+ { ngx_string("If-Modified-Since"), ngx_string("$upstream_last_modified") },
{ ngx_string("If-Unmodified-Since"), ngx_string("") },
{ ngx_string("If-None-Match"), ngx_string("") },
{ ngx_string("If-Match"), ngx_string("") },
diff -ruNp a/src/http/ngx_http_cache.h nginx-1.1.3/src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h 2011-07-29 23:09:02.000000000 +0800
+++ nginx-1.1.3/src/http/ngx_http_cache.h 2011-09-22 03:37:22.374306318 +0800
@@ -133,6 +133,7 @@ ngx_int_t ngx_http_file_cache_create(ngx
void ngx_http_file_cache_create_key(ngx_http_request_t *r);
ngx_int_t ngx_http_file_cache_open(ngx_http_request_t *r);
void ngx_http_file_cache_set_header(ngx_http_request_t *r, u_char *buf);
+void ngx_http_file_cache_set_valid(ngx_http_request_t *r);
void ngx_http_file_cache_update(ngx_http_request_t *r, ngx_temp_file_t *tf);
ngx_int_t ngx_http_cache_send(ngx_http_request_t *);
void ngx_http_file_cache_free(ngx_http_cache_t *c, ngx_temp_file_t *tf);
diff -ruNp a/src/http/ngx_http_file_cache.c
nginx-1.1.3/src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c 2011-08-26 01:29:34.000000000 +0800
+++ nginx-1.1.3/src/http/ngx_http_file_cache.c 2011-09-22
04:32:27.000000000 +0800
@@ -767,6 +767,46 @@ ngx_http_file_cache_set_header(ngx_http_
void
+ngx_http_file_cache_set_valid(ngx_http_request_t *r)
+{
+ ngx_file_t file;
+ ngx_int_t rc;
+
+ ngx_memzero(&file, sizeof(ngx_file_t));
+
+ file.name = r->cache->file.name;
+ file.log = r->connection->log;
+
+ file.fd = ngx_open_file(file.name.data, NGX_FILE_RDWR,
+ NGX_FILE_OPEN, NGX_FILE_DEFAULT_ACCESS);
+
+ if (file.fd == NGX_INVALID_FILE) {
+ ngx_log_error(NGX_LOG_EMERG, r->connection->log, ngx_errno,
+ ngx_open_file_n " \"%s\" failed",
+ r->cache->file.name.data);
+ return;
+ }
+
+ rc = ngx_write_file(&file, (u_char *) &r->cache->valid_sec,
+ sizeof(r->cache->valid_sec),
+ offsetof(ngx_http_file_cache_header_t, valid_sec));
+ if (rc == NGX_ERROR)
+ {
+ ngx_log_error(NGX_LOG_EMERG, r->connection->log, ngx_errno,
+ "write proxy_cache \"%s\" failed",
+ r->cache->file.name.data);
+ return;
+ }
+
+ if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
+ ngx_log_error(NGX_LOG_ALERT, r->connection->log, ngx_errno,
+ ngx_close_file_n " \"%s\" failed",
+ r->cache->file.name.data);
+ }
+}
+
+
+void
ngx_http_file_cache_update(ngx_http_request_t *r, ngx_temp_file_t *tf)
{
off_t fs_size;
diff -ruNp a/src/http/ngx_http_upstream.c
nginx-1.1.3/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c 2011-09-16 02:13:16.274428192 +0800
+++ nginx-1.1.3/src/http/ngx_http_upstream.c 2011-09-22 04:23:58.000000000 +0800
@@ -16,6 +16,8 @@ static ngx_int_t ngx_http_upstream_cache
ngx_http_upstream_t *u);
static ngx_int_t ngx_http_upstream_cache_status(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_upstream_last_modified(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);
@@ -342,6 +344,10 @@ static ngx_http_variable_t ngx_http_ups
ngx_http_upstream_cache_status, 0,
NGX_HTTP_VAR_NOCACHEABLE, 0 },
+ { ngx_string("upstream_last_modified"), NULL,
+ ngx_http_upstream_last_modified, 0,
+ NGX_HTTP_VAR_NOCACHEABLE, 0 },
+
#endif
{ ngx_null_string, NULL, NULL, 0, 0, 0 }
@@ -1682,6 +1688,33 @@ ngx_http_upstream_test_next(ngx_http_req
status = u->headers_in.status_n;
+#if (NGX_HTTP_CACHE)
+
+ if (u->cache_status == NGX_HTTP_CACHE_EXPIRED &&
+ status == NGX_HTTP_NOT_MODIFIED)
+ {
+ ngx_int_t rc;
+ time_t valid;
+
+ rc = u->reinit_request(r);
+
+ if (rc == NGX_OK) {
+ rc = ngx_http_upstream_cache_send(r, u);
+
+ if (r->cache->valid_sec == 0) {
+ valid =
ngx_http_file_cache_valid(u->conf->cache_valid,
u->headers_in.status_n);
+ r->cache->valid_sec = ngx_time() + valid;
+ }
+
+ ngx_http_file_cache_set_valid(r);
+
+ ngx_http_upstream_finalize_request(r, u, rc);
+ return NGX_OK;
+ }
+ }
+
+#endif
+
for (un = ngx_http_upstream_next_errors; un->status; un++) {
if (status != un->status) {
@@ -4006,6 +4039,33 @@ ngx_http_upstream_cache_status(ngx_http_
return NGX_OK;
}
+
+
+ngx_int_t
+ngx_http_upstream_last_modified(ngx_http_request_t *r,
+ ngx_http_variable_value_t *v, uintptr_t data)
+{
+ u_char *p;
+
+ if (r->upstream == NULL || r->upstream->cache_status == 0 ||
+ r->cache->last_modified == 0) {
+ v->not_found = 1;
+ return NGX_OK;
+ }
+
+ p = ngx_pcalloc(r->pool, sizeof("Mon, 28 Sep 1970 06:00:00 GMT") - 1);
+ if (p == NULL) {
+ return NGX_ERROR;
+ }
+
+ v->len = ngx_http_time(p, r->cache->last_modified) - p;
+ v->valid = 1;
+ v->no_cacheable = 0;
+ v->not_found = 0;
+ v->data = p;
+
+ return NGX_OK;
+}
#endif
The other one is open the file with via RW permission
Full file: http://m-b.cc/share/patch-nginx-proxy-304-3.txt
diff -ruNp a/src/core/ngx_open_file_cache.c
nginx-1.1.3/src/core/ngx_open_file_cache.c
--- a/src/core/ngx_open_file_cache.c 2011-09-14 22:28:55.000000000 +0800
+++ nginx-1.1.3/src/core/ngx_open_file_cache.c 2011-09-22
03:48:50.000000000 +0800
@@ -495,7 +495,7 @@ ngx_open_and_stat_file(u_char *name, ngx
* This flag has no effect on a regular files.
*/
- fd = ngx_open_file(name, NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
+ fd = ngx_open_file(name, NGX_FILE_RDWR|NGX_FILE_NONBLOCK,
NGX_FILE_OPEN, 0);
} else {
diff -ruNp a/src/http/modules/ngx_http_proxy_module.c
nginx-1.1.3/src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c 2011-09-16
02:13:16.274428192 +0800
+++ nginx-1.1.3/src/http/modules/ngx_http_proxy_module.c 2011-09-22
03:37:22.374306318 +0800
@@ -543,7 +543,7 @@ static ngx_keyval_t ngx_http_proxy_cach
{ ngx_string("Connection"), ngx_string("close") },
{ ngx_string("Keep-Alive"), ngx_string("") },
{ ngx_string("Expect"), ngx_string("") },
- { ngx_string("If-Modified-Since"), ngx_string("") },
+ { ngx_string("If-Modified-Since"), ngx_string("$upstream_last_modified") },
{ ngx_string("If-Unmodified-Since"), ngx_string("") },
{ ngx_string("If-None-Match"), ngx_string("") },
{ ngx_string("If-Match"), ngx_string("") },
diff -ruNp a/src/http/ngx_http_cache.h nginx-1.1.3/src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h 2011-07-29 23:09:02.000000000 +0800
+++ nginx-1.1.3/src/http/ngx_http_cache.h 2011-09-22 03:37:22.374306318 +0800
@@ -133,6 +133,7 @@ ngx_int_t ngx_http_file_cache_create(ngx
void ngx_http_file_cache_create_key(ngx_http_request_t *r);
ngx_int_t ngx_http_file_cache_open(ngx_http_request_t *r);
void ngx_http_file_cache_set_header(ngx_http_request_t *r, u_char *buf);
+void ngx_http_file_cache_set_valid(ngx_http_request_t *r);
void ngx_http_file_cache_update(ngx_http_request_t *r, ngx_temp_file_t *tf);
ngx_int_t ngx_http_cache_send(ngx_http_request_t *);
void ngx_http_file_cache_free(ngx_http_cache_t *c, ngx_temp_file_t *tf);
diff -ruNp a/src/http/ngx_http_file_cache.c
nginx-1.1.3/src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c 2011-08-26 01:29:34.000000000 +0800
+++ nginx-1.1.3/src/http/ngx_http_file_cache.c 2011-09-22
04:06:59.000000000 +0800
@@ -767,6 +767,31 @@ ngx_http_file_cache_set_header(ngx_http_
void
+ngx_http_file_cache_set_valid(ngx_http_request_t *r)
+{
+ ngx_int_t rc;
+
+ if (r->cache->file.fd == NGX_INVALID_FILE) {
+ ngx_log_error(NGX_LOG_EMERG, r->connection->log, ngx_errno,
+ ngx_open_file_n " \"%s\" failed",
+ r->cache->file.name.data);
+ return;
+ }
+
+ rc = ngx_write_file(&r->cache->file, (u_char *) &r->cache->valid_sec,
+ sizeof(r->cache->valid_sec),
+ offsetof(ngx_http_file_cache_header_t, valid_sec));
+ if (rc == NGX_ERROR)
+ {
+ ngx_log_error(NGX_LOG_EMERG, r->connection->log, ngx_errno,
+ "write proxy_cache \"%s\" failed",
+ r->cache->file.name.data);
+ return;
+ }
+}
+
+
+void
ngx_http_file_cache_update(ngx_http_request_t *r, ngx_temp_file_t *tf)
{
off_t fs_size;
diff -ruNp a/src/http/ngx_http_upstream.c
nginx-1.1.3/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c 2011-09-16 02:13:16.274428192 +0800
+++ nginx-1.1.3/src/http/ngx_http_upstream.c 2011-09-22 04:23:58.000000000 +0800
@@ -16,6 +16,8 @@ static ngx_int_t ngx_http_upstream_cache
ngx_http_upstream_t *u);
static ngx_int_t ngx_http_upstream_cache_status(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_upstream_last_modified(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);
@@ -342,6 +344,10 @@ static ngx_http_variable_t ngx_http_ups
ngx_http_upstream_cache_status, 0,
NGX_HTTP_VAR_NOCACHEABLE, 0 },
+ { ngx_string("upstream_last_modified"), NULL,
+ ngx_http_upstream_last_modified, 0,
+ NGX_HTTP_VAR_NOCACHEABLE, 0 },
+
#endif
{ ngx_null_string, NULL, NULL, 0, 0, 0 }
@@ -1682,6 +1688,33 @@ ngx_http_upstream_test_next(ngx_http_req
status = u->headers_in.status_n;
+#if (NGX_HTTP_CACHE)
+
+ if (u->cache_status == NGX_HTTP_CACHE_EXPIRED &&
+ status == NGX_HTTP_NOT_MODIFIED)
+ {
+ ngx_int_t rc;
+ time_t valid;
+
+ rc = u->reinit_request(r);
+
+ if (rc == NGX_OK) {
+ rc = ngx_http_upstream_cache_send(r, u);
+
+ if (r->cache->valid_sec == 0) {
+ valid =
ngx_http_file_cache_valid(u->conf->cache_valid,
u->headers_in.status_n);
+ r->cache->valid_sec = ngx_time() + valid;
+ }
+
+ ngx_http_file_cache_set_valid(r);
+
+ ngx_http_upstream_finalize_request(r, u, rc);
+ return NGX_OK;
+ }
+ }
+
+#endif
+
for (un = ngx_http_upstream_next_errors; un->status; un++) {
if (status != un->status) {
@@ -4006,6 +4039,33 @@ ngx_http_upstream_cache_status(ngx_http_
return NGX_OK;
}
+
+
+ngx_int_t
+ngx_http_upstream_last_modified(ngx_http_request_t *r,
+ ngx_http_variable_value_t *v, uintptr_t data)
+{
+ u_char *p;
+
+ if (r->upstream == NULL || r->upstream->cache_status == 0 ||
+ r->cache->last_modified == 0) {
+ v->not_found = 1;
+ return NGX_OK;
+ }
+
+ p = ngx_pcalloc(r->pool, sizeof("Mon, 28 Sep 1970 06:00:00 GMT") - 1);
+ if (p == NULL) {
+ return NGX_ERROR;
+ }
+
+ v->len = ngx_http_time(p, r->cache->last_modified) - p;
+ v->valid = 1;
+ v->no_cacheable = 0;
+ v->not_found = 0;
+ v->data = p;
+
+ return NGX_OK;
+}
#endif
Need to confirm is the patch-nginx-proxy-304-3.txt patch work for
windows version? I haven't windows compile tools here. So cannot test
it.
MagicBear
More information about the nginx-devel
mailing list