fix(dcp): build the indexer query-split topology for a single shard - #244
fix(dcp): build the indexer query-split topology for a single shard#244koush wants to merge 1 commit into
Conversation
VLLM_DCP_INDEXER_SHARDS=1 replicates the sparse-indexer K cache on every DCP rank, but the indexer topology was only built for 1 < shards < DCP, so one-shard runs fell back to _QUERY_SPLIT, which is singleton at TP == DCP. Every rank then scored every query row against the full indexer cache during prefill: DCP-wide redundant compute (~8x at DCP8, measurable as a long-context prefill throughput loss). With VLLM_DCP_QUERY_SPLIT=1 it was worse than redundant, since get_indexer_query_split_group(1) had no group to return and raised "No indexer query-split group matches the requested KV shard count". _build_indexer_replica_group_ranks already yields the correct layout for shards=1 -- single-rank owner groups and one TP-wide query-split group -- so let it run. Each rank scores totalQ/TP rows against the full replicated cache, and the existing indices-only all-gather restores the shared top-k buffer. No merge is needed or attempted: dcp_world_size stays 1 for a replicated cache, so every candidate-merge path still early-returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J4BQPVrK6LdTUbXudyPkga
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe indexer replica-group initialization condition now supports ChangesIndexer DCP initialization
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Superseded by #245. The original functional commit is preserved unchanged; the replacement adds focused regression coverage for the |
VLLM_DCP_INDEXER_SHARDS=1 replicates the sparse-indexer K cache on every DCP rank, but the indexer topology was only built for 1 < shards < DCP, so one-shard runs fell back to _QUERY_SPLIT, which is singleton at TP == DCP. Every rank then scored every query row against the full indexer cache during prefill: DCP-wide redundant compute (~8x at DCP8, measurable as a long-context prefill throughput loss). With VLLM_DCP_QUERY_SPLIT=1 it was worse than redundant, since get_indexer_query_split_group(1) had no group to return and raised "No indexer query-split group matches the requested KV shard count".
_build_indexer_replica_group_ranks already yields the correct layout for shards=1 -- single-rank owner groups and one TP-wide query-split group -- so let it run. Each rank scores totalQ/TP rows against the full replicated cache, and the existing indices-only all-gather restores the shared top-k buffer. No merge is needed or attempted: dcp_world_size stays 1 for a replicated cache, so every candidate-merge path still early-returns.
Claude-Session: https://claude.ai/code/session_01J4BQPVrK6LdTUbXudyPkga
PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.
Purpose
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)
Summary by CodeRabbit