CP HiCache: fix L3-reload partial-prefix node split crash (well-form the node)
A radix split crashed: split_committed_object got split_pages=172 (from the node
KEY length) for a target_kv object of num_pages=1 (the host backup), raising
"split page count must leave at least one page on each side". Root cause: the L3
reload inserts a MALFORMED node -- _cp_l3_reserve_reloads stores the FULL n_full-
page suffix key but only the C L3-durable pages are reserved/hashed/host-backed
(C < n_full when L3 holds a partial durable prefix via page-level GC or partial
spill). The node then has len(key)//page = n_full but host_len//page =
object.num_pages = len(hash_value) = C, violating the radix invariant every
consumer assumes; the split derives split_pages from the long key and overruns the
C-page object. Pre-existing (L3 3.2, 864b1c808e); NOT the multi-slab change
(_allocate_contiguous raises on no-fit, never partial, so multi-slab cannot produce
a partial-backed node -- it crashes single-slab too).
Elegant fix -- restore the invariant at its single source: slice the reloaded node
to its C durable pages so len(key)//page == host_len//page == object.num_pages ==
len(hash_value) == C. The un-durable suffix [C:n_full] is a correct cache miss.
This makes _split_node / CpSharedL2NodeMetadata.split / split_committed_object / the
L3 page-set builders correct unchanged, and removes the latent negative-host_len and
value/hash mis-slice the same malformed node would have triggered.
- hiradix_cache.py _cp_l3_reserve_reloads: slice suffix_key to C pages (matching the
already-sliced suffix_hashes).
- hiradix_cache.py _cp_l3_build_owned_pages (spill) + _cp_l3_build_reload_owned_pages
(reload): fail loud if the page-hash count exceeds the object's pages (else a future
desync silently corrupts a neighbour object in the same slab -- the slab accessor
only catches a whole-slab overrun).
- hiradix_cache.py _split_node: assert split_len <= host_len (fail loud on any future
malformed partial-backed node instead of the cryptic split_pages>=num_pages).
- test_cp_shared_l2_pool.py: lock the split contract (split_committed_object rejects
split_pages outside (0,num_pages); metadata.split rejects object/padded mismatch).
Design + 3-agent investigation + 2-agent review (no bugs):
docs_internal/cp_hicache_l3_reload_partial_prefix_fix_design.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1559,8 +1559,15 @@ class HiRadixCache(RadixCache):
|
||||
continue # could not fit even after evict-to-fit -> recompute
|
||||
owned_pages = self._cp_l3_build_reload_owned_pages(ranges, shashes[:c], store)
|
||||
op_id = store.submit_reload(reload_key, owned_pages)
|
||||
# Slice the candidate key to the C L3-durable pages (matching suffix_hashes[:c] and the C-page
|
||||
# reservation) so the reloaded node is WELL-FORMED:
|
||||
# len(key)//page == host_len//page == object.num_pages == len(hash_value) == C.
|
||||
# The un-durable suffix [C:n_full] is simply not present (a correct cache miss / recompute);
|
||||
# carrying the full key while only C pages are backed would make the node claim a prefix it
|
||||
# cannot serve and crash the radix split (split_pages from the long key vs the C-page object).
|
||||
self.ongoing_l3_reload[op_id] = {
|
||||
"reload_key": reload_key, "last_host_node": lhn, "suffix_key": skey,
|
||||
"reload_key": reload_key, "last_host_node": lhn,
|
||||
"suffix_key": skey[: c * self.page_size],
|
||||
"suffix_hashes": list(shashes[:c]), "n_pages": c, "waiters": [rid],
|
||||
"ttl": self.cp_l3_reload_ttl_ticks,
|
||||
}
|
||||
@@ -1622,6 +1629,15 @@ class HiRadixCache(RadixCache):
|
||||
n = len(page_hashes)
|
||||
owned = {}
|
||||
for pk, rng in ranges.items():
|
||||
if n > rng.num_pages:
|
||||
# Symmetric with the spill builder _cp_l3_build_owned_pages: base + i
|
||||
# past the object would address the neighbour object's pages in the same
|
||||
# slab. Safe by construction (n == c == ranges), fail loud on any desync.
|
||||
raise RuntimeError(
|
||||
f"[CP_L3_FAILFAST] reload page count {n} exceeds object pages "
|
||||
f"{rng.num_pages} for payload {pk!r} (node key/host_len desync); "
|
||||
f"base_page={rng.base_page} slab_id={rng.slab_id}"
|
||||
)
|
||||
base = rng.base_page
|
||||
owned[pk] = [(base + i, page_hashes[i]) for i in range(n) if i % cp_size == cp_rank]
|
||||
return owned
|
||||
@@ -1805,6 +1821,16 @@ class HiRadixCache(RadixCache):
|
||||
rng = meta.object_ranges.get(pk)
|
||||
if rng is None:
|
||||
continue
|
||||
if n > rng.num_pages:
|
||||
# base + i for i in range(n) would address PAST this object into the
|
||||
# neighbour object's pages in the same slab -> silent L3 corruption
|
||||
# (wrong content under a valid hash). After the reload-key slice this
|
||||
# cannot happen; fail loud on any future key/host_len desync.
|
||||
raise RuntimeError(
|
||||
f"[CP_L3_FAILFAST] spill page count {n} exceeds object pages "
|
||||
f"{rng.num_pages} for payload {pk!r} (node key/host_len desync); "
|
||||
f"base_page={rng.base_page} slab_id={rng.slab_id}"
|
||||
)
|
||||
base = rng.base_page
|
||||
owned[pk] = [
|
||||
(base + i, page_keys[i]) for i in range(n) if i % cp_size == cp_rank
|
||||
@@ -6045,6 +6071,18 @@ class HiRadixCache(RadixCache):
|
||||
if self._uses_cp_hicache:
|
||||
if self._node_backuped(child):
|
||||
_old_cp_meta = child.cp_hicache
|
||||
# Invariant: a backed node's host backup spans its full key, so the
|
||||
# radix split point never exceeds host_len. Fail loud if a malformed
|
||||
# partial-backed node (key longer than its host backup) ever reaches
|
||||
# here -- it would otherwise feed split_pages>num_pages to the L2 split
|
||||
# and drive child.host_len negative.
|
||||
if split_len > child.host_len:
|
||||
raise RuntimeError(
|
||||
"[CP_HICACHE_FAILFAST][split_exceeds_host_backup] radix split at "
|
||||
f"split_len={split_len} exceeds the node's backed host_len="
|
||||
f"{child.host_len} (a partial-backed node must not exist); "
|
||||
f"node_id={getattr(child, 'id', None)}"
|
||||
)
|
||||
# Leave committed for the pre-split metadata BEFORE reassigning
|
||||
# child.cp_hicache, so a first-touch lazy rebuild of the running
|
||||
# counts still sees the intact, in-tree child as committed.
|
||||
|
||||
@@ -656,6 +656,32 @@ class TestCpSharedL2Metadata(unittest.TestCase):
|
||||
with self.assertRaisesRegex(ValueError, "multiple of page_size"):
|
||||
metadata.split(32)
|
||||
|
||||
def test_shared_metadata_split_rejects_object_range_pages_mismatch(self):
|
||||
# The malformed L3-reloaded-node state the reload-key slice prevents:
|
||||
# padded_len claims 2 pages but the backing object holds only 1 (the node's
|
||||
# key was longer than its host backup). split() must fail loud rather than
|
||||
# mis-partition the object.
|
||||
metadata = CpSharedL2NodeMetadata(
|
||||
logical_len=128,
|
||||
padded_len=128, # 2 pages
|
||||
page_size=64,
|
||||
object_ranges={
|
||||
PAYLOAD_TARGET_KV: CpSharedL2ObjectRange(
|
||||
object_key="malformed",
|
||||
payload_kind=PAYLOAD_TARGET_KV,
|
||||
slab_id=0,
|
||||
base_page=0,
|
||||
num_pages=1, # only 1 page backed -> mismatch
|
||||
generation=1,
|
||||
)
|
||||
},
|
||||
required_payloads=(PAYLOAD_TARGET_KV,),
|
||||
object_key="malformed",
|
||||
)
|
||||
|
||||
with self.assertRaisesRegex(ValueError, "page count must match"):
|
||||
metadata.split(64)
|
||||
|
||||
|
||||
class TestCpSharedHostSlabPrimitives(unittest.TestCase):
|
||||
def test_create_attach_visibility_and_lifecycle_owner_unlinks_only_on_creator_close(self):
|
||||
@@ -1102,6 +1128,29 @@ class TestCpSharedL2PageAllocator(unittest.TestCase):
|
||||
self.assertTrue(allocator.release("node-12"))
|
||||
self.assertEqual(allocator.free_pages(PAYLOAD_TARGET_KV), 8)
|
||||
|
||||
def test_split_committed_object_rejects_split_pages_outside_open_interval(self):
|
||||
# The CP HiCache prefill crash: a partially-host-backed L3-reloaded node
|
||||
# (object num_pages=C) was split at a KEY-derived split_pages >= C. The
|
||||
# allocator must reject any split_pages not in (0, num_pages) loudly and
|
||||
# leave the object untouched (the page-count invariant the reload-key
|
||||
# slice restores). C=1 here mirrors the reported split_pages=172 vs
|
||||
# range_pages=1.
|
||||
allocator = self.make_allocator(pages=8)
|
||||
allocator.reserve("obj", PAYLOAD_TARGET_KV, 1) # C = 1 page
|
||||
allocator.mark_object_committed("obj")
|
||||
for bad in (0, 1, 2, 172): # 0 and >=num_pages(1) are all invalid
|
||||
with self.assertRaises(ValueError):
|
||||
allocator.split_committed_object(
|
||||
"obj",
|
||||
split_pages_by_payload={PAYLOAD_TARGET_KV: bad},
|
||||
parent_object_key="obj-parent",
|
||||
child_object_key="obj",
|
||||
)
|
||||
# object untouched after every rejected split (validated before mutation)
|
||||
self.assertTrue(allocator.is_committed("obj"))
|
||||
self.assertEqual(allocator.object_ranges("obj")[PAYLOAD_TARGET_KV].num_pages, 1)
|
||||
self.assertEqual(allocator.object_ranges("obj-parent"), {})
|
||||
|
||||
def test_duplicate_live_object_payload_reservations_are_rejected(self):
|
||||
allocator = self.make_allocator(pages=5)
|
||||
allocator.reserve("dup", PAYLOAD_TARGET_KV, 1)
|
||||
|
||||
Reference in New Issue
Block a user