Propagate --tcp-nodelay to the server (wire intent, no behavior change)#99
Merged
Conversation
The flag previously configured only client-created sockets; in -R and --bidir the server is the latency-relevant bulk sender and never heard about it - the same one-direction asymmetry issues #81/#91 had for accounting. TestStart gains a wire-additive tcp_nodelay field (serde-default, omitted when false), threaded handle_test_request -> run_test -> spawn_tcp_stream_handlers, mirroring the zerocopy plumbing. The server ORs the client's request into its own data-socket default via the new server_data_nodelay helper rather than replacing it: the server has always run its data sockets with TCP_NODELAY on (harmless for full-MSS bulk writes, keeps the final partial segment from stalling), so the client's flag can only add nodelay, never disable it, and the OR is identity-true today. The explicit threading keeps the client's intent on the wire and honored if the server-side default ever changes. No capability is added: an old server ignoring the field already runs with nodelay on, so the degradation is invisible and exact - the same call window_size made. The QUIC TestStart sends false (QUIC manages its own transport; Nagle is a TCP concept), and UDP tests send false like zerocopy does. The legacy dead-code multi-port handler now references the shared SERVER_DATA_NODELAY constant so the historical default has one source of truth.
5146719 to
d20cc33
Compare
lance0
added a commit
that referenced
this pull request
Jun 11, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
From the roadmap surface audit — with a correction to the audit's premise discovered during implementation.
The honest finding: the server already hardcodes
nodelay: trueon every TCP data socket (single-port handler covers upload/download/bidir AND the legacy fallback's accepted streams; control connections have had it since the #70 fix). So--tcp-nodelaynot reaching the server had no behavioral consequence today — the audit's "download/bidir silently don't get it" was wrong at the behavior level, right only at the wire level.Why ship the propagation anyway: it puts the client's intent on the wire (
TestStart.tcp_nodelay, wire-additive) so any future change to the server-side default can't silently strand-R/--bidirusers who passed the flag. Applied asclient_requested || SERVER_DATA_NODELAYwith the default expressed as a named constant — replacing instead of OR-ing would have disabled nodelay for every client not passing the flag, regressing deliberate historical behavior. A test pins both halves (client request honored; default never regressed).No capability added: old servers ignoring the field degrade to identical behavior (same call as
window_size).Serde roundtrip + backward-compat (absent → false) + wire-omission tests included; 297 tests green.