[PATCH 1 of 9] Upstream: re-resolvable servers

[Aleksei Bavshin]

On 7/9/2024 9:22 AM, Roman Arutyunyan wrote:
> Hi,
> 
> On Mon, Jul 08, 2024 at 06:20:58PM +0400, 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>
> 
> [..]
> 
>>> 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;
> 
> Missed this last time.  This field should be removed since all resolving is in
> worker #0.

Unfortunately, that would break the ABI compatibility between OSS and 
Plus. Replacing the field with yet another NGX_COMPAT_BEGIN isn't any 
better than leaving it in the opensource code.

> 
>>> +    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; */
>>
>>> +
>>>       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;
> 
> [..]
> 
> --
> Roman Arutyunyan
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel