Hi,
On Thu, Jun 13, 2024 at 03:29:02PM -0700, Aleksei Bavshin wrote:
> # HG changeset patch
> # User Mini Hawthorne <m...@f5.com>
> # Date 1689189645 25200
> # Wed Jul 12 12:20:45 2023 -0700
> # Node ID 8c8d8118c7ac0a0426f48dbfed94e279dddff992
> # Parent 621ba257aeac3017ea83b24fafa201e07c1c7756
> Upstream: copy upstream zone DNS valid time during config reload.
>
> Previously, all upstream DNS entries would be immediately re-resolved
> on config reload. With a large number of upstreams, this creates
> a spike of DNS resolution requests. These spikes can overwhelm the
> DNS server or cause drops on the network.
>
> This patch retains the TTL of previous resolutions across reloads
> by copying each upstream's name's expiry time across configuration
> cycles. As a result, no additional resolutions are needed.
>
> diff --git a/src/http/modules/ngx_http_upstream_zone_module.c
> b/src/http/modules/ngx_http_upstream_zone_module.c
> --- a/src/http/modules/ngx_http_upstream_zone_module.c
> +++ b/src/http/modules/ngx_http_upstream_zone_module.c
> @@ -443,6 +443,8 @@ ngx_http_upstream_zone_copy_peer(ngx_htt
> ngx_memcpy(dst->host->service.data, src->host->service.data,
> src->host->service.len);
> }
> +
> + dst->host->valid = src->host->valid;
> }
> }
>
> @@ -547,6 +549,8 @@ ngx_http_upstream_zone_preresolve(ngx_ht
>
> peer->host = template->host;
>
> + template->host->valid = host->valid;
> +
> server = template->host->service.len ? &opeer->server
> : &template->server;
>
> @@ -694,6 +698,8 @@ ngx_http_upstream_zone_init_worker(ngx_c
> static void
> ngx_http_upstream_zone_resolve_timer(ngx_event_t *event)
> {
> + time_t now, valid;
> + ngx_msec_t timer;
> ngx_resolver_ctx_t *ctx;
> ngx_http_upstream_host_t *host;
> ngx_http_upstream_rr_peer_t *template;
> @@ -705,6 +711,9 @@ ngx_http_upstream_zone_resolve_timer(ngx
> peers = host->peers;
> template = host->peer;
>
> + now = ngx_time();
> + valid = host->valid;
> +
> if (template->zombie) {
> (void) ngx_http_upstream_rr_peer_unref(peers, template);
>
> @@ -721,6 +730,10 @@ ngx_http_upstream_zone_resolve_timer(ngx
> return;
> }
>
> + if (valid > now) {
> + goto retry;
> + }
> +
> ctx = ngx_resolve_start(uscf->resolver, NULL);
> if (ctx == NULL) {
> goto retry;
> @@ -745,7 +758,11 @@ ngx_http_upstream_zone_resolve_timer(ngx
>
> retry:
>
> - ngx_add_timer(event, ngx_max(uscf->resolver_timeout, 1000));
> + /* don't delay zombie cleanup longer than resolver_timeout */
> + timer = (ngx_msec_t) 1000 * (valid > now ? valid - now + 1 : 1);
> + timer = ngx_min(timer, uscf->resolver_timeout);
In open source nginx we have no zombies to clean up, which means
1. the comment above should be removed
2. when valid > now, we should wait exactly (valid - now)
Probably it's easier to leave this code as is and add a separate ngx_add_timer()
to the block above.
An even better solution is to schedule the right timeout in
ngx_http_upstream_zone_init_worker() for the case when valid > now instead of 1.
> +
> + ngx_add_timer(event, ngx_max(timer, 1000));
> }
>
>
> @@ -1026,6 +1043,8 @@ again:
>
> done:
>
> + host->valid = ctx->valid;
> +
> ngx_http_upstream_rr_peers_unlock(peers);
>
> while (++i < ctx->naddrs) {
> diff --git a/src/http/ngx_http_upstream_round_robin.h
> b/src/http/ngx_http_upstream_round_robin.h
> --- a/src/http/ngx_http_upstream_round_robin.h
> +++ b/src/http/ngx_http_upstream_round_robin.h
> @@ -25,6 +25,7 @@ typedef struct {
> ngx_uint_t worker;
> ngx_str_t name;
> ngx_str_t service;
> + time_t valid;
> ngx_http_upstream_rr_peers_t *peers;
> ngx_http_upstream_rr_peer_t *peer;
> } ngx_http_upstream_host_t;
> diff --git a/src/stream/ngx_stream_upstream_round_robin.h
> b/src/stream/ngx_stream_upstream_round_robin.h
> --- a/src/stream/ngx_stream_upstream_round_robin.h
> +++ b/src/stream/ngx_stream_upstream_round_robin.h
> @@ -25,6 +25,7 @@ typedef struct {
> ngx_uint_t worker;
> ngx_str_t name;
> ngx_str_t service;
> + time_t valid;
> ngx_stream_upstream_rr_peers_t *peers;
> ngx_stream_upstream_rr_peer_t *peer;
> } ngx_stream_upstream_host_t;
> diff --git a/src/stream/ngx_stream_upstream_zone_module.c
> b/src/stream/ngx_stream_upstream_zone_module.c
> --- a/src/stream/ngx_stream_upstream_zone_module.c
> +++ b/src/stream/ngx_stream_upstream_zone_module.c
> @@ -544,6 +544,8 @@ ngx_stream_upstream_zone_preresolve(ngx_
>
> peer->host = template->host;
>
> + template->host->valid = host->valid;
> +
> server = template->host->service.len ? &opeer->server
> : &template->server;
>
> @@ -692,6 +694,8 @@ ngx_stream_upstream_zone_init_worker(ngx
> static void
> ngx_stream_upstream_zone_resolve_timer(ngx_event_t *event)
> {
> + time_t now, valid;
> + ngx_msec_t timer;
> ngx_resolver_ctx_t *ctx;
> ngx_stream_upstream_host_t *host;
> ngx_stream_upstream_rr_peer_t *template;
> @@ -703,6 +707,9 @@ ngx_stream_upstream_zone_resolve_timer(n
> peers = host->peers;
> template = host->peer;
>
> + now = ngx_time();
> + valid = host->valid;
> +
> if (template->zombie) {
> (void) ngx_stream_upstream_rr_peer_unref(peers, template);
>
> @@ -719,6 +726,10 @@ ngx_stream_upstream_zone_resolve_timer(n
> return;
> }
>
> + if (valid > now) {
> + goto retry;
> + }
> +
> ctx = ngx_resolve_start(uscf->resolver, NULL);
> if (ctx == NULL) {
> goto retry;
> @@ -743,7 +754,11 @@ ngx_stream_upstream_zone_resolve_timer(n
>
> retry:
>
> - ngx_add_timer(event, ngx_max(uscf->resolver_timeout, 1000));
> + /* don't delay zombie cleanup longer than resolver_timeout */
> + timer = (ngx_msec_t) 1000 * (valid > now ? valid - now + 1 : 1);
> + timer = ngx_min(timer, uscf->resolver_timeout);
> +
>Here + ngx_add_timer(event, ngx_max(timer, 1000));
Same here.
> }
>
>
> @@ -1024,6 +1039,8 @@ again:
>
> done:
>
> + host->valid = ctx->valid;
> +
> ngx_stream_upstream_rr_peers_unlock(peers);
>
> while (++i < ctx->naddrs) {
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
