Repository navigation
Conversation
SGLang's default `retract_decode` stably sorts the batch by output length descending and then prompt length ascending, pops victims from the end (shortest output, then longest prompt, the latest exact tie first), and keeps the survivors in that sorted order through `filter_batch(keep_indices=sorted_indices)`. After each release, `release_req` evicts cached pages toward `SGLANG_RETRACT_DECODE_STEPS` (20) tokens per remaining request, while only the next decode step's demand decides whether to retract again (python/sglang/srt/managers/schedule_batch.py L2191-L2194, L3168-L3245 and L3289-L3292, and mem_cache/common.py L184-L191, at v0.5.20). The simulator retracted the earliest request with the shortest output, kept the original batch order, and evicted only for the next step. Sort once per retraction pass, retract the last row that is not a host-loop ghost, and evict toward the reserve after each retraction. Ghost rows stay in the sorted batch but are never retracted. The pressure-event test's three-page pool now also loses its unprotected cached page to that eviction. Signed-off-by: Rudy Pei <yanrpei@gmail.com> Co-authored-by: Isaac <no-reply@databricks.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title precisely identifies the two behavioral changes: SGLang decode retraction order and post-release cache eviction. It is specific and not vague. |
| Description check | |
The description covers the required sections, behavior change, review map, risk, compatibility, test evidence, provenance, and tracking status. It provides exact commands and results, including the li… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Cross-Layer Contract | |
PASS. The diff changes no public input, schema field, CLI setting, default, or serialization declaration. It changes internal SGLang retraction and cache behavior in decode.rs (lines 243-279), and t… |
| Modeling And Data Evidence | |
The PR supplies both provenance and reproducible evidence. THIRD_PARTY_NOTICES.md:1447-1466 pins SGLang v0.5.20 commit 94602c9c..., names the upstream source paths and functions, and states the ad… |
| Compatibility Boundaries | |
No compatibility-boundary failure is introduced. The PR changes only internal SGLang scheduler logic, tests, and synchronized third-party notices. Scheduler implementation items remain pub(crate)/`p… |
| Review Evidence | |
The PR description names relevant commands and results: cargo test -p aisimulate-core and the focused SGLang test command, with pass counts and zero failures. It includes boundary and negative valid… |
Full details: Description check
-
Explanation
The description covers the required sections, behavior change, review map, risk, compatibility, test evidence, provenance, and tracking status. It provides exact commands and results, including the limitation that full CI was not run.
Full details: Cross-Layer Contract
-
Explanation
PASS. The diff changes no public input, schema field, CLI setting, default, or serialization declaration. It changes internal SGLang retraction and cache behavior in
decode.rs(lines 243-279), and the existing flow still carriespressure_eventsthroughDecodeResult,EnginePassResult, and replay evidence without a consumer change (core.rslines 1396-1400; replay component lines 940-947 and 2593-2605). The added Rust tests cover ordering, stable ties, ghost rows, reserve eviction, and no extra retraction (tests.rslines 752-828). The cache-commit consumer already handles reordered running requests, and an existing test covers arbitrary running order (tests.rslines 3507-3524). Documentation and source attribution are updated in both notice files. No Python or CLI consumer is stale because the PR adds no Python/CLI contract.
Full details: Modeling And Data Evidence
-
Explanation
The PR supplies both provenance and reproducible evidence.
THIRD_PARTY_NOTICES.md:1447-1466pins SGLang v0.5.20 commit94602c9c..., names the upstream source paths and functions, and states the adaptation scope. The changed pressure regression gives an explicit before/after diff: base expected one removed page and a four-token prefix; head expects two removed pages, eight available tokens, and no prefix (crates/core/src/engine/scheduler/sglang/tests.rs:680-688). New deterministic tests assert exact victim and survivor IDs, tie handling, terminal ghosts, cache prefix eviction, available tokens, and no extra retraction (tests.rs:752-828). The implementation maps those expected outputs to explicit selection and reserve-eviction logic (decode.rs:243-280). This is an explained, reproducible regression fixture with an independently pinned upstream behavioral oracle, not binary-only validation.
Full details: Compatibility Boundaries
-
Explanation
No compatibility-boundary failure is introduced. The PR changes only internal SGLang scheduler logic, tests, and synchronized third-party notices. Scheduler implementation items remain
pub(crate)/pub(super)(crates/core/src/engine/scheduler/sglang/mod.rs:8-18,core.rs:62,decode.rs:24-27), and no Python source, schema, binding, manifest, dependency lock, or version declaration changes are present. The existing reserve default remains 20 tokens and is documented in the changed Rust code. Both notice files are identical. The unchanged release contracts still define oneaisimulatewheel and one publishableaisimulate-corecrate, with no Dynamo dependency.
Full details: Review Evidence
-
Explanation
The PR description names relevant commands and results:
cargo test -p aisimulate-coreand the focused SGLang test command, with pass counts and zero failures. It includes boundary and negative validation for ordering, terminal ghosts, reserve eviction, and an unchanged-main control. It distinguishes local test results, SGLang parity/modeling evidence, and the absence of production serving changes; it also states that Fast CI and Full CI were not run. The reviewed diff changes no workflow files and does not cite an inactive nested workflow as hosted CI evidence.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
Superseded by #583, which now contains this change together with the related SGLang cache lifecycle, eviction, reporting, and KV-event corrections. The consolidated branch has passed local core and public-API validation. |
Why and what changed
SGLang decode retraction currently selects the first shortest-output request and preserves the original survivor order. Native SGLang's length policy uses a stable output/prompt-length ordering, pops the last tie, and evicts newly released cache toward a per-survivor reserve. These differences change victims and subsequent reuse under KV pressure.
Sort once by descending output length then ascending prompt length, retract from the end, retain ordered survivors, and evict toward 20 tokens per survivor after each release. Terminal ghost rows remain in the batch but cannot be retracted. The reserve does not itself cause further retraction.
Review map
crates/core/src/engine/scheduler/sglang/decode.rs, then thedecode_retractionregressions.Evidence
cargo test -p aisimulate-core— 2,431 passed, zero failures, 1 ignored. This full suite ran before the final comment/legal-only changes; final-head focused results follow.cargo test -p aisimulate-core --lib -- engine::scheduler::sglang— 95 passed, zero failures. Nightly formatting and packaged legal-file checks passed.Modeling or data provenance
Matches SGLang v0.5.20's length-policy order and release reserve. Synthetic fixtures; this does not add native priority or beam scheduling.
Tracking
None.
This pull request and its description were written by Isaac.