[nginx] Changed keepalive_requests default to 1000 (ticket #2155).

[Maxim Dounin]

Hello!

On Thu, Apr 08, 2021 at 07:56:07PM -0700, Piotr Sikora wrote:

> Hi Maxim,
> 
> > It turns out no browsers implement HTTP/2 GOAWAY handling properly, and
> > large enough number of resources on a page results in failures to load
> > some resources.  In particular, Chrome seems to experience errors if
> > loading of all resources requires more than 1 connection (while it
> > is usually able to retry requests at least once, even with 2 connections
> > there are occasional failures for some reason), Safari if loading requires
> > more than 3 connections, and Firefox if loading requires more than 10
> > connections (can be configured with network.http.request.max-attempts,
> > defaults to 10).
> >
> > It does not seem to be possible to resolve this on nginx side, even strict
> > limiting of maximum concurrency does not help, and loading issues seems to
> > be triggered by merely queueing of a request for a particular connection.
> > The only available mitigation seems to use higher keepalive_requests value.
> 
> Instead of blaming browsers, did you consider implementing graceful shutdown
> using 2-stage GOAWAY? The process is clearly described in RFC7540, sec. 6.8:
> 
>   [...] A server that is attempting to gracefully shut down a
>   connection SHOULD send an initial GOAWAY frame with the last stream
>   identifier set to 2^31-1 and a NO_ERROR code.  This signals to the
>   client that a shutdown is imminent and that initiating further
>   requests is prohibited.  After allowing time for any in-flight stream
>   creation (at least one round-trip time), the server can send another
>   GOAWAY frame with an updated last stream identifier.  This ensures
>   that a connection can be cleanly shut down without losing requests.
> 
> This is a solved problem, and the solution was pointed out years ago:
> http://mailman.nginx.org/pipermail/nginx-devel/2017-August/010439.html
> http://mailman.nginx.org/pipermail/nginx-devel/2018-March/010930.html

As you can see from the commit log, as well as the details in the 
ticket, even limiting concurrency does not help, and this means 
that two-stage GOAWAY would be useless: its only benefit is to 
make it possible to process in-flight requests without rejecting 
them.  Not to mention that all requests in tests can be easily 
retried by browsers (and many are actually retried, but not all).

Nevertheless, I've tried implementing two-stage GOAWAY as well 
while working on this, just a quick hack to see if it helps.  As 
expected from the above, it doesn't help.  Further, it triggers 
the bug in Chrome (https://crbug.com/1030255), which basically 
stops any communication with the server after the first GOAWAY, 
and does nothing till the connection is closed by the server.  
That is, a simple approach of sending GOAWAY with 2^31-1 and 
waiting for keepalive_timeout to expire and then sending the real 
GOAWAY (or waiting for the client to close the connection) clearly 
does more harm than good.  Probably it can be implemented in a way 
which doesn't hurt Chrome that much, but, given it doesn't help 
anyway, this wasn't considered.

-- 
Maxim Dounin
http://mdounin.ru/