Skip to content

fix: align SGLang cache lifecycle, eviction, and KV events - #583

Open
PeaBrane wants to merge 3 commits into
ai-dynamo:mainfrom
PeaBrane:fix/sglang-prebuilt-prompt-publication
Open

PeaBrane wants to merge 3 commits into
ai-dynamo:mainfrom
PeaBrane:fix/sglang-prebuilt-prompt-publication

Conversation

@PeaBrane

@PeaBrane PeaBrane commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Why and what changed

SGLang cache insertion, eviction recency and retraction order determine which prompts can be reused and what the KV router sees. AISimulate currently announces some pages at allocation or destination activation and caches unfinished decode output, which makes those observations differ from SGLang's radix lifecycle.

This consolidates the cache corrections and the older event-cadence change:

  • Insert complete prompts after prefill, including a completed disaggregated source before its transfer hold. Keep destination transfers private until running admission; cache full pages on ready-queue cancellation and defer running-cancellation KV insertion/release until its pending forward is observed.
  • Publish stores at radix insertion, batch compatible parent-linked stores and removals, preserve session attribution, and flush after observed prefill results, every 40 observed decode forwards and on idle. Decoded prefixes stay private until completion. Overlap result visibility and existing KV materialization accounting are preserved.
  • Refresh partial radix-edge matches/inserts and reinsertion of an already cached path. Retract in stable output-length/prompt-length order and evict toward a 20-token reserve per survivor.
  • Expose first-admission total reuse separately from later readmission reuse in per-request reports.

Review map

  • Risk level: medium; changes SGLang reuse, pressure behavior and router-event timing.
  • Start with: sglang_backend.rs (publication queue and destination lease), sglang/core.rs, decode.rs, host_loop.rs, and radix_cache.rs. Scheduler tests cover their interactions; replay/report.rs adds the report field.
  • Public or serialized contract changed: adds PerRequestRecord.first_admission_reused_input_tokens and its JSON field. Rust consumers constructing that public struct must initialize the new field. Existing reuse fields retain their meanings. SGLang handoff conformance expects zero stores at activation; other backends retain activation stores.
  • Compatibility or rollback concern: SGLang routing/cache and pressure results can change intentionally. Prefix caching disabled remains uncached. This ports the older PR onto current request leases and dense page metadata, preserving current frontend, host-loop and in-flight cleanup APIs.

Evidence

  • Tests: cargo test --locked -j 4 -p aisimulate-core — 2,475 passed, zero failures, one ignored across 19 unit/integration/doc-test binaries on the current tree. The focused overlap-loop run passed all 22 cases, also covered by the full run. The eight prefill cancellation cases reproduced premature caching or release before the fix; the regressions now verify zero pre-observation follower reuse, delayed router stores, eventual caching/release, idle decode cleanup, repeated cancellation, and request-ID reuse.
  • Public API consumer: cargo test -j 4 --manifest-path crates/tests/public-api/Cargo.toml — 16 passed, zero failures at the original consolidated head 78e6e4aa5d27723c9cb311a951b56a0a840b5685; doc-test binary also passed (zero cases). This cancellation follow-up does not change the public API; the standalone consumer was not rerun.
  • Formatting and packaged legal-file checks: nightly rustfmt --edition 2024 --check for every changed Rust file and python/aisimulate/.venv/bin/python scripts/ci/check_packaged_legal_files.py passed; root/package notices are identical.
  • Fast CI: not run locally as a separate workflow.
  • Full CI: not claimed; no CI authorization requested.
  • CodeRabbit reviewed commit: not reviewed.
  • Codex reviewed commit: fdaad18890a863ea27a0af673731a1ababe1d248; bounded gap review of the cancellation follow-up. The original combined diff was reviewed at 78e6e4aa5d27723c9cb311a951b56a0a840b5685.
  • Negative or boundary cases: partial edges at page sizes 1/4/64; stable ties and survivor ordering; reserve eviction; full running batches and same-prompt reservation retries; source holds with partial tails; ready/running cancellation; disabled caching; session-separated store chains; physical-page reuse; both overlap modes and prefill/decode/idle event cadence.
  • Expected-value derivation: complete pages are the page-aligned materialized prefix. An 8-token prompt at page size 4 publishes two prompt blocks; a 10-token transferred prompt exposes eight reusable tokens and retains its private tail until release. Cadence assertions count observed forwards rather than launches.
  • Before/after output, trace, benchmark, or golden diff: focused tests assert exact cache visibility, event hashes, parent links, IDs and pass indices. The first-admission field's canonical digest updates were isolated by omitting only that new serialized field; existing report behavior was unchanged. No new native-engine or performance measurement is claimed.

Modeling or data provenance

Modified behavioral adaptations of SGLang v0.5.20, immutable commit 94602c9c2b7cbdb8efd5c52802dac6a1c180089e:

Synthetic fixtures only; source/license attribution is recorded in the source headers and matching root/package notices.

Tracking

Supersedes #410, #580, #581 and #582.

This pull request and its description were written by Isaac.

SGLang v0.5.20 decode caches a transferred prompt only when the request is
selected into a PREBUILT running batch, and only with decode radix caching
enabled (default off, which leaves a chunk cache that inserts nothing):

- https://github.com/sgl-project/sglang/blob/94602c9c2b7cbdb8efd5c52802dac6a1c180089e/python/sglang/srt/disaggregation/decode.py#L2707-L2750
- https://github.com/sgl-project/sglang/blob/94602c9c2b7cbdb8efd5c52802dac6a1c180089e/python/sglang/srt/disaggregation/decode_schedule_batch_mixin.py#L123
- https://github.com/sgl-project/sglang/blob/94602c9c2b7cbdb8efd5c52802dac6a1c180089e/python/sglang/srt/arg_groups/fields/disagg.py#L80-L83
- https://github.com/sgl-project/sglang/blob/94602c9c2b7cbdb8efd5c52802dac6a1c180089e/python/sglang/srt/arg_groups/pd_disaggregation_hook.py#L77-L111

A request aborted from the decode waiting queue is released through the
inserting release_kv_cache path:

- https://github.com/sgl-project/sglang/blob/94602c9c2b7cbdb8efd5c52802dac6a1c180089e/python/sglang/srt/managers/scheduler.py#L5246-L5248

Activation now keeps the transferred pages private and locks only the
matched prefix. Promotion into the running batch caches the prompt's full
pages and bumps the destination capacity generation, since the new prefix
can let a pending reservation fit. Cancelling a ready request caches its full
pages and frees the tail. Handoff conformance expects no SGLang destination
stores on activation.

handoff_prefill_to_reserved_decode_owns_kv_until_prebuilt_admission asserted
publication at activation, which SGLang does not do; it now asserts that the
prompt stays invisible while the running batch is full and is published once
at running admission.

Signed-off-by: Rudy Pei <yanrpei@gmail.com>
Co-authored-by: Isaac <no-reply@databricks.com>
@PeaBrane
PeaBrane requested review from a team as code owners October 24, 2026 00:38
@copy-pr-bot

copy-pr-bot Bot commented Oct 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 78e6e4a and fdaad18.

Files selected for processing: 5
  • THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/core.rs
  • crates/core/src/engine/scheduler/sglang/host_loop.rs
  • crates/core/src/engine/scheduler/sglang/tests.rs
  • python/aisimulate/THIRD_PARTY_NOTICES.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Recent review details
Additional context used
Code guidelines: 3
AGENTS.md — configured
REVIEW.md — configured
python/aisimulate/.claude/rules/repo-guide.md — auto-discovered
Path-based instructions: 6
Review scheduler state transitions and event ordering, including cancel, preempt, drain, retry, duplicate, empty, and terminal paths.

CodeRabbit configuration file

Files:

  • crates/core/src/engine/scheduler/sglang/host_loop.rs
  • crates/core/src/engine/scheduler/sglang/core.rs
  • crates/core/src/engine/scheduler/sglang/tests.rs
Verify upstream provenance, license text, collection and overlay hashes, local modification notices, and generated derivative records remain complete and internally consistent.

CodeRabbit configuration file

Files:

  • THIRD_PARTY_NOTICES.md
Read REVIEW.md before commenting.

CodeRabbit configuration file

Files:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/host_loop.rs
  • THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/core.rs
  • crates/core/src/engine/scheduler/sglang/tests.rs
Source excerpt: Keep run logs, investigation notes, and benchmark results out of `docs/`; measured evidence belongs under `benchmarks/evidence/`.

CodeRabbit inference engine (AGENTS.md)

Files:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/host_loop.rs
  • THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/core.rs
  • crates/core/src/engine/scheduler/sglang/tests.rs
Source excerpt: Binary parquet changes require a machine-readable summary of keys, shapes, units, row counts, coverage, and anomalies plus proof that a real consumer reaches the new data.

CodeRabbit inference engine (REVIEW.md)

Files:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/host_loop.rs
  • THIRD_PARTY_NOTICES.md
  • crates/core/src/engine/scheduler/sglang/core.rs
  • crates/core/src/engine/scheduler/sglang/tests.rs
Source excerpt: **A task stays in its module.**

CodeRabbit inference engine (python/aisimulate/.claude/rules/repo-guide.md)

Files:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
Multi-repo context ai-dynamo/dynamo, ai-dynamo/aiconfigurator

Linked repositories findings

ai-dynamo/dynamo

  • Dynamo distinguishes raw engine-event gaps from published-event continuity: raw gaps are measured and reported as dropped events, while the local indexer requires applied event IDs to be consecutive. [::ai-dynamo/dynamo::lib/llm/src/kv_router/publisher/event_processor.rs:52-77] [::ai-dynamo/dynamo::lib/kv-router/src/indexer/local.rs:80-95]
  • Therefore, the PR’s deferred event-ID allocation and batching must not create gaps in the emitted stream, even when raw engine events were dropped.
  • Disaggregated SGLang launchers publish KV events from two prefill and two decode workers, all using page size 64. [::ai-dynamo/dynamo::examples/backends/sglang/launch/disagg_router.sh:65-123]
  • Replay JSONL records use direct serde_json::to_string(record) serialization, so the new per-request field needs no separate Dynamo mapping. [::ai-dynamo/dynamo::lib/bindings/python/rust/llm/replay.rs:875-887]

ai-dynamo/aiconfigurator

  • The newest compatibility entry targets SGLang 0.5.16, while the PR adapts 0.5.20 behavior; the 1.2.0 contract targets 0.5.11. [::ai-dynamo/aiconfigurator::src/aiconfigurator/generator/config/backend_version_matrix.yaml:4-12]
  • Generated configuration maps tokens_per_block to --page-size and disable_prefix_cache to --disable-radix-cache. [::ai-dynamo/aiconfigurator::src/aiconfigurator/generator/config/backend_config_mapping.yaml:77-85]
  • Routed SGLang templates and contract tests require ZMQ --kv-events-config endpoints, so changed event timing is exercised by generated routed deployments. [::ai-dynamo/aiconfigurator::src/aiconfigurator/generator/config/backend_templates/sglang/run.sh.j2:43-61] [::ai-dynamo/aiconfigurator::tests/golden/generator/test_release_1_2_0_backend_contracts.py:371-391]
Additional comments: 6
python/aisimulate/THIRD_PARTY_NOTICES.md: 1

1534-1536: 📐 Maintainability & Code Quality

The root and Python notices are byte-identical, so no canonical-copy mismatch remains.

crates/core/src/engine/scheduler/sglang/core.rs: 2

770-777: LGTM!

1269-1269: LGTM!

Also applies to: 1313-1323

crates/core/src/engine/scheduler/sglang/host_loop.rs: 1

27-29: LGTM!

Also applies to: 64-65, 139-151

crates/core/src/engine/scheduler/sglang/tests.rs: 1

4065-4255: LGTM!

THIRD_PARTY_NOTICES.md: 1

1533-1536: LGTM!

📝 Summary

Risk: Medium. The three areas that need the most human attention are:

  1. SGLang lifecycle and event timing: Verify that transferred KV remains private until running admission, that prompts and eligible cancellation prefixes become reusable at the intended point, and that queued events flush after observed prefill, every 40 observed decode forwards, and while idle.
  2. Cache recency and decode retraction: Verify partial-edge recency updates, stable retraction ordering, and eviction toward the 20-token reserve per survivor.
  3. Public replay contract and backend behavior: Check that Rust consumers initialize PerRequestRecord.first_admission_reused_input_tokens. Confirm that the changed activation-storage behavior is SGLang-specific.

The change adjusts SGLang cache insertion, eviction, cancellation cleanup, and KV-router event publication. It adds first_admission_reused_input_tokens to the per-request record. Decode-generated prefixes remain private until request completion. The inspected source contains the 20-token reserve, the new report field, cancellation deferral, and event-flush call sites. The documentation states the event cadence and reuse-field meanings.

Evidence supplied: The author reports 2,466 passing core tests, with one ignored, 252 passing focused lifecycle/cache/handoff cases, and 16 passing public API tests. Nightly Rust formatting and packaged legal-file checks also passed.

