Skip to content

Fix/hicache sidecar ok pure mla - #34002

Open
Leoyzen wants to merge 2 commits into
sgl-project:mainfrom
Leoyzen:fix/hicache-sidecar-ok-pure-mla
Open

Leoyzen wants to merge 2 commits into
sgl-project:mainfrom
Leoyzen:fix/hicache-sidecar-ok-pure-mla

Conversation

@Leoyzen

@Leoyzen Leoyzen commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

When a pure-MLA model (e.g. DeepSeek-V4) runs with TP>1, follower TP ranks have no rank-sharded sidecars to back up — all pools are replicated and TP0 handles the writes. The else branch of _page_backup set sidecar_ok = bool(backup_transfers), which evaluated to False when backup_transfers was empty, yielding completed_tokens=0. This prevented L3 restore from ever triggering on follower ranks, causing NaN on DSpark speculative hits (issue #33656).

Modifications

python/sglang/srt/mem_cache/hybrid_cache/hybrid_cache_controller.py_page_backup() method, else branch (backup_skip=True, i.e. follower TP ranks):

Changed sidecar_ok = bool(backup_transfers) to sidecar_ok = True. When there are no rank-sharded sidecars, the follower rank's backup is vacuously successful — TP0 already persisted all replicated pools. The validation loop still runs for non-empty backup_transfers and can set sidecar_ok=False on real failures.

# Before (buggy):
sidecar_ok = bool(backup_transfers)  # empty → False → completed_tokens=0 → no restore → NaN

# After (fixed):
sidecar_ok = True  # vacuously successful; validation loop still runs and can set False

test/registered/unit/mem_cache/test_hybrid_cache_controller_page_backup.py — 3 new unit tests:

  1. test_pure_mla_follower_reports_success_with_no_sidecars — Bug regression: follower rank with empty backup_transferscompleted_tokens > 0 (fails pre-fix, passes post-fix)
  2. test_follower_sidecar_write_failure_reports_zero — Negative: non-empty backup_transfers with failed writes → completed_tokens = 0
  3. test_follower_sidecar_write_success_reports_full — Positive: non-empty backup_transfers with successful writes → completed_tokens = full

Registered as base-a-test-cpu (est_time=3s). All tests pass locally on CPU.

Accuracy Tests

N/A — this change does not affect model outputs. It fixes the HiCache L3 backup/restore coordination path for pure-MLA models (DSV4 etc.) under TP>1.

Speed Tests and Profiling

N/A — no inference speed impact. The fix eliminates spurious backup-failure reporting on follower ranks when all pools are already replicated.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

Related #33656


CI States

Latest PR Test (Base): ❌ Run #31193950878
Latest PR Test (Extra): ❌ Run #31193947131

On follower TP ranks with backup_skip=True, sidecar_ok was set to
bool(backup_transfers) which is False when no rank-sharded sidecar
pools exist (e.g. DSV4 where all pools are MLA-replicated). This
caused completed_tokens=0, signaling backup failure and preventing
restore from ever triggering on follower ranks — ultimately causing
NaN crashes on DSpark speculative hits (issue sgl-project#33656).

Change sidecar_ok default to True: when backup_transfers is empty
(nothing to write), the backup is vacuously successful because TP0
already persisted all replicated pools. The validation loop still
runs and can set sidecar_ok=False if any rank-sharded sidecar write
fails.
@Leoyzen
Leoyzen force-pushed the fix/hicache-sidecar-ok-pure-mla branch from 3b8cf77 to 7612376 Compare August 7, 2026 15:42
@1e4ves

1e4ves commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

completed_tokens tracks local backup work. Followers with no sidecars correctly report 0 but still ACK; restore uses a separate prefetch operation. So this cannot block restore or cause NaNs.

But in the merged #30393, dsv4 mtp and dsv4 dspark should be in the packed path. Only dsv4+eagle3 uses the DRAFT sidecar, and its backup_skip is also fixed by should_backup() . Can you retest it ? Maybe you need also flush mooncake, because of the packed 3 dspark layers. @Leoyzen

Leoyzen added a commit to Leoyzen/sglang that referenced this pull request Aug 8, 2026
…project#32035 sgl-project#33656 sgl-project#32183 sgl-project#33145)

Applied PRs (latest from GitHub):
  sgl-project#33288  Indexer logits OOM fix
  sgl-project#30393  HiCache packed/sidecar draft caches
  sgl-project#31170  DPA prefix_affinity load balancing
  sgl-project#33795  DSpark compact ragged-verify CUDA graph JIT race
  sgl-project#32467  C128 plan-kernel warp barrier
  sgl-project#33865  DSpark x prefill CP unblock
  sgl-project#30371  SWA state pool sizing (storage page)
  sgl-project#33358  FlashMLA norm-rope K-tokens-per-block ILP
  sgl-project#33872  num_draft_tokens clamp + extend_len==0 skip (supersede sgl-project#32183)
  sgl-project#34002  Sidecar backup vacuously-successful fix (replaces sgl-project#33656, with tests)
  sgl-project#33862  Reclaim redundant host mirrors after storage backup
  sgl-project#31315  Avoid repeated Mooncake gets after stale hits
  sgl-project#32327  Q8KV8 sparse MLA prefill backend (flashmla_sparse_q8)
  sgl-project#31668  Fix sidecar pool life-time (use-after-free on prefetch abort)
  sgl-project#31195  TP0 verify-token-budget broadcast (adapted to get_schedule() API)

Dropped (per user request or superseded):
  sgl-project#32771  IndexCache C4 top-k reuse — has bug
  sgl-project#32035  DSpark C128 online compressor — has bug
  sgl-project#33656  Superseded by sgl-project#34002 (same fix + unit tests)
  sgl-project#32183  Superseded by sgl-project#33872 (included in supersede PR)
  sgl-project#33145  Base f01f706 already has superior reasoning-effort profile system

Conflicts resolved:
  sgl-project#31195: adapted to base get_schedule().disable_overlap_schedule API
  sgl-project#32327: path remapped jit_kernel/ -> kernels/jit/ and kernels/ops/attention/
  sgl-project#31668: applied cleanly on top of sgl-project#30393+sgl-project#34002+sgl-project#33862 modifications
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