Repository navigation
perf(training): share prefix caches wherever they are cleared at publication - #206
Merged
Merged
Conversation
|
Merge Oncall: @BobbyZhouZijian The merge oncall will route reviews and keep this pull request moving toward a clear decision. |
This was referenced Sep 3, 2026
BobbyZhouZijian
force-pushed
the
perf/enable-colocated-prefix-cache
branch
from
September 4, 2026 02:09
c4c2292 to
4d0a106
Compare
…ication Closes #207. Reef disabled SGLang's radix cache for every layout. The reason is real: a shared cache entry carries no runtime-load-ID identity, so reusing one across a publication would let a request generate under new weights from a prefix the previous weights encoded. The rollout's runtime-load spans cannot express that, because the contamination is in the prefix's encoding rather than in a suffix of the sequence — the training data would claim a provenance it does not have. What makes reuse safe is clearing the cache at every publication, and that is only possible once no request holds KV. A retract pause does exactly that; an in_place pause deliberately does the opposite. So sharing prefixes and retracting are one decision, not two, and the pause mode and the radix cache now come from the same predicate: a deployment cannot end up sharing entries it cannot clear. A colocated deployment already qualifies. It retracts because its KV has to leave the GPU anyway, and SGLang's release_memory_occupation flushes the tree with that allocation, so it gains cross-request prefix reuse here with no new mechanism — which is where an agent workload's long shared prefixes pay off. A disjoint deployment can opt in with --disjoint-prefix-sharing (default off). Publication then retracts in-flight requests and the rollout manager clears every engine's cache after the pause, trading a re-prefill of whatever was in flight for reuse by every request in between. One colocated engine also holds several scenarios' adapters, and the same prefix has different KV under each. SGLang folds the adapter into the entry's key; a build that does not is left with reuse off rather than trusted to keep the scenarios apart. The config validator now rejects a re-enabling override only for a deployment that preserves in-flight KV.
BobbyZhouZijian
force-pushed
the
perf/enable-colocated-prefix-cache
branch
from
September 4, 2026 02:28
4d0a106 to
a92e82c
Compare
The configure_sglang_runtime docstring ran 33 lines and stated the same rationale three times, in prose that read as generated commentary rather than as the reason a reader needs. It is now three paragraphs: why sharing is one decision with retracting publication, what each layout does, and the plugin hook. The five-line comment in the body restated that first paragraph, so it goes. requires_private_cache inverted the domain term the flag, the docs and the tests all use, leaving disable_radix_cache = not shares. shares_prefixes says it directly; retracts and lora become retracts_at_publication and uses_lora, and the LoRA rank test the function ran twice is read once. test_failed_pause_barrier_retires_engines defined a _Remote double identical to the module's existing _RemoteMethod. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main moved preflight's two LoRA request-schema checks into sglang/lora_schema.py under public names (#234), which collided with this branch's read of the same block. The branch's uses_lora guard stays and calls require_lora_tensor_request_schema / require_lora_distributed_request_schema; the two definitions the branch still carried are gone with main's. _adapter_scoped_prefix_cache_supported stays in preflight. lora_schema.py holds the request field sets that both the driver preflight and the engine actor check; this is a single-caller question about the radix cache, not a request schema. test_sglang_engine.py patches the moved names on preflight, the way the tests main added already do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The barrier retires every updatable engine when it fails, but left the marker at CHECKPOINT. Nothing recovers engines on that path: update_serving_weights rebuilds them only for UPDATING_WEIGHTS, so a retry paused handles that terminate_updatable_engines had already set to None, and the deployment could not publish again without operator intervention. test_pause_barrier_failure_leaves_checkpoint_replayable asserted exactly that pair — engines terminated, marker CHECKPOINT — and named it replayable. Its fake manager records the terminate call instead of dropping the handles, so it never exercised the replay it claimed. It now asserts what makes a replay possible, and a second test drives the retry through recovery. Both the publication path and the startup pause advance CHECKPOINT to UPDATING_WEIGHTS when the barrier fails, which is the status whose recovery rebuilds the engines and forces a full publication. The transition is durable, so it also survives the driver restart a failed startup pause causes. This predates the branch, but retracting publication adds a flush to the barrier, which is a new and more likely way to reach it. The adapter-scoped cache probe now answers "no" for any exception. It exists to survive a pin bump, so an unforeseen failure in a later SGLang request API must leave prefix sharing off rather than abort preflight for the deployment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he' into perf/enable-colocated-prefix-cache
Main moved inference ownership into reef.inference.sglang and publication into reef.runtime (#432), deleting the rollout manager and the preflight and bridge code this branch changed. Port the branch's behavior onto the new owners instead of keeping the deleted paths: - SGLangConfig decides prefix sharing: it defaults disable_radix_cache to off only when publication retracts (and, under LoRA, when SGLang keys radix entries by adapter), rejects overrides that re-enable sharing otherwise, and rejects PD groups with a retracting pause. - The Slime translation selects the retract pause for colocated engines or --disjoint-prefix-sharing and no longer forces the radix flag on. - The SGLang worker's retract pause flushes the cache after pausing. - TrainingPublication marks a CHECKPOINT UPDATING_WEIGHTS when a failed pause barrier (publication or startup) has retired the engines. - The adapter-scoped radix-key check moves beside the LoRA schema checks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BobbyZhouZijian
marked this pull request as ready for review
September 19, 2026 15:53
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #207.
Motivation
Reef disabled SGLang's radix cache for every deployment. Agent requests repeatedly send system prompts, tool schemas, and conversation history, so this prevented useful prefix reuse.
A cached KV entry must not survive a weight publication: an entry carries no runtime-load-ID identity, so a prefix encoded under old weights would be matched by a request generating under the new ones. This change couples prefix sharing to publications that retract in-flight requests and clear the cache before the weights change.
Changes
After #432 moved inference ownership into
reef.inference.sglangand publication intoreef.runtime, this branch was re-ported onto those owners (the earlier rollout manager, preflight and bridge edits no longer exist):reef/inference/sglang/config.py:SGLangConfigowns the sharing decision. Whendisable_radix_cacheis unset, sharing is enabled only ifpause_mode == "retract"and, under LoRA, SGLang keys radix entries by adapter. An explicitdisable_radix_cache: false(global or per group) is rejected otherwise. Aretractpause mode rejects prefill/decode groups, because the pinned scheduler cannot retract disaggregated requests.reef/inference/sglang/lora_schema.py:adapter_scoped_prefix_cache_supported()exercises SGLang's request and radix-key APIs: identical tokens must share a key under one adapter and differ under another. No signature inspection is used. Any failure answers "no".reef/train/slime_backend/inference.py+slime_arguments.py: colocated engines, or regular disjoint engines with--disjoint-prefix-sharing, select theretractpause mode. The translation no longer forces the radix flag. It keeps an explicit opt-out (inference.options.disable-radix-cache: true) and otherwise leaves the default toSGLangConfig.reef/inference/sglang/worker.py: aretractpause flushes each engine's cache after pausing. A failed flush raises, and the publication then retires the engines.reef/inference/sglang/plugin.py: while paused inretractmode, the destructive-memory idle check ignores the grammar queue as well as waiting requests. Grammar polling stops during a pause, so requests still compiling would otherwise block every flush. Queued requests and their compilation futures are preserved, and all GPU idle checks remain. Theinspect-based signature probing is replaced by the pinnedignore_waitingAPI.reef/runtime/publication.py: a failed pause barrier aborts, which retires the engines.TrainingPublication.publishandprepare_recoverynow move aCHECKPOINTmarker toUPDATING_WEIGHTSin that case. The retry therefore rebuilds the engines and forces a full publication instead of replayingCHECKPOINTagainst retired handles. Previously the marker stayedCHECKPOINT, and the health check reported the failure as not recoverable.Compatibility and operational impact
training.options.disjoint-prefix-sharing(off by default). Regular disjoint engines keep in-place pauses and no sharing unless they opt in. With the option, each publication retracts in-flight requests and re-prefills them under the new weights.SGLangConfignow acceptsdisable_radix_cache: falseonly where it is safe; it previously rejected it always. It also rejects unknownpause_modevalues, and PD groups underretract.disable_radix_cache.UPDATING_WEIGHTS, which is replayable in place, instead ofCHECKPOINT.Verification
CPU, at head
873c49c0(Python 3.12, pinned Slime and SGLang installed):The same 19 tests also fail on unmodified
origin/mainin this environment. The main cause is thatgit-lfsis not installed, plus several CLI and connector startup tests. None of them touch this change.Regression coverage includes:
GPU: 1× NVIDIA RTX PRO 6000 Blackwell per scenario, driver 580.173.02, torch 2.11.0+cu130, Ray 2.58.0, SGLang
f7ea06e1e519c569c1d6cb7e1c7b1128cbdbc39f, Qwen2.5-0.5B-Instruct.Each scenario runs Reef's
ReefSGLangEngineas a Ray actor with the Reef SGLang plugin enabled, builds its configuration through the Slime translation andSGLangConfig, and publishes in Reef's order:_SGLangInferenceEngines.pause()(pause plus flush), optional memory release and resume,update_weights_from_diskwith a new runtime load ID, thencontinue_generation. The prompt shares a prefix of about 2.2k tokens.in_place, radix disabled, 0 cached tokens on a repeated prefix; 4 in-flight 3000-token requests continue across an in-place publication ✅--disjoint-prefix-sharingretract, radix enabled; second request reuses 2187/2189 prompt tokens; 0 cached tokens right after a publication, 2187 again afterwards ✅engine:2for the 872 tokens before the barrier and onlyengine:3for the 2128 after ✅retract, radix and memory saver enabled; pause, flush, full memory release and resume, then weights; 6 × 3000-token requests plus 8 JSON-schema requests complete; tokens split 955 old / 2045 new; cache is empty after a publication (2187 → 0) ✅The pinned SGLang's paused
flush_cachealready ignores the waiting queue (is_fully_idle(ignore_waiting=self._engine_paused)). The plugin's contribution is the grammar queue, which the negative control confirms. The GPU host needed its GCE gIB NCCL net shim removed fromLD_LIBRARY_PATHand itsNCCL_*overrides unset; this is a single-node host setting unrelated to Reef.Not covered on GPU:
The marker changes are covered by the CPU tests above.
AI assistance
main, updated the tests, and ran the CPU and GPU validation above.Checklist
README.i18n.yaml, or do not affect README content.Review and merge
The Merge Oncall is assigned automatically. See the
maintenance model
for review and merge requirements.
🤖 Generated with Claude Code