[PATCH 1 of 9] Upstream: re-resolvable servers
Aleksei Bavshin
a.bavshin at nginx.com
Tue Jul 9 20:21:18 UTC 2024
On 7/8/2024 7:20 AM, Roman Arutyunyan wrote:
> Hi,
>
> On Thu, Jun 13, 2024 at 03:28:56PM -0700, Aleksei Bavshin wrote:
>> # HG changeset patch
>> # User Ruslan Ermilov <ru at nginx.com>
>> # Date 1392462754 -14400
>> # Sat Feb 15 15:12:34 2014 +0400
>> # Node ID 56aeae9355df8a2ee07e21b65b6869747dd9ee45
>> # Parent 02e9411009b987f408214ab4a8b6b6093f843bcd
>> Upstream: re-resolvable servers.
>>
>> Specifying the upstream server by a hostname together with the
>> "resolve" parameter will make the hostname to be periodically
>> resolved, and upstream servers added/removed as necessary.
>>
>> This requires a "resolver" at the "http" configuration block.
>>
>> The "resolver_timeout" parameter also affects when the failed
>> DNS requests will be attempted again. Responses with NXDOMAIN
>> will be attempted again in 10 seconds.
>>
>> Upstream has a configuration generation number that is incremented each
>> time servers are added/removed to the primary/backup list. This number
>> is remembered by the peer.init method, and if peer.get detects a change
>> in configuration, it returns NGX_BUSY.
>>
>> Each server has a reference counter. It is incremented by peer.get and
>> decremented by peer.free. When a server is removed, it is removed from
>> the list of servers and is marked as "zombie". The memory allocated by
>> a zombie peer is freed only when its reference count becomes zero.
>>
>> Re-resolvable servers utilize timers that also hold a reference. A
>> reference is also held while upstream keepalive caches an idle
>> connection.
>>
>> Co-authored-by: Roman Arutyunyan <arut at nginx.com>
>> Co-authored-by: Sergey Kandaurov <pluknet at nginx.com>
>> Co-authored-by: Vladimir Homutov <vl at nginx.com>
>
> I feel like it would be easier to merge this patch, SRV resolve and preresolve
> in a single change.
>
I disagree here, because
* the patches represent large, significant and logically independent
pieces of work. I squashed a lot of changes into the initial patch, but
all of those were bugfixes or compatibility fixes for newer oss/plus code.
* the commit messages and the diffs give more context for developers who
don't have access to the original history but want to understand why
certain changes were made
* it's easier to track attribution
I'll merge patches 1 and 2 though, as that makes sense. And I'll make
sure to individually test unmerged patches again in the next revision.
>> diff --git a/src/http/modules/ngx_http_upstream_hash_module.c b/src/http/modules/ngx_http_upstream_hash_module.c
>> --- a/src/http/modules/ngx_http_upstream_hash_module.c
>> +++ b/src/http/modules/ngx_http_upstream_hash_module.c
>> @@ -24,6 +24,9 @@ typedef struct {
>>
>> typedef struct {
>> ngx_http_complex_value_t key;
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_uint_t config;
>> +#endif
>> ngx_http_upstream_chash_points_t *points;
>> } ngx_http_upstream_hash_srv_conf_t;
>>
>> @@ -49,6 +52,8 @@ static ngx_int_t ngx_http_upstream_get_h
>>
>> static ngx_int_t ngx_http_upstream_init_chash(ngx_conf_t *cf,
>> ngx_http_upstream_srv_conf_t *us);
>> +static ngx_int_t ngx_http_upstream_update_chash(ngx_pool_t *pool,
>> + ngx_http_upstream_srv_conf_t *us);
>> static int ngx_libc_cdecl
>> ngx_http_upstream_chash_cmp_points(const void *one, const void *two);
>> static ngx_uint_t ngx_http_upstream_find_chash_point(
>> @@ -178,11 +183,18 @@ ngx_http_upstream_get_hash_peer(ngx_peer
>>
>> ngx_http_upstream_rr_peers_rlock(hp->rrp.peers);
>>
>> - if (hp->tries > 20 || hp->rrp.peers->single || hp->key.len == 0) {
>> + if (hp->tries > 20 || hp->rrp.peers->number < 2 || hp->key.len == 0) {
>> ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>> return hp->get_rr_peer(pc, &hp->rrp);
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (hp->rrp.peers->config && hp->rrp.config != *hp->rrp.peers->config) {
>> + ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>> + return hp->get_rr_peer(pc, &hp->rrp);
>> + }
>> +#endif
>> +
>> now = ngx_time();
>>
>> pc->cached = 0;
>> @@ -262,6 +274,7 @@ ngx_http_upstream_get_hash_peer(ngx_peer
>> }
>>
>> hp->rrp.current = peer;
>> + ngx_http_upstream_rr_peer_ref(hp->rrp.peers, peer);
>>
>> pc->sockaddr = peer->sockaddr;
>> pc->socklen = peer->socklen;
>> @@ -285,6 +298,26 @@ ngx_http_upstream_get_hash_peer(ngx_peer
>> static ngx_int_t
>> ngx_http_upstream_init_chash(ngx_conf_t *cf, ngx_http_upstream_srv_conf_t *us)
>> {
>> + if (ngx_http_upstream_init_round_robin(cf, us) != NGX_OK) {
>> + return NGX_ERROR;
>> + }
>> +
>> + us->peer.init = ngx_http_upstream_init_chash_peer;
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (us->shm_zone) {
>> + return NGX_OK;
>> + }
>> +#endif
>> +
>> + return ngx_http_upstream_update_chash(cf->pool, us);
>> +}
>> +
>> +
>> +static ngx_int_t
>> +ngx_http_upstream_update_chash(ngx_pool_t *pool,
>> + ngx_http_upstream_srv_conf_t *us)
>> +{
>> u_char *host, *port, c;
>> size_t host_len, port_len, size;
>> uint32_t hash, base_hash;
>> @@ -299,25 +332,32 @@ ngx_http_upstream_init_chash(ngx_conf_t
>> u_char byte[4];
>> } prev_hash;
>>
>> - if (ngx_http_upstream_init_round_robin(cf, us) != NGX_OK) {
>> - return NGX_ERROR;
>> + hcf = ngx_http_conf_upstream_srv_conf(us, ngx_http_upstream_hash_module);
>> +
>> + if (hcf->points) {
>> + ngx_free(hcf->points);
>> + hcf->points = NULL;
>> }
>>
>> - us->peer.init = ngx_http_upstream_init_chash_peer;
>> -
>> peers = us->peer.data;
>> npoints = peers->total_weight * 160;
>>
>> size = sizeof(ngx_http_upstream_chash_points_t)
>> - + sizeof(ngx_http_upstream_chash_point_t) * (npoints - 1);
>> + - sizeof(ngx_http_upstream_chash_point_t)
>> + + sizeof(ngx_http_upstream_chash_point_t) * npoints;
>>
>> - points = ngx_palloc(cf->pool, size);
>> + points = pool ? ngx_palloc(pool, size) : ngx_alloc(size, ngx_cycle->log);
>> if (points == NULL) {
>> return NGX_ERROR;
>> }
>>
>> points->number = 0;
>>
>> + if (npoints == 0) {
>> + hcf->points = points;
>> + return NGX_OK;
>> + }
>> +
>> for (peer = peers->peer; peer; peer = peer->next) {
>> server = &peer->server;
>>
>> @@ -401,7 +441,6 @@ ngx_http_upstream_init_chash(ngx_conf_t
>>
>> points->number = i + 1;
>>
>> - hcf = ngx_http_conf_upstream_srv_conf(us, ngx_http_upstream_hash_module);
>> hcf->points = points;
>>
>> return NGX_OK;
>> @@ -481,7 +520,22 @@ ngx_http_upstream_init_chash_peer(ngx_ht
>>
>> ngx_http_upstream_rr_peers_rlock(hp->rrp.peers);
>>
>> - hp->hash = ngx_http_upstream_find_chash_point(hcf->points, hash);
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (hp->rrp.peers->config
>> + && (hcf->points == NULL || hcf->config != *hp->rrp.peers->config))
>> + {
>> + if (ngx_http_upstream_update_chash(NULL, us) != NGX_OK) {
>> + ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>> + return NGX_ERROR;
>> + }
>> +
>> + hcf->config = *hp->rrp.peers->config;
>> + }
>> +#endif
>> +
>> + if (hcf->points->number) {
>> + hp->hash = ngx_http_upstream_find_chash_point(hcf->points, hash);
>> + }
>>
>> ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>>
>> @@ -517,6 +571,20 @@ ngx_http_upstream_get_chash_peer(ngx_pee
>> pc->cached = 0;
>> pc->connection = NULL;
>>
>> + if (hp->rrp.peers->number == 0) {
>> + pc->name = hp->rrp.peers->name;
>> + ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>> + return NGX_BUSY;
>> + }
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (hp->rrp.peers->config && hp->rrp.config != *hp->rrp.peers->config) {
>> + pc->name = hp->rrp.peers->name;
>> + ngx_http_upstream_rr_peers_unlock(hp->rrp.peers);
>> + return NGX_BUSY;
>> + }
>> +#endif
>> +
>> now = ngx_time();
>> hcf = hp->conf;
>>
>> @@ -597,6 +665,7 @@ ngx_http_upstream_get_chash_peer(ngx_pee
>> found:
>>
>> hp->rrp.current = best;
>> + ngx_http_upstream_rr_peer_ref(hp->rrp.peers, best);
>>
>> pc->sockaddr = best->sockaddr;
>> pc->socklen = best->socklen;
>> @@ -664,6 +733,7 @@ ngx_http_upstream_hash(ngx_conf_t *cf, n
>> }
>>
>> uscf->flags = NGX_HTTP_UPSTREAM_CREATE
>> + |NGX_HTTP_UPSTREAM_MODIFY
>> |NGX_HTTP_UPSTREAM_WEIGHT
>> |NGX_HTTP_UPSTREAM_MAX_CONNS
>> |NGX_HTTP_UPSTREAM_MAX_FAILS
>> diff --git a/src/http/modules/ngx_http_upstream_ip_hash_module.c b/src/http/modules/ngx_http_upstream_ip_hash_module.c
>> --- a/src/http/modules/ngx_http_upstream_ip_hash_module.c
>> +++ b/src/http/modules/ngx_http_upstream_ip_hash_module.c
>> @@ -163,11 +163,19 @@ ngx_http_upstream_get_ip_hash_peer(ngx_p
>>
>> ngx_http_upstream_rr_peers_rlock(iphp->rrp.peers);
>>
>> - if (iphp->tries > 20 || iphp->rrp.peers->single) {
>> + if (iphp->tries > 20 || iphp->rrp.peers->number < 2) {
>> ngx_http_upstream_rr_peers_unlock(iphp->rrp.peers);
>> return iphp->get_rr_peer(pc, &iphp->rrp);
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (iphp->rrp.peers->config && iphp->rrp.config != *iphp->rrp.peers->config)
>> + {
>> + ngx_http_upstream_rr_peers_unlock(iphp->rrp.peers);
>> + return iphp->get_rr_peer(pc, &iphp->rrp);
>> + }
>> +#endif
>> +
>> now = ngx_time();
>>
>> pc->cached = 0;
>> @@ -232,6 +240,7 @@ ngx_http_upstream_get_ip_hash_peer(ngx_p
>> }
>>
>> iphp->rrp.current = peer;
>> + ngx_http_upstream_rr_peer_ref(iphp->rrp.peers, peer);
>>
>> pc->sockaddr = peer->sockaddr;
>> pc->socklen = peer->socklen;
>> @@ -268,6 +277,7 @@ ngx_http_upstream_ip_hash(ngx_conf_t *cf
>> uscf->peer.init_upstream = ngx_http_upstream_init_ip_hash;
>>
>> uscf->flags = NGX_HTTP_UPSTREAM_CREATE
>> + |NGX_HTTP_UPSTREAM_MODIFY
>> |NGX_HTTP_UPSTREAM_WEIGHT
>> |NGX_HTTP_UPSTREAM_MAX_CONNS
>> |NGX_HTTP_UPSTREAM_MAX_FAILS
>> diff --git a/src/http/modules/ngx_http_upstream_least_conn_module.c b/src/http/modules/ngx_http_upstream_least_conn_module.c
>> --- a/src/http/modules/ngx_http_upstream_least_conn_module.c
>> +++ b/src/http/modules/ngx_http_upstream_least_conn_module.c
>> @@ -124,6 +124,12 @@ ngx_http_upstream_get_least_conn_peer(ng
>>
>> ngx_http_upstream_rr_peers_wlock(peers);
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + goto busy;
>> + }
>> +#endif
>> +
>> best = NULL;
>> total = 0;
>>
>> @@ -244,6 +250,7 @@ ngx_http_upstream_get_least_conn_peer(ng
>> best->conns++;
>>
>> rrp->current = best;
>> + ngx_http_upstream_rr_peer_ref(peers, best);
>>
>> n = p / (8 * sizeof(uintptr_t));
>> m = (uintptr_t) 1 << p % (8 * sizeof(uintptr_t));
>> @@ -278,8 +285,18 @@ failed:
>> }
>>
>> ngx_http_upstream_rr_peers_wlock(peers);
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + goto busy;
>> + }
>> +#endif
>
> This block is useless. >
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> +busy:
>> +#endif
>> +
>> ngx_http_upstream_rr_peers_unlock(peers);
>>
>> pc->name = peers->name;
>> @@ -303,6 +320,7 @@ ngx_http_upstream_least_conn(ngx_conf_t
>> uscf->peer.init_upstream = ngx_http_upstream_init_least_conn;
>>
>> uscf->flags = NGX_HTTP_UPSTREAM_CREATE
>> + |NGX_HTTP_UPSTREAM_MODIFY
>> |NGX_HTTP_UPSTREAM_WEIGHT
>> |NGX_HTTP_UPSTREAM_MAX_CONNS
>> |NGX_HTTP_UPSTREAM_MAX_FAILS
>> diff --git a/src/http/modules/ngx_http_upstream_random_module.c b/src/http/modules/ngx_http_upstream_random_module.c
>> --- a/src/http/modules/ngx_http_upstream_random_module.c
>> +++ b/src/http/modules/ngx_http_upstream_random_module.c
>> @@ -17,6 +17,9 @@ typedef struct {
>>
>> typedef struct {
>> ngx_uint_t two;
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_uint_t config;
>> +#endif
>> ngx_http_upstream_random_range_t *ranges;
>> } ngx_http_upstream_random_srv_conf_t;
>>
>> @@ -127,6 +130,11 @@ ngx_http_upstream_update_random(ngx_pool
>>
>> rcf = ngx_http_conf_upstream_srv_conf(us, ngx_http_upstream_random_module);
>>
>> + if (rcf->ranges) {
>> + ngx_free(rcf->ranges);
>> + rcf->ranges = NULL;
>> + }
>> +
>> peers = us->peer.data;
>>
>> size = peers->number * sizeof(ngx_http_upstream_random_range_t);
>> @@ -186,11 +194,15 @@ ngx_http_upstream_init_random_peer(ngx_h
>> ngx_http_upstream_rr_peers_rlock(rp->rrp.peers);
>>
>> #if (NGX_HTTP_UPSTREAM_ZONE)
>> - if (rp->rrp.peers->shpool && rcf->ranges == NULL) {
>> + if (rp->rrp.peers->config
>> + && (rcf->ranges == NULL || rcf->config != *rp->rrp.peers->config))
>> + {
>> if (ngx_http_upstream_update_random(NULL, us) != NGX_OK) {
>> ngx_http_upstream_rr_peers_unlock(rp->rrp.peers);
>> return NGX_ERROR;
>> }
>> +
>> + rcf->config = *rp->rrp.peers->config;
>> }
>> #endif
>>
>> @@ -220,11 +232,18 @@ ngx_http_upstream_get_random_peer(ngx_pe
>>
>> ngx_http_upstream_rr_peers_rlock(peers);
>>
>> - if (rp->tries > 20 || peers->single) {
>> + if (rp->tries > 20 || peers->number < 2) {
>> ngx_http_upstream_rr_peers_unlock(peers);
>> return ngx_http_upstream_get_round_robin_peer(pc, rrp);
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> + return ngx_http_upstream_get_round_robin_peer(pc, rrp);
>> + }
>> +#endif
>> +
>> pc->cached = 0;
>> pc->connection = NULL;
>>
>> @@ -274,6 +293,7 @@ ngx_http_upstream_get_random_peer(ngx_pe
>> }
>>
>> rrp->current = peer;
>> + ngx_http_upstream_rr_peer_ref(peers, peer);
>>
>> if (now - peer->checked > peer->fail_timeout) {
>> peer->checked = now;
>> @@ -314,11 +334,18 @@ ngx_http_upstream_get_random2_peer(ngx_p
>>
>> ngx_http_upstream_rr_peers_wlock(peers);
>>
>> - if (rp->tries > 20 || peers->single) {
>> + if (rp->tries > 20 || peers->number < 2) {
>> ngx_http_upstream_rr_peers_unlock(peers);
>> return ngx_http_upstream_get_round_robin_peer(pc, rrp);
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> + return ngx_http_upstream_get_round_robin_peer(pc, rrp);
>> + }
>> +#endif
>> +
>> pc->cached = 0;
>> pc->connection = NULL;
>>
>> @@ -384,6 +411,7 @@ ngx_http_upstream_get_random2_peer(ngx_p
>> }
>>
>> rrp->current = peer;
>> + ngx_http_upstream_rr_peer_ref(peers, peer);
>>
>> if (now - peer->checked > peer->fail_timeout) {
>> peer->checked = now;
>> @@ -467,6 +495,7 @@ ngx_http_upstream_random(ngx_conf_t *cf,
>> uscf->peer.init_upstream = ngx_http_upstream_init_random;
>>
>> uscf->flags = NGX_HTTP_UPSTREAM_CREATE
>> + |NGX_HTTP_UPSTREAM_MODIFY
>> |NGX_HTTP_UPSTREAM_WEIGHT
>> |NGX_HTTP_UPSTREAM_MAX_CONNS
>> |NGX_HTTP_UPSTREAM_MAX_FAILS
>> 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
>> @@ -18,6 +18,10 @@ static ngx_http_upstream_rr_peers_t *ngx
>> ngx_slab_pool_t *shpool, ngx_http_upstream_srv_conf_t *uscf);
>> static ngx_http_upstream_rr_peer_t *ngx_http_upstream_zone_copy_peer(
>> ngx_http_upstream_rr_peers_t *peers, ngx_http_upstream_rr_peer_t *src);
>> +static void ngx_http_upstream_zone_set_single(
>> + ngx_http_upstream_srv_conf_t *uscf);
>> +static void ngx_http_upstream_zone_remove_peer_locked(
>> + ngx_http_upstream_rr_peers_t *peers, ngx_http_upstream_rr_peer_t *peer);
>>
>>
>> static ngx_command_t ngx_http_upstream_zone_commands[] = {
>> @@ -33,6 +37,11 @@ static ngx_command_t ngx_http_upstream_
>> };
>>
>>
>> +static ngx_int_t ngx_http_upstream_zone_init_worker(ngx_cycle_t *cycle);
>> +static void ngx_http_upstream_zone_resolve_timer(ngx_event_t *event);
>> +static void ngx_http_upstream_zone_resolve_handler(ngx_resolver_ctx_t *ctx);
>
> These declarations should be moved up to other functions declarations.
>
>> static ngx_http_module_t ngx_http_upstream_zone_module_ctx = {
>> NULL, /* preconfiguration */
>> NULL, /* postconfiguration */
>> @@ -55,7 +64,7 @@ ngx_module_t ngx_http_upstream_zone_mod
>> NGX_HTTP_MODULE, /* module type */
>> NULL, /* init master */
>> NULL, /* init module */
>> - NULL, /* init process */
>> + ngx_http_upstream_zone_init_worker, /* init process */
>> NULL, /* init thread */
>> NULL, /* exit thread */
>> NULL, /* exit process */
>> @@ -188,9 +197,15 @@ ngx_http_upstream_zone_copy_peers(ngx_sl
>> ngx_http_upstream_srv_conf_t *uscf)
>> {
>> ngx_str_t *name;
>> + ngx_uint_t *config;
>> ngx_http_upstream_rr_peer_t *peer, **peerp;
>> ngx_http_upstream_rr_peers_t *peers, *backup;
>>
>> + config = ngx_slab_calloc(shpool, sizeof(ngx_uint_t));
>> + if (config == NULL) {
>> + return NULL;
>> + }
>> +
>> peers = ngx_slab_alloc(shpool, sizeof(ngx_http_upstream_rr_peers_t));
>> if (peers == NULL) {
>> return NULL;
>> @@ -214,6 +229,7 @@ ngx_http_upstream_zone_copy_peers(ngx_sl
>> peers->name = name;
>>
>> peers->shpool = shpool;
>> + peers->config = config;
>>
>> for (peerp = &peers->peer; *peerp; peerp = &peer->next) {
>> /* pool is unlocked */
>> @@ -223,6 +239,17 @@ ngx_http_upstream_zone_copy_peers(ngx_sl
>> }
>>
>> *peerp = peer;
>> + peer->id = (*peers->config)++;
>> + }
>> +
>> + for (peerp = &peers->resolve; *peerp; peerp = &peer->next) {
>> + peer = ngx_http_upstream_zone_copy_peer(peers, *peerp);
>> + if (peer == NULL) {
>> + return NULL;
>> + }
>> +
>> + *peerp = peer;
>> + peer->id = (*peers->config)++;
>> }
>>
>> if (peers->next == NULL) {
>> @@ -239,6 +266,7 @@ ngx_http_upstream_zone_copy_peers(ngx_sl
>> backup->name = name;
>>
>> backup->shpool = shpool;
>> + backup->config = config;
>>
>> for (peerp = &backup->peer; *peerp; peerp = &peer->next) {
>> /* pool is unlocked */
>> @@ -248,6 +276,17 @@ ngx_http_upstream_zone_copy_peers(ngx_sl
>> }
>>
>> *peerp = peer;
>> + peer->id = (*backup->config)++;
>> + }
>> +
>> + for (peerp = &backup->resolve; *peerp; peerp = &peer->next) {
>> + peer = ngx_http_upstream_zone_copy_peer(backup, *peerp);
>> + if (peer == NULL) {
>> + return NULL;
>> + }
>> +
>> + *peerp = peer;
>> + peer->id = (*backup->config)++;
>> }
>>
>> peers->next = backup;
>> @@ -279,6 +318,7 @@ ngx_http_upstream_zone_copy_peer(ngx_htt
>> dst->sockaddr = NULL;
>> dst->name.data = NULL;
>> dst->server.data = NULL;
>> + dst->host = NULL;
>> }
>>
>> dst->sockaddr = ngx_slab_calloc_locked(pool, sizeof(ngx_sockaddr_t));
>> @@ -301,12 +341,37 @@ ngx_http_upstream_zone_copy_peer(ngx_htt
>> }
>>
>> ngx_memcpy(dst->server.data, src->server.data, src->server.len);
>> +
>> + if (src->host) {
>> + dst->host = ngx_slab_calloc_locked(pool,
>> + sizeof(ngx_http_upstream_host_t));
>> + if (dst->host == NULL) {
>> + goto failed;
>> + }
>> +
>> + dst->host->name.data = ngx_slab_alloc_locked(pool,
>> + src->host->name.len);
>> + if (dst->host->name.data == NULL) {
>> + goto failed;
>> + }
>> +
>> + dst->host->peers = peers;
>> + dst->host->peer = dst;
>> +
>> + dst->host->name.len = src->host->name.len;
>> + ngx_memcpy(dst->host->name.data, src->host->name.data,
>> + src->host->name.len);
>> + }
>> }
>>
>> return dst;
>>
>> failed:
>>
>> + if (dst->host) {
>> + ngx_slab_free_locked(pool, dst->host);
>> + }
>> +
>> if (dst->server.data) {
>> ngx_slab_free_locked(pool, dst->server.data);
>> }
>> @@ -323,3 +388,337 @@ failed:
>>
>> return NULL;
>> }
>> +
>> +
>> +static void
>> +ngx_http_upstream_zone_set_single(ngx_http_upstream_srv_conf_t *uscf)
>> +{
>> + ngx_http_upstream_rr_peers_t *peers;
>> +
>> + peers = uscf->peer.data;
>> +
>> + if (peers->number == 1
>> + && (peers->next == NULL || peers->next->number == 0))
>> + {
>> + peers->single = 1;
>> +
>> + } else {
>> + peers->single = 0;
>> + }
>> +}
>> +
>> +
>> +static void
>> +ngx_http_upstream_zone_remove_peer_locked(ngx_http_upstream_rr_peers_t *peers,
>> + ngx_http_upstream_rr_peer_t *peer)
>> +{
>> + peers->total_weight -= peer->weight;
>> + peers->number--;
>> + peers->tries -= (peer->down == 0);
>> + (*peers->config)++;
>> + peers->weighted = (peers->total_weight != peers->number);
>> +
>> + ngx_http_upstream_rr_peer_free(peers, peer);
>> +}
>> +
>> +
>> +static ngx_int_t
>> +ngx_http_upstream_zone_init_worker(ngx_cycle_t *cycle)
>> +{
>> + ngx_uint_t i;
>> + ngx_event_t *event;
>> + ngx_http_upstream_rr_peer_t *peer;
>> + ngx_http_upstream_rr_peers_t *peers;
>> + ngx_http_upstream_srv_conf_t *uscf, **uscfp;
>> + ngx_http_upstream_main_conf_t *umcf;
>> +
>> + if (ngx_process != NGX_PROCESS_WORKER
>> + && ngx_process != NGX_PROCESS_SINGLE)
>> + {
>> + return NGX_OK;
>> + }
>> +
>> + umcf = ngx_http_cycle_get_module_main_conf(cycle, ngx_http_upstream_module);
>> +
>> + if (umcf == NULL) {
>> + return NGX_OK;
>> + }
>> +
>> + uscfp = umcf->upstreams.elts;
>> +
>> + for (i = 0; i < umcf->upstreams.nelts; i++) {
>> +
>> + uscf = uscfp[i];
>> +
>> + if (uscf->shm_zone == NULL) {
>> + continue;
>> + }
>> +
>> + peers = uscf->peer.data;
>> +
>> + do {
>> + ngx_http_upstream_rr_peers_wlock(peers);
>> +
>> + for (peer = peers->resolve; peer; peer = peer->next) {
>> +
>> + if (peer->host->worker != ngx_worker) {
>> + continue;
>> + }
>> +
>> + event = &peer->host->event;
>> + ngx_memzero(event, sizeof(ngx_event_t));
>> +
>> + event->data = uscf;
>> + event->handler = ngx_http_upstream_zone_resolve_timer;
>> + event->log = cycle->log;
>> + event->cancelable = 1;
>> +
>> + ngx_http_upstream_rr_peer_ref(peers, peer);
>
> In open source nginx a template cannot be deleted since there's no API.
> As a result, there's no reason in increase the reference counter here.
>
>> + ngx_add_timer(event, 1);
>> + }
>> +
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> +
>> + peers = peers->next;
>> +
>> + } while (peers);
>> + }
>> +
>> + return NGX_OK;
>> +}
>> +
>> +
>> +static void
>> +ngx_http_upstream_zone_resolve_timer(ngx_event_t *event)
>> +{
>> + ngx_resolver_ctx_t *ctx;
>> + ngx_http_upstream_host_t *host;
>> + ngx_http_upstream_rr_peer_t *template;
>> + ngx_http_upstream_rr_peers_t *peers;
>> + ngx_http_upstream_srv_conf_t *uscf;
>> +
>> + host = (ngx_http_upstream_host_t *) event;
>> + uscf = event->data;
>> + peers = host->peers;
>> + template = host->peer;
>> +
>> + if (template->zombie) {
>> + (void) ngx_http_upstream_rr_peer_unref(peers, template);
>> +
>> + ngx_shmtx_lock(&peers->shpool->mutex);
>> +
>> + if (host->service.len) {
>> + ngx_slab_free_locked(peers->shpool, host->service.data);
>> + }
>> +
>> + ngx_slab_free_locked(peers->shpool, host->name.data);
>> + ngx_slab_free_locked(peers->shpool, host);
>> + ngx_shmtx_unlock(&peers->shpool->mutex);
>> +
>> + return;
>> + }
>
> Since a template cannot be deleted, it cannot become a zombie as well.
> This block is useless.
>
>> + ctx = ngx_resolve_start(uscf->resolver, NULL);
>> + if (ctx == NULL) {
>> + goto retry;
>> + }
>> +
>> + if (ctx == NGX_NO_RESOLVER) {
>> + ngx_log_error(NGX_LOG_ERR, event->log, 0,
>> + "no resolver defined to resolve %V", &host->name);
>> + return;
>> + }
>> +
>> + ctx->name = host->name;
>> + ctx->handler = ngx_http_upstream_zone_resolve_handler;
>> + ctx->data = host;
>> + ctx->timeout = uscf->resolver_timeout;
>> + ctx->cancelable = 1;
>> +
>> + if (ngx_resolve_name(ctx) == NGX_OK) {
>> + return;
>> + }
>> +
>> +retry:
>> +
>> + ngx_add_timer(event, ngx_max(uscf->resolver_timeout, 1000));
>> +}
>> +
>> +
>> +static void
>> +ngx_http_upstream_zone_resolve_handler(ngx_resolver_ctx_t *ctx)
>> +{
>> + time_t now;
>> + in_port_t port;
>> + ngx_msec_t timer;
>> + ngx_uint_t i, j;
>> + ngx_event_t *event;
>> + ngx_resolver_addr_t *addr;
>> + ngx_http_upstream_host_t *host;
>> + ngx_http_upstream_rr_peer_t *peer, *template, **peerp;
>> + ngx_http_upstream_rr_peers_t *peers;
>> + ngx_http_upstream_srv_conf_t *uscf;
>> +
>> + host = ctx->data;
>> + event = &host->event;
>> + uscf = event->data;
>> + peers = host->peers;
>> + template = host->peer;
>> +
>> + ngx_http_upstream_rr_peers_wlock(peers);
>> +
>> + if (template->zombie) {
>> + (void) ngx_http_upstream_rr_peer_unref(peers, template);
>> +
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> +
>> + ngx_shmtx_lock(&peers->shpool->mutex);
>> + ngx_slab_free_locked(peers->shpool, host->name.data);
>> + ngx_slab_free_locked(peers->shpool, host);
>> + ngx_shmtx_unlock(&peers->shpool->mutex);
>> +
>> + ngx_resolve_name_done(ctx);
>> +
>> + return;
>> + }
>
> Again, this block is useless.
>
>> + now = ngx_time();
>> +
>> + if (ctx->state) {
>> + ngx_log_error(NGX_LOG_ERR, event->log, 0,
>> + "%V could not be resolved (%i: %s)",
>> + &ctx->name, ctx->state,
>> + ngx_resolver_strerror(ctx->state));
>> +
>> + if (ctx->state != NGX_RESOLVE_NXDOMAIN) {
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> +
>> + ngx_resolve_name_done(ctx);
>> +
>> + ngx_add_timer(event, ngx_max(uscf->resolver_timeout, 1000));
>> + return;
>> + }
>> +
>> + /* NGX_RESOLVE_NXDOMAIN */
>> +
>> + ctx->naddrs = 0;
>> + }
>> +
>> +#if (NGX_DEBUG)
>> + {
>> + u_char text[NGX_SOCKADDR_STRLEN];
>> + size_t len;
>> +
>> + for (i = 0; i < ctx->naddrs; i++) {
>> + len = ngx_sock_ntop(ctx->addrs[i].sockaddr, ctx->addrs[i].socklen,
>> + text, NGX_SOCKADDR_STRLEN, 0);
>> +
>> + ngx_log_debug3(NGX_LOG_DEBUG_HTTP, event->log, 0,
>> + "name %V was resolved to %*s", &host->name, len, text);
>> + }
>> + }
>> +#endif
>> +
>> + for (peerp = &peers->peer; *peerp; /* void */ ) {
>> + peer = *peerp;
>> +
>> + if (peer->host != host) {
>> + goto next;
>> + }
>> +
>> + for (j = 0; j < ctx->naddrs; j++) {
>> +
>> + addr = &ctx->addrs[j];
>> +
>> + if (addr->name.len == 0
>> + && ngx_cmp_sockaddr(peer->sockaddr, peer->socklen,
>> + addr->sockaddr, addr->socklen, 0)
>> + == NGX_OK)
>> + {
>> + addr->name.len = 1;
>> + goto next;
>> + }
>> + }
>> +
>> + *peerp = peer->next;
>> + ngx_http_upstream_zone_remove_peer_locked(peers, peer);
>> +
>> + ngx_http_upstream_zone_set_single(uscf);
>> +
>> + continue;
>> +
>> + next:
>> +
>> + peerp = &peer->next;
>> + }
>> +
>> + for (i = 0; i < ctx->naddrs; i++) {
>> +
>> + addr = &ctx->addrs[i];
>> +
>> + if (addr->name.len == 1) {
>> + addr->name.len = 0;
>> + continue;
>> + }
>> +
>> + ngx_shmtx_lock(&peers->shpool->mutex);
>> + peer = ngx_http_upstream_zone_copy_peer(peers, NULL);
>> + ngx_shmtx_unlock(&peers->shpool->mutex);
>> +
>> + if (peer == NULL) {
>> + ngx_log_error(NGX_LOG_ERR, event->log, 0,
>> + "cannot add new server to upstream \"%V\", "
>> + "memory exhausted", peers->name);
>> + break;
>> + }
>> +
>> + ngx_memcpy(peer->sockaddr, addr->sockaddr, addr->socklen);
>> +
>> + port = ((struct sockaddr_in *) template->sockaddr)->sin_port;
>> +
>> + switch (peer->sockaddr->sa_family) {
>> +#if (NGX_HAVE_INET6)
>> + case AF_INET6:
>> + ((struct sockaddr_in6 *) peer->sockaddr)->sin6_port = port;
>> + break;
>> +#endif
>> + default: /* AF_INET */
>> + ((struct sockaddr_in *) peer->sockaddr)->sin_port = port;
>> + }
>> +
>> + peer->socklen = addr->socklen;
>> +
>> + peer->name.len = ngx_sock_ntop(peer->sockaddr, peer->socklen,
>> + peer->name.data, NGX_SOCKADDR_STRLEN, 1);
>> +
>> + peer->host = template->host;
>> + peer->server = template->server;
>> +
>> + peer->weight = template->weight;
>> + peer->effective_weight = peer->weight;
>> + peer->max_conns = template->max_conns;
>> + peer->max_fails = template->max_fails;
>> + peer->fail_timeout = template->fail_timeout;
>> + peer->down = template->down;
>> +
>> + *peerp = peer;
>> + peerp = &peer->next;
>> +
>> + peers->number++;
>> + peers->tries += (peer->down == 0);
>> + peers->total_weight += peer->weight;
>> + peers->weighted = (peers->total_weight != peers->number);
>> + peer->id = (*peers->config)++;
>> +
>> + ngx_http_upstream_zone_set_single(uscf);
>> + }
>> +
>> + ngx_http_upstream_rr_peers_unlock(peers);
>> +
>> + timer = (ngx_msec_t) 1000 * (ctx->valid > now ? ctx->valid - now + 1 : 1);
>> + timer = ngx_min(timer, uscf->resolver_timeout);
>
> The last line was added to facilitate faster recycle of zombie templates.
> Since there are no zombie templates here, the line can be removed.
>
>> + ngx_resolve_name_done(ctx);
>> +
>> + ngx_add_timer(event, timer);
>> +}
>> 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
>> @@ -1565,6 +1565,26 @@ ngx_http_upstream_connect(ngx_http_reque
>>
>> u->state->peer = u->peer.name;
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (u->upstream && u->upstream->shm_zone
>> + && (u->upstream->flags & NGX_HTTP_UPSTREAM_MODIFY)
>> + ) {
>
> Style: ')' should be move to the line above.
>
>> + u->state->peer = ngx_palloc(r->pool,
>> + sizeof(ngx_str_t) + u->peer.name->len);
>> + if (u->state->peer == NULL) {
>> + ngx_http_upstream_finalize_request(r, u,
>> + NGX_HTTP_INTERNAL_SERVER_ERROR);
>> + return;
>> + }
>> +
>> + u->state->peer->len = u->peer.name->len;
>> + u->state->peer->data = (u_char *) (u->state->peer + 1);
>> + ngx_memcpy(u->state->peer->data, u->peer.name->data, u->peer.name->len);
>> +
>> + u->peer.name = u->state->peer;
>> + }
>> +#endif
>> +
>> if (rc == NGX_BUSY) {
>> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "no live upstreams");
>> ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_NOLIVE);
>> @@ -6066,6 +6086,7 @@ ngx_http_upstream(ngx_conf_t *cf, ngx_co
>> u.no_port = 1;
>>
>> uscf = ngx_http_upstream_add(cf, &u, NGX_HTTP_UPSTREAM_CREATE
>> + |NGX_HTTP_UPSTREAM_MODIFY
>> |NGX_HTTP_UPSTREAM_WEIGHT
>> |NGX_HTTP_UPSTREAM_MAX_CONNS
>> |NGX_HTTP_UPSTREAM_MAX_FAILS
>> @@ -6151,7 +6172,11 @@ ngx_http_upstream(ngx_conf_t *cf, ngx_co
>> return rv;
>> }
>>
>> - if (uscf->servers->nelts == 0) {
>> + if (uscf->servers->nelts == 0
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + && uscf->shm_zone == NULL
>> +#endif
>
> In open source nginx empty upstreams are not allowed, irrespective of the zone.
> No new servers can appear in the upstream during runtime since there's no API.
>
>> + ) {
>> ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
>> "no servers are inside upstream");
>> return NGX_CONF_ERROR;
>> @@ -6171,6 +6196,9 @@ ngx_http_upstream_server(ngx_conf_t *cf,
>> ngx_url_t u;
>> ngx_int_t weight, max_conns, max_fails;
>> ngx_uint_t i;
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_uint_t resolve;
>> +#endif
>> ngx_http_upstream_server_t *us;
>>
>> us = ngx_array_push(uscf->servers);
>> @@ -6186,6 +6214,9 @@ ngx_http_upstream_server(ngx_conf_t *cf,
>> max_conns = 0;
>> max_fails = 1;
>> fail_timeout = 10;
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + resolve = 0;
>> +#endif
>>
>> for (i = 2; i < cf->args->nelts; i++) {
>>
>> @@ -6274,6 +6305,13 @@ ngx_http_upstream_server(ngx_conf_t *cf,
>> continue;
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (ngx_strcmp(value[i].data, "resolve") == 0) {
>> + resolve = 1;
>> + continue;
>> + }
>> +#endif
>> +
>> goto invalid;
>> }
>>
>> @@ -6282,6 +6320,13 @@ ngx_http_upstream_server(ngx_conf_t *cf,
>> u.url = value[1];
>> u.default_port = 80;
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (resolve) {
>> + /* resolve at run time */
>> + u.no_resolve = 1;
>> + }
>> +#endif
>> +
>> if (ngx_parse_url(cf->pool, &u) != NGX_OK) {
>> if (u.err) {
>> ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
>> @@ -6292,8 +6337,45 @@ ngx_http_upstream_server(ngx_conf_t *cf,
>> }
>>
>> us->name = u.url;
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> +
>> + if (resolve && u.naddrs == 0) {
>> + ngx_addr_t *addr;
>> +
>> + /* save port */
>> +
>> + addr = ngx_pcalloc(cf->pool, sizeof(ngx_addr_t));
>> + if (addr == NULL) {
>> + return NGX_CONF_ERROR;
>> + }
>> +
>> + addr->sockaddr = ngx_palloc(cf->pool, u.socklen);
>> + if (addr->sockaddr == NULL) {
>> + return NGX_CONF_ERROR;
>> + }
>> +
>> + ngx_memcpy(addr->sockaddr, &u.sockaddr, u.socklen);
>> +
>> + addr->socklen = u.socklen;
>> +
>> + us->addrs = addr;
>> + us->naddrs = 1;
>> +
>> + us->host = u.host;
>> +
>> + } else {
>> + us->addrs = u.addrs;
>> + us->naddrs = u.naddrs;
>> + }
>> +
>> +#else
>> +
>> us->addrs = u.addrs;
>> us->naddrs = u.naddrs;
>> +
>> +#endif
>> +
>> us->weight = weight;
>> us->max_conns = max_conns;
>> us->max_fails = max_fails;
>> 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
>> @@ -104,7 +104,11 @@ typedef struct {
>>
>> unsigned backup:1;
>>
>> - NGX_COMPAT_BEGIN(6)
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_str_t host;
>> +#endif
>> +
>> + NGX_COMPAT_BEGIN(4)
>> NGX_COMPAT_END
>> } ngx_http_upstream_server_t;
>>
>> @@ -115,6 +119,7 @@ typedef struct {
>> #define NGX_HTTP_UPSTREAM_FAIL_TIMEOUT 0x0008
>> #define NGX_HTTP_UPSTREAM_DOWN 0x0010
>> #define NGX_HTTP_UPSTREAM_BACKUP 0x0020
>> +#define NGX_HTTP_UPSTREAM_MODIFY 0x0040
>> #define NGX_HTTP_UPSTREAM_MAX_CONNS 0x0100
>>
>>
>> @@ -133,6 +138,8 @@ struct ngx_http_upstream_srv_conf_s {
>>
>> #if (NGX_HTTP_UPSTREAM_ZONE)
>> ngx_shm_zone_t *shm_zone;
>> + ngx_resolver_t *resolver;
>> + ngx_msec_t resolver_timeout;
>> #endif
>> };
>>
>> diff --git a/src/http/ngx_http_upstream_round_robin.c b/src/http/ngx_http_upstream_round_robin.c
>> --- a/src/http/ngx_http_upstream_round_robin.c
>> +++ b/src/http/ngx_http_upstream_round_robin.c
>> @@ -32,10 +32,15 @@ ngx_http_upstream_init_round_robin(ngx_c
>> ngx_http_upstream_srv_conf_t *us)
>> {
>> ngx_url_t u;
>> - ngx_uint_t i, j, n, w, t;
>> + ngx_uint_t i, j, n, r, w, t;
>> ngx_http_upstream_server_t *server;
>> ngx_http_upstream_rr_peer_t *peer, **peerp;
>> ngx_http_upstream_rr_peers_t *peers, *backup;
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_uint_t resolve;
>> + ngx_http_core_loc_conf_t *clcf;
>> + ngx_http_upstream_rr_peer_t **rpeerp;
>> +#endif
>>
>> us->peer.init = ngx_http_upstream_init_round_robin_peer;
>>
>> @@ -43,23 +48,99 @@ ngx_http_upstream_init_round_robin(ngx_c
>> server = us->servers->elts;
>>
>> n = 0;
>> + r = 0;
>> w = 0;
>> t = 0;
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + resolve = 0;
>> +#endif
>> +
>> for (i = 0; i < us->servers->nelts; i++) {
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (server[i].host.len) {
>> + resolve = 1;
>> + }
>> +#endif
>> +
>> if (server[i].backup) {
>> continue;
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (server[i].host.len) {
>> + r++;
>> +
>> + } else {
>> + n += server[i].naddrs;
>> + w += server[i].naddrs * server[i].weight;
>> +
>> + if (!server[i].down) {
>> + t += server[i].naddrs;
>> + }
>> + }
>> +#else
>
> The code above and below is the same code. The reason behind duplication was
> to simplify the diff. Now duplication makes no sense. Instead, the following
> can be done:
>
> #if (NGX_HTTP_UPSTREAM_ZONE)
> if (server[i].host.len) {
> r++;
> continue;
> }
> #endif
>
>> n += server[i].naddrs;
>> w += server[i].naddrs * server[i].weight;
>>
>> if (!server[i].down) {
>> t += server[i].naddrs;
>> }
>> +#endif
>> }
>>
>> - if (n == 0) {
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (us->shm_zone) {
>> +
>> + if (resolve && !(us->flags & NGX_HTTP_UPSTREAM_MODIFY)) {
>> + ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
>> + "load balancing method does not support"
>> + " resolving names at run time in"
>> + " upstream \"%V\" in %s:%ui",
>> + &us->host, us->file_name, us->line);
>> + return NGX_ERROR;
>> + }
>> +
>> + clcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_core_module);
>> +
>> + us->resolver = clcf->resolver;
>> + us->resolver_timeout = clcf->resolver_timeout;
>> +
>> + /*
>> + * Without "resolver_timeout" in http{}, the value is unset.
>> + * Even if we set it in ngx_http_core_merge_loc_conf(), it's
>> + * still dependent on the module order and unreliable.
>> + */
>> + ngx_conf_init_msec_value(us->resolver_timeout, 30000);
>> +
>> + if (resolve
>> + && (us->resolver == NULL
>> + || us->resolver->connections.nelts == 0))
>> + {
>> + ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
>> + "no resolver defined to resolve names"
>> + " at run time in upstream \"%V\" in %s:%ui",
>> + &us->host, us->file_name, us->line);
>> + return NGX_ERROR;
>> + }
>> +
>> + } else if (resolve) {
>> +
>> + ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
>> + "resolving names at run time requires"
>> + " upstream \"%V\" in %s:%ui"
>> + " to be in shared memory",
>> + &us->host, us->file_name, us->line);
>> + return NGX_ERROR;
>> + }
>> +#endif
>> +
>> + if (n == 0
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + && us->shm_zone == NULL
>> +#endif
>
> An empty zone will always be empty in open source nginx. This should be checked
> instead:
>
> if (n + r == 0) { ... }
>
>> + ) {
>> ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
>> "no servers in upstream \"%V\" in %s:%ui",
>> &us->host, us->file_name, us->line);
>> @@ -71,7 +152,8 @@ ngx_http_upstream_init_round_robin(ngx_c
>> return NGX_ERROR;
>> }
>>
>> - peer = ngx_pcalloc(cf->pool, sizeof(ngx_http_upstream_rr_peer_t) * n);
>> + peer = ngx_pcalloc(cf->pool, sizeof(ngx_http_upstream_rr_peer_t)
>> + * (n + r));
>> if (peer == NULL) {
>> return NGX_ERROR;
>> }
>> @@ -86,11 +168,46 @@ ngx_http_upstream_init_round_robin(ngx_c
>> n = 0;
>> peerp = &peers->peer;
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + rpeerp = &peers->resolve;
>> +#endif
>> +
>> for (i = 0; i < us->servers->nelts; i++) {
>> if (server[i].backup) {
>> continue;
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (server[i].host.len) {
>> +
>> + peer[n].host = ngx_pcalloc(cf->pool,
>> + sizeof(ngx_http_upstream_host_t));
>> + if (peer[n].host == NULL) {
>> + return NGX_ERROR;
>> + }
>> +
>> + peer[n].host->name = server[i].host;
>> +
>> + peer[n].sockaddr = server[i].addrs[0].sockaddr;
>> + peer[n].socklen = server[i].addrs[0].socklen;
>> + peer[n].name = server[i].addrs[0].name;
>> + peer[n].weight = server[i].weight;
>> + peer[n].effective_weight = server[i].weight;
>> + peer[n].current_weight = 0;
>> + peer[n].max_conns = server[i].max_conns;
>> + peer[n].max_fails = server[i].max_fails;
>> + peer[n].fail_timeout = server[i].fail_timeout;
>> + peer[n].down = server[i].down;
>> + peer[n].server = server[i].name;
>> +
>> + *rpeerp = &peer[n];
>> + rpeerp = &peer[n].next;
>> + n++;
>> +
>> + continue;
>> + }
>> +#endif
>> +
>> for (j = 0; j < server[i].naddrs; j++) {
>> peer[n].sockaddr = server[i].addrs[j].sockaddr;
>> peer[n].socklen = server[i].addrs[j].socklen;
>> @@ -115,6 +232,7 @@ ngx_http_upstream_init_round_robin(ngx_c
>> /* backup servers */
>>
>> n = 0;
>> + r = 0;
>> w = 0;
>> t = 0;
>>
>> @@ -123,15 +241,37 @@ ngx_http_upstream_init_round_robin(ngx_c
>> continue;
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (server[i].host.len) {
>> + r++;
>> +
>> + } else {
>> + n += server[i].naddrs;
>> + w += server[i].naddrs * server[i].weight;
>> +
>> + if (!server[i].down) {
>> + t += server[i].naddrs;
>> + }
>> + }
>> +#else
>
> See above.
>
>> n += server[i].naddrs;
>> w += server[i].naddrs * server[i].weight;
>>
>> if (!server[i].down) {
>> t += server[i].naddrs;
>> }
>> +#endif
>> }
>>
>> - if (n == 0) {
>> + if (n == 0
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + && us->shm_zone == NULL
>> +#endif
>> + ) {
>> + return NGX_OK;
>> + }
>
> See above.
>
> if (n + r == 0) { .. }
>
>> +
>> + if (n + r == 0 && !(us->flags & NGX_HTTP_UPSTREAM_BACKUP)) {
>> return NGX_OK;
>> }
>
> After the change above this block will be useless.
>
Actually, this code should be preserved as is (or moved to the
subsequent patches).
The way we handle SRV peer weights, we may place some of the peers to
the backup list. That requires always initializing an empty backup list
if the upstream has resolvable servers and the lb method supports backup
servers.
The condition could be optimized for the opensource code though:
if (n + r == 0
#if (NGX_HTTP_UPSTREAM_ZONE)
&& (!resolve || !(us->flags & NGX_HTTP_UPSTREAM_BACKUP))
#endif
) {
return NGX_OK;
}
WDYT?
>> @@ -140,12 +280,16 @@ ngx_http_upstream_init_round_robin(ngx_c
>> return NGX_ERROR;
>> }
>>
>> - peer = ngx_pcalloc(cf->pool, sizeof(ngx_http_upstream_rr_peer_t) * n);
>> + peer = ngx_pcalloc(cf->pool, sizeof(ngx_http_upstream_rr_peer_t)
>> + * (n + r));
>> if (peer == NULL) {
>> return NGX_ERROR;
>> }
>>
>> - peers->single = 0;
>> + if (n > 0) {
>> + peers->single = 0;
>> + }
>> +
>> backup->single = 0;
>> backup->number = n;
>> backup->weighted = (w != n);
>> @@ -156,11 +300,46 @@ ngx_http_upstream_init_round_robin(ngx_c
>> n = 0;
>> peerp = &backup->peer;
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + rpeerp = &backup->resolve;
>> +#endif
>> +
>> for (i = 0; i < us->servers->nelts; i++) {
>> if (!server[i].backup) {
>> continue;
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (server[i].host.len) {
>> +
>> + peer[n].host = ngx_pcalloc(cf->pool,
>> + sizeof(ngx_http_upstream_host_t));
>> + if (peer[n].host == NULL) {
>> + return NGX_ERROR;
>> + }
>> +
>> + peer[n].host->name = server[i].host;
>> +
>> + peer[n].sockaddr = server[i].addrs[0].sockaddr;
>> + peer[n].socklen = server[i].addrs[0].socklen;
>> + peer[n].name = server[i].addrs[0].name;
>> + peer[n].weight = server[i].weight;
>> + peer[n].effective_weight = server[i].weight;
>> + peer[n].current_weight = 0;
>> + peer[n].max_conns = server[i].max_conns;
>> + peer[n].max_fails = server[i].max_fails;
>> + peer[n].fail_timeout = server[i].fail_timeout;
>> + peer[n].down = server[i].down;
>> + peer[n].server = server[i].name;
>> +
>> + *rpeerp = &peer[n];
>> + rpeerp = &peer[n].next;
>> + n++;
>> +
>> + continue;
>> + }
>> +#endif
>> +
>> for (j = 0; j < server[i].naddrs; j++) {
>> peer[n].sockaddr = server[i].addrs[j].sockaddr;
>> peer[n].socklen = server[i].addrs[j].socklen;
>> @@ -273,7 +452,12 @@ ngx_http_upstream_init_round_robin_peer(
>>
>> rrp->peers = us->peer.data;
>> rrp->current = NULL;
>> - rrp->config = 0;
>> +
>> + ngx_http_upstream_rr_peers_rlock(rrp->peers);
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + rrp->config = rrp->peers->config ? *rrp->peers->config : 0;
>> +#endif
>>
>> n = rrp->peers->number;
>>
>> @@ -281,6 +465,10 @@ ngx_http_upstream_init_round_robin_peer(
>> n = rrp->peers->next->number;
>> }
>>
>> + r->upstream->peer.tries = ngx_http_upstream_tries(rrp->peers);
>> +
>> + ngx_http_upstream_rr_peers_unlock(rrp->peers);
>> +
>> if (n <= 8 * sizeof(uintptr_t)) {
>> rrp->tried = &rrp->data;
>> rrp->data = 0;
>> @@ -296,7 +484,6 @@ ngx_http_upstream_init_round_robin_peer(
>>
>> r->upstream->peer.get = ngx_http_upstream_get_round_robin_peer;
>> r->upstream->peer.free = ngx_http_upstream_free_round_robin_peer;
>> - r->upstream->peer.tries = ngx_http_upstream_tries(rrp->peers);
>> #if (NGX_HTTP_SSL)
>> r->upstream->peer.set_session =
>> ngx_http_upstream_set_round_robin_peer_session;
>> @@ -446,6 +633,12 @@ ngx_http_upstream_get_round_robin_peer(n
>> peers = rrp->peers;
>> ngx_http_upstream_rr_peers_wlock(peers);
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + goto busy;
>> + }
>> +#endif
>> +
>> if (peers->single) {
>> peer = peers->peer;
>>
>> @@ -458,6 +651,7 @@ ngx_http_upstream_get_round_robin_peer(n
>> }
>>
>> rrp->current = peer;
>> + ngx_http_upstream_rr_peer_ref(peers, peer);
>>
>> } else {
>>
>> @@ -508,8 +702,18 @@ failed:
>> }
>>
>> ngx_http_upstream_rr_peers_wlock(peers);
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + if (peers->config && rrp->config != *peers->config) {
>> + goto busy;
>> + }
>> +#endif
>
> This block is useless.
>
>> }
>>
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> +busy:
>> +#endif
>> +
>> ngx_http_upstream_rr_peers_unlock(peers);
>>
>> pc->name = peers->name;
>> @@ -580,6 +784,7 @@ ngx_http_upstream_get_peer(ngx_http_upst
>> }
>>
>> rrp->current = best;
>> + ngx_http_upstream_rr_peer_ref(rrp->peers, best);
>>
>> n = p / (8 * sizeof(uintptr_t));
>> m = (uintptr_t) 1 << p % (8 * sizeof(uintptr_t));
>> @@ -617,9 +822,16 @@ ngx_http_upstream_free_round_robin_peer(
>>
>> if (rrp->peers->single) {
>>
>> + if (peer->fails) {
>> + peer->fails = 0;
>> + }
>> +
>> peer->conns--;
>>
>> - ngx_http_upstream_rr_peer_unlock(rrp->peers, peer);
>> + if (ngx_http_upstream_rr_peer_unref(rrp->peers, peer) == NGX_OK) {
>> + ngx_http_upstream_rr_peer_unlock(rrp->peers, peer);
>> + }
>> +
>> ngx_http_upstream_rr_peers_unlock(rrp->peers);
>>
>> pc->tries = 0;
>> @@ -661,7 +873,10 @@ ngx_http_upstream_free_round_robin_peer(
>>
>> peer->conns--;
>>
>> - ngx_http_upstream_rr_peer_unlock(rrp->peers, peer);
>> + if (ngx_http_upstream_rr_peer_unref(rrp->peers, peer) == NGX_OK) {
>> + ngx_http_upstream_rr_peer_unlock(rrp->peers, peer);
>> + }
>> +
>> ngx_http_upstream_rr_peers_unlock(rrp->peers);
>>
>> if (pc->tries) {
>> 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
>> @@ -14,8 +14,23 @@
>> #include <ngx_http.h>
>>
>>
>> +typedef struct ngx_http_upstream_rr_peers_s ngx_http_upstream_rr_peers_t;
>> typedef struct ngx_http_upstream_rr_peer_s ngx_http_upstream_rr_peer_t;
>>
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> +
>> +typedef struct {
>> + ngx_event_t event; /* must be first */
>> + ngx_uint_t worker;
>> + ngx_str_t name;
>> + ngx_http_upstream_rr_peers_t *peers;
>> + ngx_http_upstream_rr_peer_t *peer;
>> +} ngx_http_upstream_host_t;
>> +
>> +#endif
>> +
>> +
>> struct ngx_http_upstream_rr_peer_s {
>> struct sockaddr *sockaddr;
>> socklen_t socklen;
>> @@ -46,7 +61,12 @@ struct ngx_http_upstream_rr_peer_s {
>> #endif
>>
>> #if (NGX_HTTP_UPSTREAM_ZONE)
>> + unsigned zombie:1;
>
> I suggest declaring this as in other similar places:
>
> ngx_uint_t zombie; /* unsigned zombie:1; */
>
We already use bitfields in the same file though.
Also, in the most popular configuration (64-bit Unix-like platform, GCC
or Clang), the suggested change would increase the struct size by 8
bytes. Currently the flag is located in a 4 byte hole left between int
and ngx_atomic_t, so we get it for free.
>> +
>> ngx_atomic_t lock;
>> + ngx_uint_t id;
>
> This field is not used in open source nginx and should not be added or assigned.
>
>> + ngx_uint_t refs;
>> + ngx_http_upstream_host_t *host;
>> #endif
>>
>> ngx_http_upstream_rr_peer_t *next;
>> @@ -56,8 +76,6 @@ struct ngx_http_upstream_rr_peer_s {
>> };
>>
>>
>> -typedef struct ngx_http_upstream_rr_peers_s ngx_http_upstream_rr_peers_t;
>> -
>> struct ngx_http_upstream_rr_peers_s {
>> ngx_uint_t number;
>>
>> @@ -78,6 +96,12 @@ struct ngx_http_upstream_rr_peers_s {
>> ngx_http_upstream_rr_peers_t *next;
>>
>> ngx_http_upstream_rr_peer_t *peer;
>> +
>> +#if (NGX_HTTP_UPSTREAM_ZONE)
>> + ngx_uint_t *config;
>> + ngx_http_upstream_rr_peer_t *resolve;
>> + ngx_uint_t zombies;
>
> This field is unused in open source nginx and should not be added or assigned.
>
>> +#endif
>> };
>>
>>
>> @@ -114,6 +138,67 @@ struct ngx_http_upstream_rr_peers_s {
>> ngx_rwlock_unlock(&peer->lock); \
>> }
>>
>> +
>> +#define ngx_http_upstream_rr_peer_ref(peers, peer) \
>> + (peer)->refs++;
>> +
>> +
>> +static ngx_inline void
>> +ngx_http_upstream_rr_peer_free_locked(ngx_http_upstream_rr_peers_t *peers,
>> + ngx_http_upstream_rr_peer_t *peer)
>> +{
>> + if (peer->refs) {
>> + peer->zombie = 1;
>> + peers->zombies++;
>> + return;
>> + }
>> +
>> + ngx_slab_free_locked(peers->shpool, peer->sockaddr);
>> + ngx_slab_free_locked(peers->shpool, peer->name.data);
>> +
>> + if (peer->server.data && (peer->host == NULL || peer->host->peer == peer)) {
>> + ngx_slab_free_locked(peers->shpool, peer->server.data);
>> + }
>> +
>> +#if (NGX_HTTP_SSL)
>> + if (peer->ssl_session) {
>> + ngx_slab_free_locked(peers->shpool, peer->ssl_session);
>> + }
>> +#endif
>> +
>> + ngx_slab_free_locked(peers->shpool, peer);
>> +}
>> +
>> +
>> +static ngx_inline void
>> +ngx_http_upstream_rr_peer_free(ngx_http_upstream_rr_peers_t *peers,
>> + ngx_http_upstream_rr_peer_t *peer)
>> +{
>> + ngx_shmtx_lock(&peers->shpool->mutex);
>> + ngx_http_upstream_rr_peer_free_locked(peers, peer);
>> + ngx_shmtx_unlock(&peers->shpool->mutex);
>> +}
>> +
>> +
>> +static ngx_inline ngx_int_t
>> +ngx_http_upstream_rr_peer_unref(ngx_http_upstream_rr_peers_t *peers,
>> + ngx_http_upstream_rr_peer_t *peer)
>> +{
>> + peer->refs--;
>> +
>> + if (peers->shpool == NULL) {
>> + return NGX_OK;
>> + }
>> +
>> + if (peer->refs == 0 && peer->zombie) {
>> + ngx_http_upstream_rr_peer_free(peers, peer);
>> + peers->zombies--;
>> + return NGX_DONE;
>> + }
>> +
>> + return NGX_OK;
>> +}
>> +
>> #else
>>
>> #define ngx_http_upstream_rr_peers_rlock(peers)
>> @@ -121,6 +206,8 @@ struct ngx_http_upstream_rr_peers_s {
>> #define ngx_http_upstream_rr_peers_unlock(peers)
>> #define ngx_http_upstream_rr_peer_lock(peers, peer)
>> #define ngx_http_upstream_rr_peer_unlock(peers, peer)
>> +#define ngx_http_upstream_rr_peer_ref(peers, peer)
>> +#define ngx_http_upstream_rr_peer_unref(peers, peer) NGX_OK
>>
>> #endif
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> https://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> --
> Roman Arutyunyan
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
The rest of the comments make sense and will be addressed in the next
revision.
Thanks for the review!
More information about the nginx-devel
mailing list