Repository navigation
FEAT: support SGLang Xavier KV sharing and native GPU PD - #5631
Conversation
b4c4ce6 to
fd44b33
Compare
rogercloud
left a comment
There was a problem hiding this comment.
Major
xinference/model/llm/sglang/core.py:994— room deadline is fixed at 300s from the firstprepare()andcheck()expires first, so completed long P/D requests are discarded; extend or exempt completed rooms (see inline). Trigger: non-stream P/D request taking >300s end-to-end, or a stream whose first chunk arrives >300s after prepare. Impact: finished output is dropped with "SGLang Xavier decode did not complete its GPU KV transfer".xinference/core/worker.py:6749—recover_modelrelaunches Xavier P/D replicas, butregister_peerrejects the newnixl://host:0pool, so recovery always fails and evicts the replica; allow re-registration after unregister or fail fast (see inline).
Minor
xinference/model/llm/sglang/xavier/gpu.py:526— blockingopen()in the scheduler thread with unhandled errors and a leakedself.rooms[room]; convert toKVPoll.Failed(see inline).xinference/model/llm/sglang/pd.py:44— worker routable host forwarded tosgl.Runtime, possibly exposing the unauthenticated SGLang HTTP API; keep HTTP on loopback (see inline).xinference/core/worker.py:1867— truthiness check on_xavier_cache_configalso drops non-P/D HiCache replicas from restart recovery; gate on role (see inline).xinference/core/supervisor.py:3131—model_format != "pytorch"rejects the defaultNone; usenot in (None, "pytorch")(see inline).xinference/core/pd_model.py:518—p_taskis never awaited when decode ends first, losing prefill errors; await it beforecompleted = True(see inline).xinference/model/llm/sglang/xavier/gpu.py:263— hard-coded 120s receive deadline covers prefill queue plus compute; make configurable (see inline).xinference/model/llm/xavier/backends/torch/storage.py:54—_keys(limit=128)raises and the adapter swallows it as all-miss; chunk or raise limit (see inline).xinference/model/llm/sglang/xavier/plugin.py:31—register()patches every SGLang process unconditionally; guard onGPU_CONFIG_ENV(see inline).xinference/model/llm/sglang/xavier/config.py:130— silent BF16 to FP16 cast; warn and document (see inline).xinference/model/llm/sglang/xavier/config.py:114— synchronous full SHA-256 of all safetensors on every load; cache by (path, size, mtime) (see inline)..github/workflows/python.yaml:384—test_gc_lifecycle.pynever runs in CI andabort_request, request-body construction andrecover_modelroles lack unit tests (see inline).xinference/core/supervisor.py:5259— failed cache-actor destroy logged at debug only; relaunch may hit actor-already-exists (see inline).xinference/model/llm/sglang/core.py:983— request id registered before the lazy generator starts and leaks if never iterated (see inline).xinference/api/restful_api.py:574— stale comment about respecting an existing freeze (see inline).benchmark/benchmark_sglang_pd.py:485— cwd-relative source hashing breaks outside repo root (see inline).
Simplification
xinference/model/llm/xavier/backends/torch/canonical.py:L35: deleteencode_layer/decode_layer/export_token_slots, only tests call them. Drop until a consumer lands. (~120 lines)xinference/model/llm/xavier/contract.py:L167: deleteKVBlockKey/KVLayerMetadata/build_block_key/build_prefix_keys, reached only from tests andcanonical.py. Defer withcanonical.py. (~160 lines)xinference/model/llm/xavier/backends/torch/storage.py:L73: deletereserve/prepare/validate/release_handoff, no production callers; thepd_model.py:267release_handoffbranch is unreachable since stored SGLang handoffs always have mode "gpu". Remove both. (~60 lines)xinference/model/llm/sglang/pd.py:L34: delete the"_xavier_cache_config"check, it is popped incore.pybeforeconfigure_nixlso it never matches. Drop it. (1 line)
net: -340 lines possible
Blocking: yes — recommended event: COMMENT
Blocking issues:
xinference/model/llm/sglang/core.py:994— major — completed long PD requests discarded after the 300s room deadline [new]
|
Addressed all 17 inline review threads on Validation: 354 focused tests passed (9 hardware tests skipped locally), full-repository pre-commit and benchmark lint passed, and three real GPU tests passed for byte/chunk/cancellation correctness, end-to-end P/D cleanup, and native Runtime authentication. Updated catalogs and rendered pages were verified for all nine maintained locales. For the simplification suggestions: removed the unreachable |
rogercloud
left a comment
There was a problem hiding this comment.
Major
xinference/model/llm/sglang/xavier/gpu.py:211— SGLang direct tickets still expire after the hard-coded 120s, notXINFERENCE_SGLANG_XAVIER_TRANSFER_TIMEOUT; see inline. [prior]xinference/core/pd_model.py:494— prefill failure after decode already returned its stream leaks the decode stream and itsrequest_limitsslot; see inline.xinference/model/llm/sglang/xavier/gpu.py:624— sender/receiverabort/clearstill block the scheduler onkv_mgr.call(...)(up to 180s); see inline. [prior]
Minor
xinference/model/llm/sglang/core.py:1040— unguardedpd.release(...)infinallycan turn a successful result into an error; see inline.xinference/model/llm/sglang/xavier/storage.py:37— per-RPC actor-loop acquire/release recreates the loop and connection for every HiCache batch; see inline.xinference/core/supervisor.py:3281— native NIXL host skipsresolve_nixl_host, so a0.0.0.0worker advertises an unroutable bootstrap host; see inline.xinference/model/llm/sglang/core.py:868— PD decode stream lacksraise_for_statusand treats a missing/abortfinish as success; see inline.xinference/model/llm/sglang/core.py:799—cache_saltis forwarded but Xavier HiCache keys may not include it (unverified); see inline.xinference/model/llm/sglang/xavier/config.py:87— explicitnullconfig values bypass.get()defaults; see inline.- Test coverage:
xinference/model/llm/sglang/tests/test_xavier_gpu.pyandtest_xavier_gpu_pd.pyare gated onXINFERENCE_TEST_SGLANG_XAVIER, which no workflow sets (.github/workflows/pd-gpu.yamlonly runs the vLLM suites), so the real CUDA IPC/NIXL SGLang path never runs in CI. Add them to the GPU job or document how to run them manually.
Simplification
xinference/model/llm/sglang/xavier/pd.pyL18: delete: thepage_sizeparameter ofSGLangXavierHandoff.__init__is never read; drop it and the argument passed fromcore.py.
net: -2 lines possible
Blocking: yes — recommended event: COMMENT
Blocking issues:
xinference/model/llm/sglang/xavier/gpu.py:211— major — SGLang GPU handoff fails when decode pulls >120s after prefill registration, regardless of the configured transfer timeout [prior]
|
Updated on Eight of the nine new inline threads are addressed: the >120-second ticket lease, decode stream/serve-slot cleanup after prefill failure, cleanup-error preservation, persistent HiCache RPC connections, worker-local wildcard bootstrap resolution, HTTP/terminal response failure handling, verified salted HiCache isolation, and nullable model defaults. Removed the unused handoff constructor page_size argument. The PR description now documents the exact opt-in SGLang GPU test command. The abort thread remains open. clear() is asynchronous, but abort() still drains active GPU access before returning because native SGLang immediately reclaims the KV slots afterward. An asynchronous RPC alone would allow those slots to be overwritten before transfer completion. This is an explicit remaining scheduler-liveness limitation under the native ownership contract. Validation: 697 focused tests passed, 18 hardware/platform skips locally; full-repository pre-commit and benchmark lint passed. Five installed-SGLang/CUDA tests passed for salt isolation, remote HiCache reuse, byte/chunk/cancellation correctness, P/D lifecycle and native Runtime authentication, with no foreign GPU interference. The merged CI dependency stack passes runtime imports; 18 catalogs and the two deployment pages were validated in all nine locales. Performance figures remain the final controlled measurements labeled with their measured head. Previous-head CI failures were inspected individually:
The last two are existing main concurrency issues outside the SGLang adapter changes. Their implementations are unchanged in this PR. Fresh CI is pending on the new head. |
rogercloud
left a comment
There was a problem hiding this comment.
Minor
xinference/model/llm/sglang/xavier/gpu.py:254:wait_donehas no deadline, and lease expiry never completes or aborts the room (inline).xinference/model/llm/sglang/xavier/directory.py:47: completed rooms are never expired, so a lost decode release leaks a slot toward the 4096 cap (inline).xinference/model/llm/sglang/xavier/gpu.py:275: fixed 1-5 ms supervisor/actor polling with no backoff for the whole prefill duration (inline).xinference/core/pd_model.py:269: the vLLM abandon path now runs for handoffs withticket=""(inline).xinference/model/llm/sglang/core.py:254:get_transport_hostre-parses an already-unbracketed IPv6 host (inline).xinference/model/llm/xavier/backends/torch/storage.py:155:get()hits don't refresh LRU recency (inline).xinference/core/supervisor.py:3098: SGLangenable_xavierlacks the vLLMreplica<=1downgrade (inline).doc/source/user_guide/pd_separation.rst:49: still says "In the Web UI, select vLLM", although the frontend and the new SGLang section (L56) now support SGLang through the Web UI. Change it to "select vLLM or SGLang".- [prior]
xinference/model/llm/sglang/tests/test_xavier_gpu.py/test_xavier_gpu_pd.py: these are gated onXINFERENCE_TEST_SGLANG_XAVIERand no workflow sets it. The run instructions (XINFERENCE_TEST_SGLANG_XAVIER=1,XINFERENCE_TEST_PD_MODEL_PATH,SGLANG_PLUGINS) live only in the PR description. Put them in the test module docstring or a doc page so they survive merge.
Simplification
- xinference/core/supervisor.py:L3125: delete:
transport_backend != "xavier" or(unreachable, sincesglang_xavieralready requires the xavier transport). Checkn_worker != 1only. - xinference/core/supervisor.py:L3357: delete: the
hasattr(self, "_xavier_cache_mapping")guard and its initializer (__init__at L1069 always sets it). Assign intoself._xavier_cache_mappingdirectly. - xinference/core/supervisor.py:L5246: delete: the
getattr(self, "_xavier_cache_mapping", {})fallback. Useself._xavier_cache_mapping.pop(model_uid, None).
net: -2 lines possible
Blocking: no — recommended event: APPROVE
|
Addressed this review in 0910aaa and replied to/resolved all seven inline threads. The review-body suggestions are also handled: removed the redundant supervisor checks/fallbacks (cache mapping now initializes in the constructor), documented both GPU test invocations in the test modules, and updated the Web UI instructions and timeout/lease behavior in English and all nine locales. Validation: 588 focused tests and two rank0 cleanup tests passed; all five opt-in two-GPU SGLang tests passed, covering remote prefix restore, real GPU bytes, concurrent P/D, streaming disconnect/recovery, ordinary-engine output equivalence and authenticated native startup. Pre-commit, catalog validation/compilation and rendered English/nine-locale documentation passed. GPU test processes exited cleanly. Also corrected the Windows 3.10 shutdown test's floating-point boundary ( |
Adds Xavier KV-cache sharing and prefill/decode separation to SGLang, with native SGLang NIXL available through the same Xinference API and P/D router.
Changes
xoscar[nixl]>=0.11.1. Xavier P/D has no CPU fallback or retained GPU history; decode radix caching is rejected at launch.Final performance
Head
f26b5f669; two RTX 3090 Ti GPUs with NVLink, Qwen2.5-0.5B, FP16, SGLang 0.5.21, and identical 6 GiB KV pools per role. Values are output tokens/s on repeated warm prompts.Warm throughput is close to native; first-token tail and cold-arrival latency remain higher. Prompt-fingerprint caching benefits repeated text; unique prompts still use the existing tokenization validation.
Validation
Head
878372e67: 697 focused tests pass (18 hardware/platform-dependent tests skipped locally), full-repository pre-commit and benchmark Black/Ruff pass. Five installed-SGLang/CUDA regressions cover salted cache isolation, remote HiCache restoration, bitwise GPU transfer/chunks/cancellation, end-to-end P/D cleanup and authenticated native Runtime startup. GPU monitoring found no unrelated workload interference. The merged CI dependency stack passes its runtime import checks. All 18 updated locale catalogs pass validation and compilation; the changed pages render in English and all nine maintained languages.The full performance controls for the measured head above completed 5,536 requests with matching outputs, verified source/workload/KV capacity, and clean GPU isolation.
Manual SGLang GPU regressions
Run on Linux with two idle CUDA GPUs, SGLang 0.5.21, compatible PyTorch/NIXL,
xoscar[nixl]>=0.11.1, and local unquantized FP16 Qwen2.5-0.5B weights. The real GPU tests are opt-in and are not enabled by the automatic GPU workflow.Aborting an in-flight GPU handoff can synchronously wait for its drain fence before native SGLang reclaims the KV slots. The review records this native slot-ownership constraint;
clear()cleanup is asynchronous. A nonblocking abort requires a separate deferred-reclamation protocol.