new directive: "proxy_next_tries N"

[Maxim Dounin]

Hello!

On Wed, Apr 24, 2013 at 04:32:20PM +0300, Shai Duvdevani wrote:

> >> diff -ur /old/src/http/ngx_http_upstream.c
> /new/src/http/ngx_http_upstream.c
> >> --- /old/src/http/ngx_http_upstream.c    2013-04-21 18:25:09.619437856
> +0000
> >> +++ /new/src/http/ngx_http_upstream.c    2013-04-23 21:29:06.106568703
> +0000
> >> @@ -2904,6 +2904,11 @@
> >>      if (status) {
> >>          u->state->status = status;
> >>
> >> +        if (u->conf->next_upstream_tries != NGX_CONF_UNSET_UINT &&
> ++r->us_tries >= u->conf->next_upstream_tries) {
> >> +                ngx_http_upstream_finalize_request(r, u, status);
> >> +                return;
> >> +        }
> >> +
> >>          if (u->peer.tries == 0 || !(u->conf->next_upstream & ft_type)) {
> >>
> >>  #if (NGX_HTTP_CACHE)
> >
> >Introducing r->us_tries for this looks wrong, there is no need for
> >such counter at request level.  Instead, probably u->peer.tries
> >should be set accordingly.
> >
> >The test against NGX_CONF_UNSET_UINT looks wrong, too, and
> >suggests that configuration inheritance isn't handled properly.
> 
> [Gist: https://gist.github.com/shai-d/5446961  ]
> 
> Maxim, thank you for your review! :)
> 
> I agree about comparing to NGX_CONF_UNSET_UINT. It should be set to 0
> (endless tries) by default.
> 
> I avoided u->peer.tries because we wanted N retries per request and not N
> retries per upstream.
> As I understand it, all requests share the same instance of peers.
> If this is the case, In a high concurrency system with some percentage of
> errors, peers will statistically always have tries > N and many requests
> will be lost.
> Am I wrong?

The u->peer structure is allocated per request (it is actually a 
ngx_peer_connection_t sturcture within the ngx_http_upstream_t 
stucture).  It is initialized once request enters upstream module 
and nginx starts to talk to a group of upstream servers, see 
ngx_http_upstream_init_request() function.

-- 
Maxim Dounin
http://nginx.org/en/donation.html