Add server connection state machine #1760
Conversation
Motivation: The server pipeline needs a channel handler which manages the lifecycle of TCP connections. This includes: policing keepalive pings sent by the client, managing graceful shutdown, shutting down idle connections, and shutting down connections which have lived for too long. Much of this logic can be abstracted into a state machine. This change add that state machine. Modifications: - Add a server connection handler state machine which tracks the graceful shutdown state and polices keepalive pings sent by the client. - Replace a few READMEs with empty Swift files to suppress a few build warnings. Result: State machine for managing server connections.
| case .active(var state): | ||
| let tooManyPings = state.keepAlive.receivedPing( | ||
| atTime: time, | ||
| hasOpenStreams: !state.openStreams.isEmpty | ||
| ) | ||
|
|
||
| if tooManyPings { | ||
| onPing = .enhanceYourCalmThenClose(state.lastStreamID) | ||
| self.state = .closed | ||
| } else { | ||
| onPing = .sendAck | ||
| self.state = .active(state) | ||
| } | ||
|
|
||
| case .closing(var state): | ||
| let tooManyPings = state.keepAlive.receivedPing( | ||
| atTime: time, | ||
| hasOpenStreams: !state.openStreams.isEmpty | ||
| ) | ||
|
|
||
| if tooManyPings { | ||
| onPing = .enhanceYourCalmThenClose(state.lastStreamID) | ||
| self.state = .closed | ||
| } else { | ||
| onPing = .sendAck | ||
| self.state = .closing(state) | ||
| } |
There was a problem hiding this comment.
These two cases are pretty similar, except for the state that is set in the else branch. Could the duplication be avoided or do we want to see clearly the steps for each case?
There was a problem hiding this comment.
It's somewhat unavoidable here as the associated state types are different. IMO deduplicating here would also result in less readable code.
Sources/GRPCHTTP2Core/Server/Connection/ServerConnectionHandler+StateMachine.swift
Outdated
Show resolved
Hide resolved
Sources/GRPCHTTP2Core/Server/Connection/ServerConnectionHandler+StateMachine.swift
Outdated
Show resolved
Hide resolved
| if isAcceptablePing { | ||
| self.lastValidPingTime = time | ||
| tooManyPings = false |
There was a problem hiding this comment.
I assume the bad pings don't have to be consecutive, right? Otherwise, we should reset the ping strikes to 0.
There was a problem hiding this comment.
Yes, that's my understanding from the various gRFCs.
| } | ||
|
|
||
| /// Reset ping strikes and the time of the last valid ping. | ||
| mutating func reset() { |
There was a problem hiding this comment.
We're never calling this, which makes me think we haven't yet implemented the ping strike reset logic when receiving DATA/HEADERS frames - correct?
There was a problem hiding this comment.
Correct. This will have to be done when the connection handler is done as it requires input from the stream channels as well (which is a bit annoying...). There's effectively nothing to do in the state machine beyond calling reset though.
There was a problem hiding this comment.
I'll add a test that the reset actually works though as that's missing right now.
Tests/GRPCHTTP2CoreTests/Server/Connection/ServerConnectionHandler+StateMachineTests.swift
Show resolved
Hide resolved
Motivation: The server pipeline needs a channel handler which manages the lifecycle of TCP connections. This includes: policing keepalive pings sent by the client, managing graceful shutdown, shutting down idle connections, and shutting down connections which have lived for too long. Much of this logic can be abstracted into a state machine. This change add that state machine. Modifications: - Add a server connection handler state machine which tracks the graceful shutdown state and polices keepalive pings sent by the client. - Replace a few READMEs with empty Swift files to suppress a few build warnings. Result: State machine for managing server connections.
3 participants
Motivation:
The server pipeline needs a channel handler which manages the lifecycle of TCP connections. This includes: policing keepalive pings sent by the client, managing graceful shutdown, shutting down idle connections, and shutting down connections which have lived for too long. Much of this logic can be abstracted into a state machine. This change add that state machine.
Modifications:
Result:
State machine for managing server connections.