Skip to content

Conversation

evanj
Copy link

@evanj evanj commented Sep 17, 2025

Motivation

I recently ran into problems where we accidentally "lost" the tcp_nodelay=true setting, due to refactoring some code to make it easier to test. This caused mysterious 40 ms latency increases. I hope this makes it less likely for others to make this mistake.

Solution

The documentation of the Server::tcp_nodelay() function said "Enabled by default", but that was only true when using Server::builder(). The default() method set this to false. To fix this:

  • Change Server::default() to set tcp_nodelay: true.
  • Change Server::builder() to just call Server::default().
  • Add a test to verify the settings for nodelay and keepalive.
  • Document the functions to note that the TCP settings are ignored when using serve_with_incoming. tcp_keepalive already had this note. I added the same documentation to tcp_nodelay and to serve_with_incoming so it is less likely to be missed.

The documentation of the Server::tcp_nodelay function said "Enabled
by default", but that was only true when using Server::builder().
The default() method set this to false. To fix this:

* Change Server::default() to set tcp_nodelay: true.
* Change Server::builder() to just call Server::default().
* Add a test to verify the settings for nodelay and keepalive.
* Document the functions to note that the TCP settings are ignored
  when using serve_with_incoming. tcp_keepalive already had this
  note. I added the same documentation to tcp_nodelay and to
  serve_with_incoming so it is less likely to be missed.

I recently ran into problems where we accidentally "lost" the
tcp_nodelay=true setting, due to refactoring some code to make it
easier to test. This caused mysterious 40 ms latency increases. I
hope this makes it less likely for others to make this mistake.
@evanj evanj force-pushed the evan.jones/nodelay-defaults-docs branch from 32218e1 to 03aeb02 Compare September 17, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant