[shipiyouniao]

Summary

• keep SGLang KVCacheManager map/unmap ownership local to each TP worker
• preserve the real TP world size for rank-aware kvcached IPC listener setup
• add a CPU regression test covering the ownership split

Problem

SGLang constructs and drives a KV pool independently in every TP worker. Passing the real TP world size into each KVCacheManager makes every worker broadcast its own local map/unmap operations. During null-block reservation, peers receive a second map for their already-mapped local page 0 and emit Page 0 is already mapped.

The TP world size is still required by init_kvcached() for listener/socket setup, so this change only scopes the manager itself to world_size=1.

Local Pool Ownership Versus TP Identity

The manager construction uses world_size=1 only for a worker's local allocator:

manager = KVCacheManager(
    num_blocks, block_size, cell_size, num_layers,
    world_size=1,
    pp_rank=_pp_rank,
    # Remaining manager options are unchanged.
)

The integration still retains the real TP size for listener setup. This is an ownership correction, not a change from TP inference to single-rank inference.

Before

sequenceDiagram
    participant A as TP worker A and its pool
    participant B as TP worker B and its pool
    A->>A: Initialize and map local page 0
    B->>B: Initialize and map local page 0
    A->>B: Broadcast A local map operation
    B-->>A: Page 0 is already mapped
    Note over A,B: Every worker already drives its own local pool

Loading

After

sequenceDiagram
    participant A as TP worker A
    participant PA as A local pool
    participant B as TP worker B
    participant PB as B local pool
    A->>PA: Map or unmap local pages
    PA-->>A: Local result
    B->>PB: Map or unmap local pages
    PB-->>B: Local result
    Note over A,B: Real TP identity retained for the integration and listeners
    Note over PA,PB: No extra broadcast of independently owned pool operations

Loading

The observed failure was duplicate page mapping, not a proven model outage. The paired results below remain historical measurements at their listed revisions; adding this explanation does not constitute a new TP2 or chaos run.

Real TP2 validation

Tested on two physical RTX 4090 GPUs with strict Docker GPU isolation:

• image: docker.1ms.run/lmsysorg/sglang:v0.5.13
• model: Qwen3-8B
• topology: SGLang TP2 on physical GPUs 6 and 7
• PyTorch: 2.11.0+cu130
• CUDA runtime: 13.0
• MPS active thread percentage: 100
• kvcached page preallocation: disabled

Both runs used the same image, model, launch arguments, and environment. The fixed run differed only by overlaying this PR's kvcached/integration/sglang/interfaces.py.

Case	Commit	Manager world size	Page 0 is already mapped	Request result
Base	ce76a129	2	2	1/1 HTTP 200, non-empty OK.
Fix	79b8e5d5	1	0	5/5 HTTP 200, non-empty OK.

Neither same-condition run had a scheduler exception, CUDA error, OOM, illegal-memory error, or traceback. The base run remained serviceable, so the verified defect is the duplicate cross-worker map operation and its error, not a demonstrated server outage.

SGLang 0.5.13's idle pool invariant was set to non-fatal warning mode in both runs because the existing scheduler_memory_leak compatibility patch does not apply to that SGLang version. This is independent of the ownership change and was held constant across the comparison.

Focused validation

• python -m pytest -q tests/test_sglang_local_pool_ownership.py
• python -m ruff check kvcached/integration/sglang/interfaces.py tests/test_sglang_local_pool_ownership.py
• python -m isort --check-only kvcached/integration/sglang/interfaces.py tests/test_sglang_local_pool_ownership.py
• repository pre-commit and mypy CI: passed

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[Copilot]

Pull request overview

This PR adjusts the SGLang integration so that each tensor-parallel (TP) worker owns its KV pool map/unmap lifecycle locally (avoiding cross-worker broadcast conflicts), while still preserving the real TP world_size during init_kvcached() for rank-aware IPC listener setup. It also adds a CPU-friendly regression test to validate the ownership split behavior.

Changes:

• Force KVCacheManager construction in SGLang integration to use world_size=1 to keep map/unmap operations local to each TP worker.
• Preserve and retain the real TP world_size in init_kvcached() for listener/socket setup.
• Add a CPU regression test to validate that IPC listener setup still uses the real TP size while manager ownership remains local.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
kvcached/integration/sglang/interfaces.py	Constructs KVCacheManager with world_size=1 to prevent cross-TP-worker broadcast of local map/unmap operations.
tests/test_sglang_local_pool_ownership.py	Adds regression coverage ensuring init uses real TP size for IPC listener setup while manager ownership stays local.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shipiyouniao]

Moving this back to draft until a real SGLang TP runtime reproduces the duplicate local-pool mapping failure and confirms the ownership split fixes it. The CPU regression test alone is not enough evidence for changing runtime ownership semantics.

[shipiyouniao]

Real SGLang TP2 validation is now complete on two RTX 4090 GPUs with SGLang v0.5.13 and Qwen3-8B. Under identical launch conditions, the base commit emitted two duplicate page-0 mapping errors while the fix emitted none; base returned 1/1 valid response and the fix returned 5/5 valid responses. I updated the PR description with the exact environment, commits, caveat, and results, and corrected the claim so it does not imply a verified server outage.

[Copilot]

Pull request overview

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

Suppressed comments (1)

tests/test_sglang_local_pool_ownership.py:57

• This test mutates module-level globals in kvcached.integration.sglang.interfaces via init_kvcached() (e.g., _world_size, _pp_rank, _async_sched, _kvcached_device) but only _kvcached_initialized is monkeypatched. Those globals will remain changed after the test, which can create order-dependent behavior if any later test imports and uses this module in the same process.

    monkeypatch.setattr(interfaces, "_kvcached_initialized", False)
    monkeypatch.setattr(interfaces, "_init_kvcached_impl", lambda *args: initialized.append(args))
    monkeypatch.setattr(
        interfaces,
        "start_worker_listener_thread",
        lambda tp_rank, pp_rank: listeners.append((tp_rank, pp_rank)),
    )
    monkeypatch.setattr(interfaces, "KVCacheManager", FakeManager)

    interfaces.init_kvcached(
        tp_rank=2,
        world_size=4,
        pp_rank=1,
        device="cuda:2",
        async_sched=True,
    )

[shipiyouniao]

Addressed the fresh Copilot review on the current head by isolating every SGLang module global mutated by init_kvcached() (_kvcached_initialized, device, async mode, TP world size, and PP rank).

Validation:

• focused pytest: 1 passed
• Linux full pre-commit run --all-files: passed
• Linux Python 3.12 focused pytest: 1 passed

Updated head: 53c57330.

[jeff3071]

Run this patch with Qwen3-8B TP=2 on 2xA2000. There is no CUDA or engine failure.

[jeff3071]

LGTM.

[rishabhsinha17]

LGTM, this is the right fix for the 0.5.20 lane. sglang runs one scheduler process per TP rank (tensor parallel), each driving its own KV pool, so broadcasting map and unmap to peers repeats their own local work, page 0 being the first collision.

the one line removes the peer broadcast and nothing else. world_size in the manager and csrc/page_allocator.cpp gates only the broadcast callbacks, no sizing reads it, should_use_worker_ipc is false in sglang processes, and the real TP size still reaches the listener threads and observability.

cpu checks pass at 665677f and merged with current main. reverting the line fails both TP cases of the new test, and the merge after #480 and #482 gives 178 passed, with the one no-cuda listener failure identical on plain main. TP2 evidence is shipiyouniao's 4090 table and jeff3071's A2000 run.