Evidence still missing: Fast CI and full CI were not run separately. No native-engine or performance measurements are reported. Current review findings were not supplied, so review severity counts are unavailable. No reviewer approval is established.

Technical quality: The reported tests cover lifecycle, cache, handoff, and public API behavior. Merge readiness: The supplied evidence does not establish full CI status or reviewer approval.

The SGLang cache and scheduler now use updated prefix-caching, KV-event publication, cancellation, and decode-retraction behavior. Replay reports add a first-admission reuse field. Handoff conformance checks account for SGLang activation timing. Documentation and third-party notices describe these changes and their provenance.

Changes: SGLang cache and replay behavior

Layer / File(s) Summary
Refresh radix-cache recency on partial access
crates/core/src/engine/cache/radix_cache.rs, python/aisimulate/THIRD_PARTY_NOTICES.md
Partial matches and reinsertion without new pages refresh cache-path timestamps. Tests check eviction ordering across page sizes and matching paths.
Cache prefixes and publish KV events at defined boundaries
crates/core/src/engine/kv_manager/sglang_backend.rs, crates/core/src/engine/scheduler/sglang/core.rs, crates/core/src/engine/scheduler/sglang/host_loop.rs, crates/core/src/engine/scheduler/sglang/tests.rs, THIRD_PARTY_NOTICES.md, python/aisimulate/THIRD_PARTY_NOTICES.md
Transferred prompts are cached at running admission rather than activation. The scheduler caches materialized prefixes on source holds and cancellation. Running-request cancellation can defer cleanup until an in-flight result is observed. KV events are queued, coalesced, and assigned IDs when flushed at defined boundaries. Tests cover transfer, cancellation, retries, and event cadence.
Order decode retraction and defer decoded-prefix caching
crates/core/src/engine/scheduler/sglang/decode.rs, crates/core/src/engine/scheduler/sglang/tests.rs, THIRD_PARTY_NOTICES.md, python/aisimulate/THIRD_PARTY_NOTICES.md
Retraction uses output-length and prompt-length ordering, excludes terminal ghost rows from retraction, and evicts cache toward the survivor reserve. Prefill commits happen before decode-page reservation. Decoded output is cached only when a request finishes.
Record first-admission reuse and validate handoff behavior
crates/core/src/replay/report.rs, crates/core/src/replay/canonical.rs, crates/core/src/replay/handoff.rs, crates/core/src/replay/disagg_tests.rs, crates/core/tests/agentx_qualification.rs, crates/tests/public-api/src/lib.rs
Per-request reports add first_admission_reused_input_tokens, with serialization and public API coverage. SGLang handoff validation allows zero destination storage at activation, and a conformance test checks destination-first completion.
Document cache behavior and adaptation sources
docs/replay/engine/kv-cache.md, THIRD_PARTY_NOTICES.md, python/aisimulate/THIRD_PARTY_NOTICES.md
Documentation describes cache insertion, KV-event cadence, decode retraction, and replay reuse fields. The notices record SGLang v0.5.20 provenance and Apache-2.0 attribution.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • ai-dynamo/aisimulate#332: Changes destination reservation, admission headroom, and capacity-generation retries used by the handoff lifecycle updated here.
  • ai-dynamo/aisimulate#363: Changes the native SGLang radix-cache core used by the cache recency updates in this pull request.
  • ai-dynamo/aisimulate#581: Changes partial-edge recency and cached-prefix reinsertion in the same radix-cache adapter.
Merge Risk: Minimal · up to fdaad

This change aligns SGLang cache and KV-event behavior with the radix-cache lifecycle. Cancelled in-flight requests no longer expose unobserved KV to followers, and the author reports passing tests. No actionable merge-blocking risk remains in the supplied evidence.

Pre-merge checks: Passed 5 · Failed 2 · Inconclusive 1

Failed Failed checks: 2 warnings · 1 inconclusive

