Skip to content

fix(hicache): handle finite prefetch I/O failures safely - #42149

Open
mks-hash wants to merge 1 commit into
sgl-project:mainfrom
mks-hash:fix/hicache-prefetch-failure-cleanup
Open

mks-hash wants to merge 1 commit into
sgl-project:mainfrom
mks-hash:fix/hicache-prefetch-failure-cleanup

Conversation

@mks-hash

@mks-hash mks-hash commented Oct 1, 2026 •

Copy link
Copy Markdown

Motivation

Current main (7a719e9a65e7f52b012ab37211b5f174e5d3d38d) now catches storage query/read batch exceptions in unified-memory mode. Two lifecycle gaps remain: a duplicate terminal ACK can reclaim the same tail twice, and an exception after acknowledged batch progress can be published as a partial successful restore instead of an explicit terminal failure.

With the unified-memory flag enabled, the ten regression cases reproduce 2 failures / 8 passes on this base: allocator Double-free detected on duplicate completion, and partial publication after the second batch raises. The latter is a terminal-outcome/cleanup contract difference, not a claim that all shared published KV is leaked. Earlier worker-death reproducers on the old base are retained as regression coverage, not presented as current unified-mode behavior.

Modifications

  • Preserve upstream's terminal-ACK finally and query/read fallback for unsupported consumers.
  • For supported single-worker resident KV-only operations, route finite exceptions to explicit terminal failure and keep the worker alive.
  • Consume terminal ACKs once per operation, preserving cancellation and replacement identity.
  • Use upstream's new _retire_ongoing_prefetch cleanup helper; do not reintroduce the old inline reclamation.
  • Keep ten real-cache/file-backend regressions. Their config now explicitly enables unified memory so upstream's new exception paths are exercised.

Scope remains resident FULL, synchronous finite I/O and no rank collectives/PP tickets. No distributed exception recovery, permanently blocked-I/O reclamation, scheduler redesign, retries or proactive-prefetch feature is added. Production diff is 74 additions / 3 deletions in three files; the fourth file contains regression tests.

Accuracy Tests

Refreshed against the base above:

  • Current-main baseline with unified memory enabled: 2 failed / 8 passed; patched lifecycle suite: 10 passed, zero skips.
  • Relevant existing CPU selection: 128 passed / 39 CUDA-fixture skips, 43 subtests. Skips are not passes.
  • Additional physical-transfer/dispatch/host-assembler selection: 64 passed / 2 CUDA-only skips, 20 subtests.
  • Separate existing two-rank Gloo lifecycle: 1 passed. This checks the existing lifecycle for regressions; distributed exception recovery remains out of scope.
  • Applicable changed-file pre-commit hooks and git diff --check passed. Rust hooks had no matching files; no Rust validation is claimed.

CPU fixtures use real pools/controller/file I/O, unpinned host tensors and fixture-only host admission-budget accommodations on this constrained machine. The page-envelope accommodation was external to this branch, applies only to tiny allocations, and does not alter production runtime.

No new GPU H2D/model-inference run was performed against this refreshed SHA. Previous GPU evidence remains tied to its recorded source versions.

Speed Tests and Profiling

N/A: lifecycle correctness contribution; no performance claim.

Checklist

  • Applicable changed-file formatting/lint/registry hooks passed.
  • Real-cache regression coverage and relevant existing tests run.
  • Scope and CPU/GPU validation limits stated explicitly.

CI States

Latest PR Test (Base): ❌ Run #37224418283
Latest PR Test (Extra): ❌ Run #37224417998
Latest PR Test (AMD ROCm 10): ❌ Run #37224418252

@mks-hash
mks-hash force-pushed the fix/hicache-prefetch-failure-cleanup branch from 37d07d6 to c109823 Compare October 4, 2026 18:25

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

hicache Hierarchical Caching for SGLang unified-radix-cache

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant