Skip to content

[Doc] Update TCP connection pool docs for the default-on flip - #4473

Merged
alogfans merged 1 commit into
kvcache-ai:mainfrom
he-yufeng:docs/tcp-pool-default-on
Oct 7, 2026
Merged

alogfans merged 1 commit into
kvcache-ai:mainfrom
he-yufeng:docs/tcp-pool-default-on

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

The TCP connection pool has been enabled by default since #3569, but the docs never caught up:

  • design/transfer-engine/index.md still presents MC_TCP_ENABLE_CONNECTION_POOL as an opt-in flag.
  • troubleshooting/troubleshooting.md tells operators to export MC_TCP_ENABLE_CONNECTION_POOL=1 (now a no-op), and its "two known limitations" describe the removed dynamic pool (cleanupIdleConnections(), pool_mutex_). The flag has routed to the lane-based implementation since the lanes landed, so those limitations no longer describe any shipping code.

This PR rewrites both spots against the current implementation: default-on with 0/false/no as opt-out, bounded per-peer lanes (MC_TCP_LANES_PER_PEER, default 4), reuse only after a cleanly completed exchange, dirty sockets closed and re-established. The troubleshooting entry keeps the old-builds note (=1 for pre-flip releases) and replaces the stale limitations with the lane caveats that actually apply: no idle reaping (up to lanes-per-peer sockets held per peer for process lifetime), no keepalive probe before reuse (a middlebox-dropped idle connection stalls the next transfer instead of failing fast), and dirty-socket discard with lane cooldown retry.

Surfaced while reviewing #4464; the behavioral half of that PR is the already-merged #3569.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Docs-only change. Claims were verified against upstream/main (0d1a8040): the env parsing in TcpTransport::TcpTransport() (0/false/no opt-out, default on), lane defaults in the same constructor, dirty-socket handling in handleLaneTerminal, absence of an idle reaper or SO_KEEPALIVE in the lane/session code, and session keep-open behavior in tcp_transport_session_impl.h.

Test commands:

cd docs && uv venv && uv pip install -r requirements-docs.txt
python -m sphinx -b html -q source /tmp/mc_docs_build
pre-commit run --files docs/source/design/transfer-engine/index.md docs/source/troubleshooting/troubleshooting.md

Test results:

  • Unit tests pass
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

Sphinx HTML build completed with no warnings on either edited page; all applicable pre-commit hooks pass (codespell, end-of-files, etc.).

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Drafted with AI assistance (Kimi Code): the tool read the current lane implementation and traced which doc statements no longer match shipping code. I reviewed every changed line against the sources cited above and can defend the change end-to-end.

The pool has been enabled by default since kvcache-ai#3569, but the docs still
present it as opt-in and the troubleshooting guide's "known limitations"
describe the removed dynamic pool (cleanupIdleConnections, pool_mutex_),
not the lane-based implementation the flag now selects.

- index.md: document the default-on behavior, the bounded per-peer lanes,
  the clean-exchange reuse rule, and the 0/false/no opt-out.
- troubleshooting.md: reframe the ephemeral-port fix around the default,
  and replace the stale limitations with the lane implementation's actual
  caveats (no idle reaping, no keepalive probe on reuse, dirty-socket
  discard + lane cooldown retry).

Verified with sphinx html build (no warnings on either page) and
pre-commit run --files on both files.

@alogfans alogfans 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

@alogfans
alogfans merged commit a7daa9c into kvcache-ai:main Oct 7, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation run-ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants