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

[Roman Arutyunyan]

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.

> > +    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