Repository navigation
[TE] Increase TCP queued transfer admission thresholds - #3710
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Transfer Engine’s classic TCP transport defaults to allow substantially more per-peer queued transfers and pending admissions before hard rejection, aiming to reduce QUEUE_FULL under high fanout.
Changes:
- Increased default per-peer queued transfer limit from 1024 to 65535.
- Increased max per-peer queued transfer limit from 65535 to 1048576.
- Increased default/max pending admission limits in the same way.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| constexpr size_t kDefaultLanesPerPeer = 4; | ||
| constexpr size_t kDefaultQueuedTransfersPerPeer = 1024; | ||
| constexpr size_t kMaxQueuedTransfersPerPeer = 65535; | ||
| constexpr size_t kDefaultPendingAdmissionsPerPeer = 1024; | ||
| constexpr size_t kMaxPendingAdmissionsPerPeer = 65535; | ||
| constexpr size_t kDefaultQueuedTransfersPerPeer = 65535; | ||
| constexpr size_t kMaxQueuedTransfersPerPeer = 1048576; | ||
| constexpr size_t kDefaultPendingAdmissionsPerPeer = 65535; | ||
| constexpr size_t kMaxPendingAdmissionsPerPeer = 1048576; |
| constexpr size_t kDefaultQueuedTransfersPerPeer = 65535; | ||
| constexpr size_t kMaxQueuedTransfersPerPeer = 1048576; | ||
| constexpr size_t kDefaultPendingAdmissionsPerPeer = 65535; | ||
| constexpr size_t kMaxPendingAdmissionsPerPeer = 1048576; |
| constexpr size_t kDefaultQueuedTransfersPerPeer = 65535; | ||
| constexpr size_t kMaxQueuedTransfersPerPeer = 1048576; | ||
| constexpr size_t kDefaultPendingAdmissionsPerPeer = 65535; | ||
| constexpr size_t kMaxPendingAdmissionsPerPeer = 1048576; |
catyans
left a comment
catyans
left a comment
There was a problem hiding this comment.
Raising both default per-peer limits from 1,024 to 65,535 is not safe without a byte-based bound or load evidence. PeerConnectionGroup retains each TcpWorkItem and its associated task/source-buffer lifetime, while the code already notes that the item limit does not bound queued bytes and that a connected payload can stall a lane indefinitely. With this change, one slow or stalled peer can retain 65,535 queued items plus another 65,535 pending admissions by default, turning an early QUEUE_FULL signal into potentially very large memory retention and extreme tail latency. The configured maximum of 1,048,576 makes the same failure mode substantially worse.
Please keep the conservative default until admission is also bounded by queued bytes, and add a sustained slow/stalled-peer benchmark reporting queue depth, queued bytes/RSS, rejection rate, and tail latency before increasing it. The PR currently says it has not been tested, so there is no evidence that this 64x increase improves throughput without destabilizing overload behavior.
|
@catyans Thanks for raising this. I agree that a byte-based admission bound is the proper long-term solution, and this change is not intended to establish the final overload policy. This is a temporary operational adjustment because the existing 1,024-item threshold is too small for normal TCP workloads: it triggers A follow-up should add queued-byte limits and the slow/stalled-peer measurements you suggested; the default can then be recalibrated based on those results. |
catyans
left a comment
catyans
left a comment
There was a problem hiding this comment.
Thanks for clarifying the intended scope. Re-reviewed the current head: this only adjusts the existing item-count defaults and maxima; it does not change queue ownership, admission timeout, or QUEUE_FULL semantics. Given that this is an operational stopgap and all checks are green, LGTM.
suggestion (non-blocking): please follow through with queued-byte admission and slow/stalled-peer measurements covering queue depth, RSS, rejection rate, and tail latency. The larger configurable maxima make those resource bounds and observability especially important.
|
@staryxchen In some stress workload (e.g. running tebench), the default value is relatively small. Note: If some users report this, they can change the config file to increase the default value in the release. |
…mits for Mooncake <= 0.3.13 A Store transfer that cannot complete blocks for Mooncake's fixed 60 s timeout. Open a breaker on the first transfer failure or a get slower than DEEPINFER_HICACHE_L3_SLOW_S (5 s); while open, skip reads and drop queued puts; close after DEEPINFER_HICACHE_L3_COOLDOWN_S (30 s, doubling on consecutive trips, capped at 10 min). Mooncake <= 0.3.13 queues at most 1024 TCP transfers per peer, fewer than one long prompt's batch put; default MC_TCP_MAX_QUEUED_TRANSFERS_PER_PEER / MC_TCP_MAX_PENDING_ADMISSIONS_PER_PEER to 65535 (the default since kvcache-ai/Mooncake#3710) unless set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Description
This PR raises the classic TCP transport's default per-peer queued transfer and pending admission limits.
Previously, TCP used:
kDefaultQueuedTransfersPerPeer = 1024kMaxQueuedTransfersPerPeer = 65535kDefaultPendingAdmissionsPerPeer = 1024kMaxPendingAdmissionsPerPeer = 65535This change increases them to:
655351048576655351048576The intent is to reduce premature
QUEUE_FULLhard rejections under high TCP transfer fanout while preserving bounded admission behavior.Note: this branch currently also adds
extern/yalantinglibsas a new gitlink without a.gitmodulesupdate. Please confirm whether that submodule pointer is intentional before merging.Module
mooncake-transfer-engine)mooncake-store)mooncake-reshard)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
How Has This Been Tested?
Not run yet.
Test commands:
# Not runTest results:
Checklist
./scripts/code_format.shAI Assistance Disclosure
AI assistance was used to inspect the TCP admission behavior and draft this PR description. The human submitter should review and validate all changed lines before merge.