Check name Status Explanation Resolution
Cross-Layer Contract Warning The cross-layer contract remains incomplete. This PR adds and populates PerRequestRecord.first_admission_reused_input_tokens in crates/core/src/replay/report.rs:984-986,2289-2298 and covers it in … Update the downstream Dynamo per-request documentation to define first_admission_reused_input_tokens and its readmission semantics. Add a readmission case where first-admission reuse differs from reused_input_tokens. Assert the new fiel…
Modeling And Data Evidence Warning The PR still refreshes all 16 AgentX qualification digests without reproducible before/after evidence. The changed test hashes each report, per-request record set, and lifecycle JSONL into an opaque 1… Add reproducible before/after report, per-request, and lifecycle artifacts for all 16 qualification cases, or commit an explained machine-readable golden diff with anomaly checks for counts, statuses, and changed fields. Tie each changed va…
Review Evidence Inconclusive The visible description names commands and results, lists boundary cases, and states that Fast CI was not run locally and Full CI was not claimed. It also separates SGLang reference/parity scope from … Provide the complete pull-request description, or confirm that the omitted text contains no hosted-CI claim for nested workflows and preserves the stated local, simulator, parity, and production evidence distinctions.
Passed Passed checks: 5 passed
Check name Status Explanation
Title check Passed The title "fix: align SGLang cache lifecycle, eviction, and KV events" names the three main areas of the changeset: SGLang radix-cache lifecycle, eviction/retraction, and KV-event publication. It is s…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Compatibility Boundaries Passed Compatibility boundaries remain synchronized. PerRequestRecord adds the new field at the Rust public/JSON surface, populates it, updates the external public-API literal test, and documents its meani…
Description check Passed The pull request description follows the required template. It documents the motivation, behavior changes, review map, public contract impact, compatibility concerns, test evidence, review status, bou…
Full details: Title check

Explanation

The title "fix: align SGLang cache lifecycle, eviction, and KV events" names the three main areas of the changeset: SGLang radix-cache lifecycle, eviction/retraction, and KV-event publication. It is specific to the change, follows the conventional prefix, and is not vague. The title does not mention the new report field or the public API change, but those are secondary to the main change.

Full details: Compatibility Boundaries

Explanation

Compatibility boundaries remain synchronized. PerRequestRecord adds the new field at the Rust public/JSON surface, populates it, updates the external public-API literal test, and documents its meaning (crates/core/src/replay/report.rs:938-987,2289-2292; crates/tests/public-api/src/lib.rs:790-852; docs/replay/engine/kv-cache.md:98-108). The Python runner preserves native per-request records as JSON without a second strict model (python/aisimulate/src/aisimulate/runner.py:2265-2288). Replay input schema version and package versions are unchanged, and no defaults or bindings were omitted. The PR adds no Dynamo dependency or import. Dynamo consumes the records through serialization and has no PerRequestRecord struct literal at commit d043ae04174571aeac28dccb156f52d59c3f77fd (lib/bindings/python/rust/llm/replay.rs:875-891; lib/mocker/src/replay/entrypoints.rs:3049). The one-wheel/one-crate release checks remain unchanged, and the root and packaged notices are byte-identical. The release contract still permits exactly one aisimulate wheel and one aisimulate-core crate (scripts/release/build_release_artifacts.py:45-92,155-169).

Full details: Description check

Explanation

The pull request description follows the required template. It documents the motivation, behavior changes, review map, public contract impact, compatibility concerns, test evidence, review status, boundary cases, expected-value derivation, provenance, and tracking information. It also clearly states that Fast CI and Full CI were not run or claimed.

Full details: Cross-Layer Contract

Explanation

The cross-layer contract remains incomplete. This PR adds and populates PerRequestRecord.first_admission_reused_input_tokens in crates/core/src/replay/report.rs:984-986,2289-2298 and covers it in the Rust canonical/public-API tests. Generic Rust serialization converts each record with serde_json::to_value (crates/core/src/python.rs:3038-3048), and the CLI writes the resulting mappings to requests.jsonl (python/aisimulate/src/aisimulate/main.py:235-241). The PR also updates the AISimulate KV-cache documentation. However, the downstream Dynamo documentation at ai-dynamo/dynamo@d043ae04174571aeac28dccb156f52d59c3f77fd, docs/fern/pages/reference/components/dynosim-replay-cli-reference.mdx:326-327, still lists only the G1 and host first-admission fields. A repository-wide search of that downstream commit finds no first_admission_reused_input_tokens. Its consumer tests at components/src/dynamo/replay/tests/test_agentx_g2.py:200-366 and binding tests at lib/bindings/python/tests/replay/test_replay_host_offload.py:1-90 assert older fields only. No changed PR file updates those downstream documentation or consumer contracts.

Resolution

