Hello!
On Thu, May 23, 2024 at 10:52:13AM +0100, Kirill A. Korinsky wrote:
> Greetings,
>
> Here is a patch that exposes the constructed cache key as
> $upstream_cache_key variable.
>
> Sometimes it's quite useful when debugging complicated setups and fighting
> some typos.
Thanks for the patch.
I have no objections in general, see below for some minor
comments.
>
> index 2ce9f2114..561108681 100644
> --- src/http/ngx_http_upstream.c
> +++ src/http/ngx_http_upstream.c
> @@ -23,6 +23,8 @@ static ngx_int_t
> ngx_http_upstream_cache_check_range(ngx_http_request_t *r,
> 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_cache_key(ngx_http_request_t *r,
> + ngx_http_variable_value_t *v, uintptr_t data);
> static ngx_int_t ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t data);
> static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r,
> @@ -414,6 +416,10 @@ static ngx_http_variable_t ngx_http_upstream_vars[] = {
> ngx_http_upstream_cache_status, 0,
> NGX_HTTP_VAR_NOCACHEABLE, 0 },
>
> + { ngx_string("upstream_cache_key"), NULL,
> + ngx_http_upstream_cache_key, 0,
> + NGX_HTTP_VAR_NOCACHEABLE, 0 },
> +
> { ngx_string("upstream_cache_last_modified"), NULL,
> ngx_http_upstream_cache_last_modified, 0,
> NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
> @@ -5990,6 +5996,47 @@ ngx_http_upstream_cache_status(ngx_http_request_t *r,
> }
>
>
> +static ngx_int_t
> +ngx_http_upstream_cache_key(ngx_http_request_t *r,
> + ngx_http_variable_value_t *v, uintptr_t data)
> +{
> + size_t len;
> + ngx_uint_t i;
> + ngx_str_t *key;
Nitpicking: wrong variables order, should be sorted by type length.
> + ngx_http_cache_t *c;
> +
> + if (r->cache == NULL || r->cache->keys.nelts == 0) {
> + v->not_found = 1;
> + return NGX_OK;
> + }
> +
> + c = r->cache;
> +
> + key = c->keys.elts;
> + len = 0;
> + for (i = 0; i < c->keys.nelts; i++) {
> + len += key[i].len;
> + }
> +
> + v->data = ngx_pcalloc(r->pool, len);
There is no need to use ngx_pcalloc() here: the memory is anyway
to be written to, so just normal ngx_palloc(), without "c", would
be enough. Further, given the result is a string, no alignment is
needed, so ngx_pnalloc() would be appropriate.
> + if (v->data == 0) {
In [free]nginx code, NULL is used in pointer contexts.
> + return NGX_ERROR;
> + }
> +
> + v->len = 0;
> + for (i = 0; i < c->keys.nelts; i++) {
> + memcpy(v->data + v->len, key[i].data, key[i].len);
> + v->len += key[i].len;
Usual approach for such loops is doing something like
p = ngx_cpymem(p, key[i].data, key[i].len);
This avoids an extra addition per iteration, and generally easier
to read.
(Also, using memcpy() directly is discouraged, it should be either
ngx_memcpy() or ngx_cpymem().)
> + }
> +
> + v->valid = 1;
> + v->no_cacheable = 0;
> + v->not_found = 0;
> +
> + return NGX_OK;
> +}
> +
> +
> static ngx_int_t
> ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t data)
>
Patch with the above comments incorporated, please take a look:
# HG changeset patch
# User Kirill A. Korinsky <kir...@korins.ky>
# Date 1716479312 -10800
# Thu May 23 18:48:32 2024 +0300
# Node ID bd920ccd6f1aa43fcb40507ba463e44548ca4676
# Parent 46ecad404a296042c0088e699f275a92758e5ab9
Upstream: $upstream_cache_key variable.
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
@@ -23,6 +23,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_cache_key(ngx_http_request_t *r,
+ ngx_http_variable_value_t *v, uintptr_t data);
static ngx_int_t ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data);
static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r,
@@ -414,6 +416,10 @@ static ngx_http_variable_t ngx_http_ups
ngx_http_upstream_cache_status, 0,
NGX_HTTP_VAR_NOCACHEABLE, 0 },
+ { ngx_string("upstream_cache_key"), NULL,
+ ngx_http_upstream_cache_key, 0,
+ NGX_HTTP_VAR_NOCACHEABLE, 0 },
+
{ ngx_string("upstream_cache_last_modified"), NULL,
ngx_http_upstream_cache_last_modified, 0,
NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
@@ -6004,6 +6010,49 @@ ngx_http_upstream_cache_status(ngx_http_
static ngx_int_t
+ngx_http_upstream_cache_key(ngx_http_request_t *r,
+ ngx_http_variable_value_t *v, uintptr_t data)
+{
+ u_char *p;
+ size_t len;
+ ngx_str_t *key;
+ ngx_uint_t i;
+ ngx_http_cache_t *c;
+
+ if (r->cache == NULL || r->cache->keys.nelts == 0) {
+ v->not_found = 1;
+ return NGX_OK;
+ }
+
+ c = r->cache;
+
+ len = 0;
+ key = c->keys.elts;
+
+ for (i = 0; i < c->keys.nelts; i++) {
+ len += key[i].len;
+ }
+
+ p = ngx_pnalloc(r->pool, len);
+ if (p == NULL) {
+ return NGX_ERROR;
+ }
+
+ v->len = len;
+ v->valid = 1;
+ v->no_cacheable = 0;
+ v->not_found = 0;
+ v->data = p;
+
+ for (i = 0; i < c->keys.nelts; i++) {
+ p = ngx_cpymem(p, key[i].data, key[i].len);
+ }
+
+ return NGX_OK;
+}
+
+
+static ngx_int_t
ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data)
{
--
Maxim Dounin
http://mdounin.ru/
--
nginx mailing list
nginx@freenginx.org
https://freenginx.org/mailman/listinfo/nginx
