Http2 idle connection and worker shutdown

[Maxim Dounin]

Hello!

On Thu, Oct 20, 2022 at 10:51:06AM -0700, Jojy Varghese via nginx-devel wrote:

> Hi
>   We noticed that the Http2 idle flag 
>   (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.h#L126) 
>   is initialized to `1` 
>   (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.c#L335) 
>   but not reset by  the http2 state machine. It also looks like 
>   Nginx does not set keep alive timer for http2 as the keep 
>   alive setter code path is short circuited for http2 
>   (https://github.com/nginx/nginx/blob/master/src/http/ngx_http_request.c#L2706). 
> 
>   During worker shutdown when idle connections are reaped 
>   (https://github.com/nginx/nginx/blob/master/src/core/ngx_connection.c#L1363), 
>   due to the fact that http2 connections are always `idle`,  
>   ongoing streams end up prematurely getting closed on the read 
>   side(sends GOAWAY frame). 

No streams are closed by the http2 connection read handler when 
it's called by ngx_close_idle_connections().  Consider re-checking 
the ngx_http_v2_read_handler() code:

http://hg.nginx.org/nginx/file/tip/src/http/v2/ngx_http_v2.c#l365

Instead, the handler closes the connection as long as there are no 
active streams.  If there are any active streams, it sends the 
GOAWAY frame to the connection, so the client will know that no 
additional streams will be accepted, and proceeds with normal 
handling of the remaining streams.

> It looks like the `idle` connection implementation as it is 
> today only answers the question - “Can nginx prematurely close 
> the streams when shutting down?”   Is the above behavior 
> intentional? 
> 
> It appears from the name of the flag (`idle`) that the intent is 
> to reflect the dynamic state of the connection (are reads and 
> writes happening for the connection for any of the streams) but 
> the implementation does not reflect that.
> 
> What is the intended behavior ? We would have liked a `idle` 
> flag with its corresponding timeout configuration that lets us 
> control the behavior during shutdown (and other flows).

The "idle" flag used to be a way to distinguish HTTP connections 
without active requests and therefore prepared to be closed during 
shutdown by calling c->read->handler() with c->close set.  With 
HTTP/2, essentially all connections are prepared to be closed 
during shutdown (in some cases this may take a while though), so 
all HTTP/2 connections have c->idle flag set.  While from 
linguistic point of view this isn't exactly correct, it's 
certainly correct from semantic point of view and believed to 
result in correct behaviour.

If you think that nginx does something wrong, you may want to 
elaborate on the details of the incorrect behaviour you observe 
(as well as why you think it's incorrect, and how would you like 
to improve it).

Hope this helps.

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