Update the downstream Dynamo per-request documentation to define first_admission_reused_input_tokens and its readmission semantics. Add a readmission case where first-admission reuse differs from reused_input_tokens. Assert the new field in the native-report, Python-binding, CLI, and requests.jsonl consumer tests. Retain the existing Rust public-API and canonical serialization assertions.

Full details: Modeling And Data Evidence

Explanation

The PR still refreshes all 16 AgentX qualification digests without reproducible before/after evidence. The changed test hashes each report, per-request record set, and lifecycle JSONL into an opaque 16-character BLAKE3 value (crates/core/tests/agentx_qualification.rs:637-653), and the diff only replaces the expected constants. No report artifacts, semantic golden diff, or machine-readable anomaly checks were added. The test comment requires semantic comparison before re-pinning (:657-659), but does not perform or record that comparison. SGLang source provenance and focused lifecycle assertions do not explain the AgentX qualification output changes. The new first_admission_reused_input_tokens field may affect serialized reports, but the PR provides no per-field before/after evidence for the 16 cases.

Resolution

Add reproducible before/after report, per-request, and lifecycle artifacts for all 16 qualification cases, or commit an explained machine-readable golden diff with anomaly checks for counts, statuses, and changed fields. Tie each changed value to the cache-lifecycle or report-schema change, then update the digest constants. Otherwise revert the digest refresh.

Full details: Review Evidence

Explanation

The visible description names commands and results, lists boundary cases, and states that Fast CI was not run locally and Full CI was not claimed. It also separates SGLang reference/parity scope from local AISimulate tests and disclaims native-engine and production evidence. However, the authored description is explicitly truncated. This check requires the complete description to verify that omitted text does not cite an inactive nested workflow as hosted CI evidence. Repository policy confirms that workflows under python/aisimulate/ are not hosted-CI evidence (REVIEW.md:138-140), but the omitted prose cannot be checked.

Resolution

Provide the complete pull-request description, or confirm that the omitted text contains no hosted-CI claim for nested workflows and preserves the stated local, simulator, parity, and production evidence distinctions.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Publish radix inserts at the prefill, completion, and transferred-request
lifecycle boundaries. Batch compatible KV events and flush them from the
observed forward in overlap mode. Align partial-edge recency and stable
retraction/reserve eviction, and expose total first-admission reuse.

Port the earlier event-cadence contribution onto current request leases and
page metadata. Preserve current forward materialization and timing behavior.

Signed-off-by: Rudy Pei <yanrpei@gmail.com>
Co-authored-by: Isaac <no-reply@databricks.com>

@tedzhouhk tedzhouhk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 78e6e4aa5d27723c9cb311a951b56a0a840b5685 against the pinned SGLang source.

Validation: core suite 2,466 passed / 1 ignored; public API consumer 16 passed; all 16 AgentX qualification digests match the base after removing only the added per-request field. One reproduced P2 concerning overlap cancellation is recorded inline. Fast CI is green; Full CI and native-engine/GPU parity were not established by this review.

Comment on lines +770 to +772
let capacity_improved = if was_running {
let releases_kv = request.kv_lease.is_active();
cleanup_completed_request(&mut request, &mut self.kv_manager, self.config.block_size);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Defer caching canceled in-flight requests until result observation

My main concern is that this immediately inserts an unobserved forward's KV when host_loop is enabled. With a 16-token prompt and a 20 ms prefill, the first pass returns at t=0 without observing the result; canceling there caches all 16 prompt tokens, and a same-prompt follower selected at t=0 reuses 15 tokens instead of 0. Both one-token and multi-token output cases fail on this head, pass after reverting only this cancellation hunk, and fail again when the hunk is restored.

The pinned SGLang abort_request only sets to_finish on a running request; process_batch_result_prefill releases/caches it after result observation. Could we preserve deferred cleanup for in-flight cancellation and add cache-visibility/follower-reuse assertions to the existing overlap cancellation test? Otherwise canceled requests expose future KV to admission and the router.

Running cancellation under the overlap host loop discards pending outputs,
but immediately finishing the lease exposes KV from an unobserved forward.
A follower selected before that forward ends can then reuse future KV.

Keep canceled running requests with the pending batch until observation,
then cache and release their leases without retiring reused request IDs.
Preserve immediate cleanup when no forward is pending. Cover first-token
and continuing requests, both cache modes and page sizes, router events,
idle decode cleanup, repeated cancellation, and request-ID reuse.

Signed-off-by: Rudy Pei <yanrpei@gmail.com>
Co-authored-by: Isaac <no-reply@databricks.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants