Skip to content

[MemCache] test: require every ComponentData field to be classified by each split hook - #42518

Merged
ispobock merged 1 commit into
sgl-project:mainfrom
alphabetc1:test/component-split-field-coverage
Oct 7, 2026
Merged

ispobock merged 1 commit into
sgl-project:mainfrom
alphabetc1:test/component-split-field-coverage

Conversation

@alphabetc1

@alphabetc1 alphabetc1 commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

redistribute_on_node_split is hand-written once per component, and the recurring bug is a field nobody remembered. SWA dropped host_lock_ref on a split (#38138), and FULL dropped the same field again until #38480 — in both cases a split moved half of a pinned host slice onto a new parent with host_lock_ref == 0, so the next duplicate reclaim freed host pages an in-flight L3 write was still reading, and the object that landed in L3 mixed two nodes' KV.

Nothing made that omission visible: sanity_check derives protected sizes from the refcounts, so a wrong refcount stays self-consistent. The three hooks drift independently, and ComponentData has seven fields today.

Modifications

A test that makes the decision explicit and machine-checked. SPLIT_POLICY classifies every ComponentData field for every component in COMPONENT_REGISTRY as copy / divide / reset (plus the metadata boundary uuids, which migrate), and three checks enforce it:

  • the policy table and dataclasses.fields(ComponentData) must agree, and the table must cover every registered component — adding a field or a component without deciding what a split does with it fails here;
  • the real hook is run over a parent/child pair whose every field is set to a distinguishable value, and each field is verified against its policy;
  • the segment-boundary uuids must move to the parent and must not stay on the child.

Test-only, no product change. The hooks are driven directly with a mock tree core, so it is a CPU test (base-a-test-cpu) that needs no fixture and no GPU.

Not covered: the Rust core's split. It models per-(component × tier) state as a two-field ValueState { value, lock_ref } — the host lock is the host slot's lock_ref — so the "N parallel fields, each hook remembers a subset" shape this guards against is specific to the Python ComponentData.

Accuracy Tests

python -m pytest test/registered/unit/mem_cache/test_component_split_contract.py, with CUDA_VISIBLE_DEVICES="" to confirm it needs no GPU: 3 passed, 6 subtests passed, 8s.

Three negative controls, each reverted afterwards and the suite re-run green:

  • drop new_parent.component_data[ct].host_lock_ref = child_cd.host_lock_ref from FULL (i.e. revert [HiCache]: Fix host lock ownership across radix splits #38480's core line) → AssertionError: 0 != 2 : FULL.host_lock_ref must reach the parent;
  • add a field to ComponentData → 'brand_new_ownership_field' : FULL: ComponentData fields and the split policy disagree;
  • drop SWA's metadata.pop("host_uuid") → 'host_uuid' unexpectedly found ... : SWA: host_uuid must not stay on the child too.

Checklist

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ❌ Run #37396584391
Latest PR Test (Extra): ❌ Run #37396583798
Latest PR Test (AMD ROCm 10): ❌ Run #37396585256

@alphabetc1 alphabetc1 added the run-ci CI: run the baseline test suite on this PR label Oct 4, 2026
…t hook

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alphabetc1
alphabetc1 force-pushed the test/component-split-field-coverage branch from 628c3d1 to 27f3759 Compare October 6, 2026 00:56
@alphabetc1

Copy link
Copy Markdown
Collaborator Author

/rerun-test -c

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

🚀 ubuntu-latest (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/mem_cache/test_component_split_contract.py

@ispobock
ispobock merged commit 2889158 into sgl-project:main Oct 7, 2026
158 of 181 checks passed
@alphabetc1
alphabetc1 deleted the test/component-split-field-coverage branch October 7, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants