Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change tcp_nodelay config to be true #4986

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

milindl
Copy link
Contributor

@milindl milindl commented Mar 7, 2025

Currently, we use Nagle's algorithm, which doesn't make much sense as we're already implementing batching behaviour with linger.ms. In cases the user wants to optimize the latency and sets linger.ms = 0, it will still be delayed, which is undesirable/buggy behaviour.

@Copilot Copilot bot review requested due to automatic review settings March 7, 2025 13:17
@milindl milindl requested a review from a team as a code owner March 7, 2025 13:17
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR updates the default configuration for the TCP_NODELAY setting to improve latency when batching with linger.ms. The changes include:

  • Updated the changelog to indicate that the Nagle algorithm is disabled (TCP_NODELAY enabled) by default.
  • Modified the configuration documentation to change the default value of socket.nagle.disable from false to true.

Reviewed Changes

File Description
CHANGELOG.md Updated changelog entries to reflect the new default setting for TCP_NODELAY.
CONFIGURATION.md Changed the default value for socket.nagle.disable from false to true.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

@marcin-krystianc
Copy link

marcin-krystianc commented Mar 10, 2025

I fully support this change. I was surprised to discover that the current default setting utilizes Nagle's algorithm. I only became aware of this while investigating some unexpected latency spikes in our benchmarks, which were resolved upon disabling Nagle's algorithm (socket.nagle.disable=true).

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.

2 participants