Implement graceful shutdown for http3 servers. #4407
Conversation
|
@marten-seemann Do you have time to take a look at this draft? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4407 +/- ##
==========================================
- Coverage 85.15% 84.29% -0.86%
==========================================
Files 154 154
Lines 14768 14842 +74
==========================================
- Hits 12575 12510 -65
- Misses 1688 1808 +120
- Partials 505 524 +19 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thank you for working on this @WeidiDeng!
I’ve been working on a big refactor of the http3 package (#4115, #4116, more coming), which at the end will enable HTTP datagram support, which will all be included in the next release. For proper datagram support, we’ll introduce some kind of connection and stream tracking (no PR yet), which would be helpful for this PR. I suggest we defer graceful shutdown until these PRs have landed.
http3/client.go
Outdated
| config: conf, | ||
| opts: opts, | ||
| dialer: dialer, | ||
| receivedGoawayID: quic.StreamID(-4), |
There was a problem hiding this comment.
This seems hacky. Introduce a protocol.InvalidStreamID instead?
There was a problem hiding this comment.
I got the stream ID from rfc 9000. For different kinds of streams, the criteria for invalid id is different. I think a function that verifies ID would be better instead of a new type.
| @@ -217,7 +218,8 @@ type Server struct { | |||
| mutex sync.RWMutex | |||
| listeners map[*QUICEarlyListener]listenerInfo | |||
|
|
|||
| closed bool | |||
| closed bool | |||
| connections map[*quic.Connection]func() | |||
There was a problem hiding this comment.
Better: Use the tracing ID as a map key.
There was a problem hiding this comment.
But the methods on the quic.Connection needs to be called. To properly stop server from accepting new streams at the grace period's end. Malicious client may attempt to open new streams even if goaway is received. Goaway itself doesn't close a connection.
# Conflicts: # http3/client.go # http3/client_test.go # http3/server.go # http3/server_test.go
# Conflicts: # http3/client.go # http3/client_test.go # http3/conn.go # http3/server.go # http3/server_test.go
# Conflicts: # http3/client_test.go # http3/server_test.go
Fix 6195 for caddy.