[net] internal/http3: implement idle timeout for the server
Nicholas Husin (Gerrit)
Message from Nicholas Husin![]( "Open in Gerrit")
Set Ready For Review
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil added 6 comments![]( "Open in Gerrit")
// to gracefully terminate.Preexisting condition, but I think we're missing something to stop us from accepting new connections while in shutdown.
go sc.sendGoawayAndDrain("server is shutting down")sendGoawayAndDrain is now a long-running function--it sleeps until the goawayTimeout or until the connection is closed.
The sequence of events in a shutdown operation are spread across server.shutdown and sendGoawayAndDrain, which makes it harder to reason about them. (sendGoawayAndDrain applies a fixed one-second goawayTimeout, shutdown applies whatever the context's lifetime is, and the interaction between the two is mediated through the connClosed channel.)
I think we can make this simpler:
- Let's rename sendGoawayAndDrain to serverConn.shutdown. (Describes the purpose rather than the mechanism.)
- serverConn.shutdown accepts a context, and arranges for the connection to close when the context is canceled.
- serverConn.shutdown returns when the connection is closed (and not before).
- server.shutdown calls serverConn.shutdown on each connection, waits for all the calls to return, stops the server, and returns.
The server shutdown process is now "shut down all the connections", and the connection shutdown process is entirely managed in serverConn.shutdown.
ctx, cancel = context.WithCancel(s.serveCtx)Why WithCancel here? As far as I can tell, this only gets canceled after we've stopped using it.
sc.sendGoawayAndDrain("exceeded idle timeout")Complex edge case scenario to think about: What happens if the server is shut down while a conn is in the post-idle-timeout drain phase?
I think this is a case which might be easier to handle with one or two timers rather than a blocking sendGoAwayAndDrain, but I haven't thought it through in depth.
// No lock in this section in case writing to stream blocks. This is safeVague thought not related to this CL at all:
I wonder if quic.Stream should have a way to ensure writes to a stream are non-blocking. If you're writing data to a request stream, you want to block to get pushback when send buffers fill up. But if you're writing to the control stream, maybe you don't want to ever block. (You'd want to handle a slow peer in some fashion, perhaps by closing the connection if too much data piles up in the control stream.)
sc.controlStream.Flush()Doesn't have to be in this CL, but eventually we'll want the graceful shutdown procedure from https://www.rfc-editor.org/rfc/rfc9114.html#section-5.2-8
Let's say we implement my suggestion above of passing the shutdown context to this function. Then I think we could implement https://www.rfc-editor.org/rfc/rfc9114.html#section-5.2-8 with:
Wait for in flight requests:
1. Set inFlightDeadline to 1 second in the future. (Or some other reasonable value, ideally this would be derived off of the connection RTT but that's a bit gold plated.) (Actually, we probably should have a quic.Conn method that gives you the current RTT, it's pretty useful to know.)
2. If inFlightDeadline is after the context deadline, or if the context is cancelable (ctx.Done() != nil) and has no known deadline, then skip to "wait for drain".
3. Send GOAWAY with stream ID 2^62-4.
4. Wait up to inFlightDeadline for the connection to close. If the connection closes, return.
Wait for drain:
1. Stop accepting new requests.
2. Send GOAWAY with stream ID maxRequestStreamID.
3. Wait until the context is canceled, the connection closes, or there are no active requests.
Close the connection before returning.
A few scenarios that this should handle, with a shutdown context that:
- is already canceled: We send a GOAWAY(max-seen) and close the conn
- expires 1ms in the future: We send a GOAWAY(max-seen), wait 1ms for handlers to finish, and close the conn
- expires 10s in the future: We send a GOAWAY(2^62-4), wait 1s for any new requests, send a GOAWAY(max-seen), wait 9s for handlers to finish, and close the conn
- expires never: We send a GOAWAY(2^62-4), wait 1s for any new requests, send a GOAWAY(max-seen), wait forever for handlers to finish
Related details
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |