Skip to content

perf(net): disable Nagle on the API server, proxy and envd clients - #109

Merged
yingdi-shan merged 1 commit into
kvcache-ai:mainfrom
cleverhu:perf/tcp-nodelay
Aug 6, 2026
Merged

perf(net): disable Nagle on the API server, proxy and envd clients#109
yingdi-shan merged 1 commit into
kvcache-ai:mainfrom
cleverhu:perf/tcp-nodelay

Conversation

@cleverhu

@cleverhu cleverhu commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What

Set TCP_NODELAY on every hop of the sandbox command path: the API server's accepted connections, the sandbox proxy's connector, and both envd client connectors (h1 and h2).

Why

Short-lived commands executed through a sandbox had a hard ~40 ms latency floor that was independent of how long the command itself ran.

envd streams a command's lifecycle as a burst of tiny Connect-RPC frames. With Nagle enabled, the sender holds the second small frame until the peer acknowledges the first, and the peer's kernel delays that ACK by ~40 ms. Every hop on this path exchanges small frames only, so the coalescing win Nagle trades latency for never materializes.

Packet captures showed exactly one ~40 ms gap per command, and the first packet after the gap was a bare ACK from the client back to the node — the signature of delayed ACK meeting Nagle. The effect is a floor rather than an additive delay:

true                 exited=43 ms    <- floor
echo hi              exited=44 ms    <- floor
echo hi; sleep 0.2   exited=208 ms   <- only +8 ms
sleep 0.5            exited=508 ms   <- only +8 ms

A minimal two-small-writes-on-one-connection reproduction confirms the mechanism and the fix:

server socket median samples (ms)
TCP_NODELAY off 41.0 ms 0, 41, 41, 41, 41, 41, 41, 42, 41, 41, 41, 0
TCP_NODELAY on 0.1 ms all 0

The leading and trailing zeros are the kernel's quickack window, which also explains the reported "fast after idle, slow when run back to back" behavior.

Related issue

None. Submitted directly per CONTRIBUTING: the scope is a socket option on an existing code path with no API or format impact. Happy to open an issue first if maintainers prefer.

Scope and non-goals

Included: TCP_NODELAY at the three places that carry envd RPC traffic.

Excluded:

  • WebSocket/PTY connections — tokio-tungstenite already sets the option.
  • TCP keepalive on the proxy connector, which addresses a different problem and is sent as a separate PR.
  • Any change to timeouts, pooling, or retry behavior.

Design and behavior changes

src/bin/server.rs uses axum::serve::ListenerExt::tap_io to set the option on each accepted connection; a failure is logged at warn and the connection proceeds, since a missing socket option is a latency problem rather than a correctness one.

src/api/proxy.rs and thirdparty/envd/src/transport.rs set it on their HttpConnectors. The envd change is factored into a small nodelay_connector() helper because both the h1 and h2 clients need it.

No change to protocol framing, ordering, or error handling. Disabling Nagle can increase packet count for senders that write many small chunks, which is precisely the traffic pattern being fixed here, and these are short-lived localhost-to-VM connections.

Compatibility and operations

  • Public API or generated protocol: N/A, no wire format or endpoint change.
  • Configuration or defaults: N/A, no new knobs. The option is applied unconditionally.
  • Snapshot manifest, artifact layout, or storage format: N/A.
  • Upgrade and rollback: transparent both ways; nothing is persisted.
  • Host requirements, permissions, ports, or dependencies: N/A. axum's ListenerExt is already available in the pinned version.

Validation

  • make fmt
  • make clippy
  • make test-unit
  • Relevant Rust integration tests
  • make -C services test (required when services/ changes)
  • Generated clients/server regenerated with the documented make target
  • Documentation updated
  • Benchmarks or performance comparison completed

Commands and results:

$ cargo fmt --all -- --check
(clean)

$ cargo clippy --workspace --all-targets -- -D warnings
Finished `dev` profile

$ cargo check --workspace --all-targets
Finished `dev` profile

Mechanism A/B (minimal reproduction, server writes two small frames on one connection):
  server TCP_NODELAY=off   median 41.0 ms
  server TCP_NODELAY=on    median  0.1 ms

Skipped checks and reasons:

  • make test-unit not run: no existing unit test observes socket options, and this change adds no branch to test. The behavior is only observable at the TCP layer, which the reproduction above covers.
  • Integration tests not run: they require root, /dev/kvm, and a provisioned host; unavailable in the environment used here.
  • End-to-end latency numbers on a deployed cluster are not included. The evidence above is the packet capture plus the isolated reproduction. Anyone with a running node can confirm with scripts/bench.py or by timing a trivial command over the API before and after.

Risks and reviewer notes

Low risk. The main reviewer question is whether disabling Nagle is right for all proxied traffic rather than just the RPC control frames. It is: the proxy carries sandbox HTTP traffic over short-lived localhost-to-VM connections, where the bandwidth saving from coalescing is irrelevant and the latency cost is paid on every request.

Second question: tap_io failures are swallowed with a warn. Failing the connection instead would turn a performance regression into an outage, which seems worse.

Most important file: src/bin/server.rs.

Checklist

  • The PR contains one coherent change and no unrelated formatting or refactoring.
  • New behavior is covered by tests, or I explained why testing is impractical.
  • Logs and examples contain no credentials, tokens, or private registry information.
  • I did not manually edit generated code without updating its source and regenerating it.

envd streams a command's lifecycle as a burst of tiny Connect-RPC
frames. With Nagle enabled, every frame after the first waits for the
peer's delayed ACK, which puts a ~40ms floor under each short-lived
command regardless of how long the command itself runs.

Every hop on that path exchanges small frames only, so the coalescing
Nagle trades latency for never materializes. Set TCP_NODELAY on the API
server's accepted connections, on the sandbox proxy's connector and on
both envd client connectors.
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s).

@LSX-s-Software LSX-s-Software self-assigned this Aug 5, 2026
@yingdi-shan
yingdi-shan self-requested a review August 6, 2026 01:40

@yingdi-shan yingdi-shan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

@yingdi-shan
yingdi-shan merged commit b735f96 into kvcache-ai:main Aug 6, 2026
8 checks passed
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.

3 participants