Repository navigation
[Bugfix] kv offload: fence pending stores before zeroing new pages - #73
Merged
Merged
Conversation
The worker zeroes the pages in new_block_ids_to_zero in the step they are allocated, before that step's store jobs run. The connector fenced pending stores only against pages that reach it through scheduled requests' new blocks and offloaded groups' load destinations. A request parked for an async CPU load is not scheduled, and the QSA compressor-ring group is not offloaded, so a ring page it claimed could still be the source of another request's pending store: the page was zeroed before the store read it and the CPU tier stored zeros under a valid prefix key. Flush every pending store that reads a page announced for zeroing. Fixes #69 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Leonccaa <166551845+Leonccaa@users.noreply.github.com>
Owner
Author
|
GPU validation: the same fence change is part of the 0.2.0rc3 acceptance run on 4x V100 (offload readback, stress and long-context checks passed) and now running in production. Marking ready. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft, based on main. Replaces #71, which was stacked on the partial prefix-cache port (#70); this fix does not depend on it. GPU validation is planned for the next validation window.
Fixes #69
Change
The worker zeroes the pages in
new_block_ids_to_zeroin the step they are allocated, after the connector has submitted that step's pending stores. The connector fenced pending stores only against pages that reach it through scheduled requests' new blocks and offloaded groups' load destinations. A request parked for an async CPU load is not scheduled, and the QSA compressor-ring group is not offloaded, so a ring page it claimed could still be read by another request's pending store. The page was zeroed first and the CPU tier stored zeros under a valid prefix key.build_connector_metanow flushes every pending store that reads a page innew_block_ids_to_zero. The extra flushes only happen when a page announced for zeroing still has a pending store.The issue also affects 0.1.5; the change applies there unchanged.
Validation
CPU, in the ReVolta build image:
New tests in
tests/v1/kv_connector/unit/offloading_connector/test_zeroing_fence.py(the file carries the minimal production cache layout it needs):test_zeroed_pages_are_fenced_against_pending_stores: a pending store on a page innew_block_ids_to_zerois flushed; other pages are not.test_parked_request_ring_page_is_fenced_before_zeroing: reproduces the issue with a realKVCacheManagerin the production group layout (DCP2, 1,664-token pages). Request A offloads a drafter boundary state and moves past the page; request C is admitted with an async CPU load and receives that page as its compressor-ring page. Without the changejobs_to_flushis empty; with it the store is flushed.tests/v1/core,tests/v1/kv_offloadandtests/v1/kv_connector/unit/offloading_connector: 823 passed, 253 failed, 26 errors, 6 skipped. The failures and errors are identical by node id tomain(CUDA-only and pre-existing); the only additional passes are the two new tests, and no test that passes onmainfails here.Not yet run on GPU.
Attribution
The defect was found in an independent review with an AI assistant; this change and its tests were written with an AI assistant (Claude Code). The submitting human reviews every changed line before merge. Commits are signed off under the DCO.
🤖 Generated with Claude Code