Repository navigation
Conversation
KV-aware routers build their view of each worker's cache from the BlockStored/BlockRemoved stream. During decode, publish_kv_events ran only after the decode_log_interval early return in report_decode_stats, so blocks stored when a request finished and blocks evicted by decode allocation reached the router up to 40 steps late. PD decode workers never run prefill batches, so they always saw this lag. Flush the queue before the periodic return, as prefill already does. Signed-off-by: Sohom Chakraborty <16609933+sohom-cs@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
BBuf
left a comment
BBuf
left a comment
There was a problem hiding this comment.
[BBuf Review Agent] LGTM. Please wait for the related CI.
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
…cadence Signed-off-by: Sohom Chakraborty <16609933+sohom-cs@users.noreply.github.com>
|
/rerun-failed-ci |
This comment has been minimized.
This comment has been minimized.
|
[BBuf Review Agent] @sohom-cs the required
|
…cadence Signed-off-by: Sohom Chakraborty <16609933+sohom-cs@users.noreply.github.com>
|
Merged the latest main, so CI now runs on a fresh merge commit. The lint failure came from main itself (a repeated |
|
[BBuf Review Agent] @sohom-cs Confirmed the duplicate |
Motivation
With
--kv-events-config, a KV-aware router (sgl-router, or Dynamo) builds its view of each worker's cache from the BlockStored / BlockRemoved stream. During decode, the scheduler publishes that stream only everydecode_log_intervalsteps (40 by default). Blocks inserted when a request finishes, and blocks evicted to make room for decode, reach the router up to 40 steps late, and a follow-up turn that arrives in that window is routed on a stale view.Prefill publishes after every batch, so mixed workers partly hide this. PD decode workers run only decode batches, so they always lag.
Modifications
report_decode_statscalledpublish_kv_events()after its periodic early return, so 39 of 40 decode steps skipped it. The call now runs before that return, so every decode step publishes, as every prefill batch already does. On ranks without a publisher it is a no-op, and a step with no events costs onetake_events()call.Accuracy Tests
New case in
test/registered/unit/observability/test_forward_pass_metrics.py: a realSchedulerKvEventsPublisherbehind the metrics reporter, one queued event, one decode step that is not a log step.AssertionError: Lists differ: [] != [[<object ...>]](nothing published)CPU only. I have not measured the effect on a live router.
Speed Tests and Profiling
Not applicable: one extra queue drain per decode step, on the publishing rank only.
Checklist
Related: #44516 (decode cache affinity reads these events).
Developed with AI assistance (Claude); every changed line was reviewed and the tests were run locally by me.
CI States
Latest PR Test (Base): 🚫 Run #41528985096
Latest PR Test (Extra): ❌ Run #41528984971
Latest PR Test (AMD ROCm 10): ❌ Run #41528984894
Tip
Base CI only covers the core cases, and its workload may not exercise your change. Use
/rerun-testto run the tests your change touches, or copy this prompt for an agent: