docs: discarded server TCP connection config #1473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Server type accepts TCP configuration parameters (keepalive & TCP_NODELAY) which are only respected when using serve() or serve_with_shutdown(). Serve methods that take a stream of AsyncRead + AsyncWrite are necessarily unaware of the underlying transport, and therefore silently discard any TCP specific configuration.
LucioFranco
approved these changes
Aug 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some methods on the
Routerutilise the TCP configuration of theServer, and others don't.It's fairly obvious why when looking at the generic bounds, but we were using
serve_with_incoming()with aTcpListenerStream(which seems to be not uncommon) and the author hadn't made the mental leap that this would causeTCP_NODELAYto be disabled / default discarded.I nearly didn't open this, because it's somewhat obvious in hindsight, but a small docs tweak might save some others a bit of confusion.
Solution
Update the docs. Maybe it needs a comment on the
Server'stcp_keepalive()andtcp_nodelay()methods instead?