Re: [PATCH] Upstream: add $upstream_cache_age variable

2014-10-20 Thread Piotr Sikora
Hey Roman,
sorry for late reply.

 Another issue i’ve found reading the RFC.

 If the reason for $upstream_cache_age is adding HTTP Age response header then 
 it makes
 sense to improve age calculation algorithm.

It is and it isn't ;)

There are two use cases for this variable:

1. Exposing it as Age or X-Cache-Age HTTP response header. The RFC
describes Age as the time that passed since the response was
generated at the origin server, taking into account the information
from the intermediary proxies (in practice - just the previous one).
As you've already noticed, this variable shows time that passed since
the response was received (or revalidated) and ignores information
from the intermediary proxies, so using it for as the Age header
isn't strictly RFC-conformant, but it's good enough and it's
actually what most people we've been dealing with expect to see.

2. Logging of the cached file's age to the access logs. The more
information you have in the logs when you need them, the better...
That should go without saying ;)

Best regards,
Piotr Sikora

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Upstream: add $upstream_cache_age variable

2014-10-06 Thread Maxim Dounin
Hello!

On Mon, Oct 06, 2014 at 02:10:18PM +0400, Roman Arutyunyan wrote:

[...]

 Another issue i’ve found reading the RFC.
 
 If the reason for $upstream_cache_age is adding HTTP Age response header then 
 it makes
 sense to improve age calculation algorithm.
 
 According to RFC 7234 / 4.2.3. Calculating Age” the local cache age should 
 be added
 to the Age value received from backend (“age_value).
 
 response_delay = response_time - request_time;
 corrected_age_value = age_value + response_delay;

I agree, I don't think that the date value as stored in the 
cache can/should be used for Age header.  And I doubt it should be 
exposed as a variable at all, even if calculated properly.

-- 
Maxim Dounin
http://nginx.org/

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Upstream: add $upstream_cache_age variable

2014-10-03 Thread Roman Arutyunyan

On 03 Oct 2014, at 01:22, Piotr Sikora pi...@cloudflare.com wrote:

 # HG changeset patch
 # User Piotr Sikora pi...@cloudflare.com
 # Date 1412284916 25200
 #  Thu Oct 02 14:21:56 2014 -0700
 # Node ID 488aa8f86041dbe99a8f8ebbb3df533b9ce3e19e
 # Parent  a215d9021f137b9e2d4f69c37c7f992a2bef12c6
 Upstream: add $upstream_cache_age variable.
 
 This variable represents the amount of time that passed since the response
 was fetched (or successfully revalidated) from the upstream server.
 
 Signed-off-by: Piotr Sikora pi...@cloudflare.com
 
 diff -r a215d9021f13 -r 488aa8f86041 src/http/ngx_http_upstream.c
 --- a/src/http/ngx_http_upstream.cTue Sep 30 17:20:33 2014 +0400
 +++ b/src/http/ngx_http_upstream.cThu Oct 02 14:21:56 2014 -0700
 @@ -21,6 +21,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);
 @@ -373,6 +375,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_null_string, NULL, NULL, 0, 0, 0 }
 @@ -4851,6 +4857,32 @@ 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)
 +{
 +if (r-upstream == NULL
 +|| r-upstream-cache_status = NGX_HTTP_CACHE_EXPIRED
 +|| r-upstream-cache_status == NGX_HTTP_CACHE_REVALIDATED)
 +{
 +v-not_found = 1;
 +return NGX_OK;
 +}
 +

Probably it would be more clear to list the valid 3 statuses here.
One extra operation though.

if (r-upstream == NULL) {
v-not_found = 1;
return NGX_OK;
}

if (r-upstream-cache_status != NGX_HTTP_CACHE_STALE
 r-upstream-cache_status != NGX_HTTP_CACHE_UPDATING
 r-upstream-cache_status != NGX_HTTP_CACHE_HIT)
{
v-not_found = 1;
return NGX_OK;
}

 +v-data = ngx_pnalloc(r-pool, NGX_TIME_T_LEN);
 +if (v-data == NULL) {
 +return NGX_ERROR;
 +}
 +
 +v-len = ngx_sprintf(v-data, %T, ngx_time() - r-cache-date) - 
 v-data;
 +v-valid = 1;
 +v-no_cacheable = 0;
 +v-not_found = 0;
 +
 +return NGX_OK;
 +}
 +
 #endif
 
 
 
 ___
 nginx-devel mailing list
 nginx-devel@nginx.org
 http://mailman.nginx.org/mailman/listinfo/nginx-devel
 

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel