grpc / grpc-java / #20535
89%

DEFAULT BRANCH: master
Ran
Jobs 1
Files 696
Run time 1min
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
coverage: 89.464% (+0.04%) from 89.428%
#20535

push

github

web-flow 
netty: drain before closing a gracefully shut down server connection (#13086)

Fixes #9566.

After the second GOAWAY of a graceful shutdown (`maxConnectionAge`,
`maxConnectionIdle`, `Server.shutdown()`), `Http2ConnectionHandler`
closes the socket as soon as the last stream's final frame has been
written to the kernel. If the client hasn't read the response yet, the
response is still in the server's send queue. The client's next frame (a
BDP PING or WINDOW_UPDATE) then reaches a socket the server has closed,
so the server's kernel answers with RST and drops the unsent bytes,
trailers included. The client fails an RPC the server completed, with
`UNAVAILABLE: Connection closed after GOAWAY. HTTP/2 error code:
NO_ERROR, debug data: max_age`. The response headers had already
arrived, so the call is committed and isn't retried. The full analysis
and a standalone reproducer are in
https://github.com/grpc/grpc-java/issues/9566#issuecomment-5930328142.

### Change

Once the second GOAWAY has been sent and no streams are active,
`NettyServerHandler` keeps the connection open and reading for up to 1
second before closing. The client normally closes first, as soon as it
has read the GOAWAY and its streams have completed, so the drain usually
ends early. This is the approach Go's HTTP/2 server took
(golang/net@cd69bc3).

- It overrides `isGracefulShutdownComplete()`, the hook
`NettyClientHandler` already uses, so the close still goes through
`Http2ConnectionHandler` and works the same with TLS.
- It never extends the grace time: if `maxConnectionAgeGrace` (or the
shutdown grace) runs out first, the connection closes then.
- The drain is cancelled when the channel becomes inactive.

### Tests

- `NettyServerHandlerTest`: 9 tests asserted that the channel closes
immediately after the second GOAWAY. They now advance the fake clock by
the drain time first. Two new tests:
`maxConnectionAge_drainStartsWhenLastStreamCloses` and
`maxConnectionAgeGrace_shorterThanDrain_clo... (continued)

39401 of 44041 relevant lines covered (89.46%)

0.89 hits per line

Coverage Regressions

Lines Coverage ∆ File
47
91.26
0.6% ../netty/src/main/java/io/grpc/netty/NettyServerHandler.java
23
93.56
0.04% ../netty/src/main/java/io/grpc/netty/NettyClientHandler.java
5
83.02
-9.43% ../core/src/main/java/io/grpc/internal/Http2Ping.java
4
75.25
-3.96% ../servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java
3
95.1
-0.43% ../core/src/main/java/io/grpc/internal/RetriableStream.java
2
81.05
-2.11% ../okhttp/src/main/java/io/grpc/okhttp/ExceptionHandlingFrameWriter.java
2
93.1
-0.14% ../okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
1
90.97
0.35% ../core/src/main/java/io/grpc/internal/DelayedClientCall.java
1
87.63
-0.21% ../inprocess/src/main/java/io/grpc/inprocess/InProcessTransport.java
1
84.36
-0.41% ../netty/src/main/java/io/grpc/netty/Utils.java
Jobs
ID Job ID Ran Files Coverage
1 #20535.1 696
89.46
Source Files on build #20535