[PATCH] Documented behaviour of a single server in upstream with keepalive

[Sergey Kandaurov]

> On 11 Oct 2022, at 18:50, Yaroslav Zhuravlev <yar at nginx.com> wrote:
> 
>> On 10 Oct 2022, at 00:55, Maxim Dounin <mdounin at mdounin.ru> wrote:
>> 
>> Hello!
>> 
>> On Mon, Oct 03, 2022 at 09:21:52PM +0100, Yaroslav Zhuravlev wrote:
>> 
> [..]
>> 
>>> diff --git a/xml/en/docs/http/ngx_http_upstream_module.xml b/xml/en/docs/http/ngx_http_upstream_module.xml
>>> --- a/xml/en/docs/http/ngx_http_upstream_module.xml
>>> +++ b/xml/en/docs/http/ngx_http_upstream_module.xml
>>> @@ -10,7 +10,7 @@
>>> <module name="Module ngx_http_upstream_module"
>>>        link="/en/docs/http/ngx_http_upstream_module.html"
>>>        lang="en"
>>> -        rev="88">
>>> +        rev="89">
>>> 
>>> <section id="summary">
>>> 
>>> @@ -351,6 +351,11 @@
>>> If there is only a single server in a group, <literal>max_fails</literal>,
>>> <literal>fail_timeout</literal> and <literal>slow_start</literal> parameters
>>> are ignored, and such a server will never be considered unavailable.
>>> +If an error occurred while trying to reuse a
>>> +<link id="keepalive">keepalive</link> connection
>>> +with a single server, and the request is allowed to be passed to the
>>> +<link doc="ngx_http_proxy_module.xml" id="proxy_next_upstream">next server</link>
>>> +on error, such server will be selected again.
>>> </note>
>>> </para>
>> 
>> If an error occurs?
>> 
>> The "with a single server" clause looks wrong, we are talking 
>> about a group with only a single server here.  It probably should 
>> be either "with such server" or "the server" (probably "... with 
>> such server ... the server will be ..." would be good enough 
>> considering the whole sentence).
> 
> Thanks, updated:
> 
> # HG changeset patch
> # User Yaroslav Zhuravlev <yar at nginx.com>
> # Date 1663861151 -3600
> #      Thu Sep 22 16:39:11 2022 +0100
> # Node ID 3b878f0c18cc277bfccb6095afd2cc7dc0cdec0f
> # Parent  9708787aafc70744296baceb2aa0092401a4ef34
> Documented behaviour of a single server in upstream with keepalive.
> 
> diff --git a/xml/en/docs/http/ngx_http_upstream_module.xml b/xml/en/docs/http/ngx_http_upstream_module.xml
> --- a/xml/en/docs/http/ngx_http_upstream_module.xml
> +++ b/xml/en/docs/http/ngx_http_upstream_module.xml
> @@ -10,7 +10,7 @@
> <module name="Module ngx_http_upstream_module"
>         link="/en/docs/http/ngx_http_upstream_module.html"
>         lang="en"
> -        rev="88">
> +        rev="89">
> 
> <section id="summary">
> 
> @@ -350,7 +350,13 @@
> <note>
> If there is only a single server in a group, <literal>max_fails</literal>,
> <literal>fail_timeout</literal> and <literal>slow_start</literal> parameters
> -are ignored, and such a server will never be considered unavailable.
> +are ignored, and such server will never be considered unavailable.
> +If an error occurs
> +while passing a request through
> +a <link id="keepalive">keepalive</link> connection to such server
> +and the request is allowed to be passed to the
> +<link doc="ngx_http_proxy_module.xml" id="proxy_next_upstream">next
> +server</link> on error, the server will be selected again.

I am in doubt about the note "on error", as such condition can be
insufficient to express the desired next server logic, e.g.:
    proxy_next_upstream error non_idempotent;

So it could be enough to write
"and the request is allowed to be passed to the next server ...".

Next, usually we don't refer to the specific proxy module
from the upstream module documentation.
Consider the following existing text for the upstream block:

: If an error occurs during communication with a server, the request will
: be passed to the next server, and so on until all of the functioning
: servers will be tried.

BTW, this text could be used instead as a basis to expand on to cached
connections, so far as the next server logic belongs to the upstream group,
not to a distinct server.
For example, consider the next addition under the above citation:

: If an error occurs during communication with a single server in a group
: through a <link id="keepalive">keepalive</link> connection, that means
: the request will be passed to the next cached connection instead,
: and so on until all cached connections will be tried first. 

Or just:

: <note>
: In case of communicating with a single server through
: a <link id="keepalive">keepalive</link> connection, that means
: the request will be passed to the next cached connection instead,
: and so on until all cached connections will be tried first. 
: </note>

-- 
Sergey Kandaurov