From 1d168d061f3f5df0f4f2a3284e4a873ae0e14350 Mon Sep 17 00:00:00 2001 From: laoyao0822 Date: Sat, 13 Jun 2026 03:44:23 +0800 Subject: [PATCH] Preserve pending CP HiCache backups through final insert Deferred non-chunked inserts can happen while a CP HiCache per-layer backup is still in flight. Rolling back the unattached prepared backup at unfinished-request time drops the only reservation that final insertion can attach, so cache_finished_req has to start a post-forward backup and catch up all layers synchronously.\n\nKeep the prepared backup for non-chunked deferred inserts and continue rolling it back for chunked middle inserts, where the suffix may be extended by later chunks and the old backup would cover the wrong range. Document the failure mode so future changes do not rediscover the same fallback path.\n\nConstraint: CP HiCache radix split cannot mutate in-flight backup nodes.\nConstraint: Chunked prefill middle inserts still need rollback because their backup range is not final.\nRejected: Always rollback unattached backups | causes post-forward catch_up_all_layers for non-chunked deferred inserts.\nRejected: Always preserve unattached backups | can attach stale backup ranges for chunked middle inserts.\nConfidence: high\nScope-risk: narrow\nDirective: Do not clear req.cp_hicache_prepared_backup on non-chunked deferred insert without proving final insert no longer needs it.\nTested: remote cjy-glm5-new py_compile radix_cache.py\nTested: remote cjy-glm5-new pytest test_cp_hicache_metadata.py::{nonchunked preserve,chunked rollback} => 2 passed\nNot-tested: live ETE replay after restarting prefill with this exact commit --- ..._prefill_cp_page_aligned_cache_contract.md | 45 +++++++++++++++++ python/sglang/srt/mem_cache/radix_cache.py | 8 +++ .../mem_cache/test_cp_hicache_metadata.py | 50 ++++++++++++++++++- 3 files changed, 102 insertions(+), 1 deletion(-) diff --git a/docs/advanced_features/nsa_prefill_cp_page_aligned_cache_contract.md b/docs/advanced_features/nsa_prefill_cp_page_aligned_cache_contract.md index d7bacded2..620fdd8aa 100644 --- a/docs/advanced_features/nsa_prefill_cp_page_aligned_cache_contract.md +++ b/docs/advanced_features/nsa_prefill_cp_page_aligned_cache_contract.md @@ -5489,3 +5489,48 @@ Validation: - Remote targeted regression set passed (`4 passed`): disabled sync materialize, consume miss warning, first-layer no-warning, and create-skip warning when enabled. + +### C128 — 2026-06-13 Non-chunked deferred insert must preserve prepared CP backup + +Finding: + +- Remote log `sglang_cp_hicache_20260612_183417.log` still showed + `[CP_HICACHE_FALLBACK][post_forward_catch_up_backup]` followed by + `[CP_HICACHE_FALLBACK][catch_up_all_layers]` for a small set of nodes. +- The fallback always followed `insert_deferred_pending_backup_split`, usually + with `reason=node_pending_backup` and non-empty `ongoing/pending` write state. +- This means the request had already completed forward and tried to update the + radix tree, but the insert could not safely split a node whose CP HiCache + backup was still in flight. +- `cache_unfinished_req()` treated every deferred insert the same way: it rolled + back an unattached `cp_hicache_prepared_backup` and cleared it from the request. +- For disaggregated non-chunked prefill this rollback is too early. The request + is still kept in the inflight transfer queue and later reaches + `cache_finished_req()` when the transfer completes. If the prepared backup is + preserved, that final insert can attach the already-registered per-layer + backup. If it is cleared, the final insert has no prepared reservation and + immediately falls back to post-forward all-layer catch-up backup. + +Correction: + +- Preserve unattached prepared CP HiCache backup across deferred + `cache_unfinished_req(chunked=False)` so the later final insert can attach it. +- Keep the previous rollback behavior for `chunked=True` middle chunks. A + chunked request may execute another forward for the same request before final + insert; retaining an old prepared reservation would cover the wrong suffix and + block the next chunk from preparing its own backup. + +Validation: + +- Remote RED/GREEN targeted tests in `cjy-glm5-new`: + - `test_cache_unfinished_req_deferred_insert_preserves_unattached_backup_for_final_insert` + - `test_cache_unfinished_chunked_deferred_insert_rolls_back_unattached_backup` +- Remote full metadata suite passed: + `PYTHONPATH=python python -m pytest -q test/registered/unit/mem_cache/test_cp_hicache_metadata.py` + => `127 passed, 21 warnings`. + +Remaining validation gap: + +- Needs a fresh ETE/replay run after restart to confirm the runtime + `CP_HICACHE_FALLBACK][catch_up_all_layers]` events disappear or reduce to only + genuinely unsupported cases. diff --git a/python/sglang/srt/mem_cache/radix_cache.py b/python/sglang/srt/mem_cache/radix_cache.py index ca6cf0419..b4598aad9 100644 --- a/python/sglang/srt/mem_cache/radix_cache.py +++ b/python/sglang/srt/mem_cache/radix_cache.py @@ -725,7 +725,15 @@ class RadixCache(BasePrefixCache): prepared_cp_backup is not None and not getattr(prepared_cp_backup, "attached", False) and hasattr(self, "_rollback_prepared_cp_backup") + and chunked ): + # A chunked middle insert can be followed by another forward for + # the same request, so an unattached prepared reservation would + # cover the wrong suffix and block the next chunk from preparing + # its own backup. Non-chunked disagg prefill still has a later + # cache_finished_req() pass after KV transfer; keep the prepared + # backup alive so that final insert can attach it instead of + # falling back to post-forward all-layer catch-up backup. self._rollback_prepared_cp_backup( prepared_cp_backup, "unfinished_pending_backup_split_deferred" ) diff --git a/test/registered/unit/mem_cache/test_cp_hicache_metadata.py b/test/registered/unit/mem_cache/test_cp_hicache_metadata.py index 12a80e154..6e7e62450 100644 --- a/test/registered/unit/mem_cache/test_cp_hicache_metadata.py +++ b/test/registered/unit/mem_cache/test_cp_hicache_metadata.py @@ -2019,7 +2019,9 @@ class TestHiRadixCacheCPBackup(CustomTestCase): self.assertEqual(req.prefix_indices.tolist(), list(range(6))) self.assertEqual(cache.token_to_kv_pool_allocator.freed, []) - def test_cache_unfinished_req_deferred_insert_rolls_back_unattached_backup(self): + def test_cache_unfinished_req_deferred_insert_preserves_unattached_backup_for_final_insert( + self, + ): cache = HiRadixCache.__new__(HiRadixCache) cache.disable = False cache._uses_cp_hicache = True @@ -2059,6 +2061,52 @@ class TestHiRadixCacheCPBackup(CustomTestCase): cache.cache_unfinished_req(req, chunked=False) + self.assertEqual(rollbacks, []) + self.assertIs(req.cp_hicache_prepared_backup, prepared) + self.assertEqual(req.prefix_indices.tolist(), list(range(6))) + + def test_cache_unfinished_chunked_deferred_insert_rolls_back_unattached_backup( + self, + ): + cache = HiRadixCache.__new__(HiRadixCache) + cache.disable = False + cache._uses_cp_hicache = True + cache.is_eagle = False + cache.page_size = 4 + deferred_node = TreeNode() + rollbacks = [] + + class Pool: + req_to_token = torch.arange(8, dtype=torch.int64).view(1, 8) + + cache.req_to_token_pool = Pool() + cache.token_to_kv_pool_allocator = RecordingTokenAllocator() + cache.insert = lambda params: types.SimpleNamespace( + prefix_len=1, + pending_backup_deferred_node=deferred_node, + ) + cache.match_prefix = lambda params: (_ for _ in ()).throw( + AssertionError("deferred insert must not rematch") + ) + cache._free_kv_indices_range = lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("deferred unfinished insert must keep request KV") + ) + cache._rollback_prepared_cp_backup = lambda prepared, reason: rollbacks.append( + (prepared, reason) + ) + prepared = types.SimpleNamespace(attached=False) + req = types.SimpleNamespace( + fill_ids=list(range(6)), + req_pool_idx=0, + extra_key=None, + cache_protected_len=1, + last_node=TreeNode(), + prefix_indices=torch.empty((0,), dtype=torch.int64), + cp_hicache_prepared_backup=prepared, + ) + + cache.cache_unfinished_req(req, chunked=True) + self.assertEqual( rollbacks, [(prepared, "unfinished_pending_backup_split_deferred")] )