Conversation
…ive drafters The namespace hash did not cover two factors that change stored-block compatibility: - block_size: two deployments differing only in tokens-per-block derived the same namespace; only vLLM's block-hash grouping kept them from colliding, by accident rather than by construction. - speculative drafters with their own KV cache (MTP, EAGLE, draft model): the drafter adds registered KV layers, changing the block slot layout. MTP-on and MTP-off deployments of the same model shared a namespace, so a warm namespace kept reporting hits the other side could never serve (loud load failures since the seal-topology change). N-gram-style methods (ngram, ngram_gpu, suffix) have no drafter KV and keep sharing the namespace; unknown methods conservatively split. All existing namespaces change on upgrade. That is a one-time cold cache: the SSD index is in-memory, metaserver entries expire via TTL, and resident blocks age out through LRU. A golden-value test now pins the hash so future factor changes are explicit decisions. Refs #344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build_copy_descs took copy sizes from the instance's GPU layout and read host block segments with no size validation. A stored block from an incompatible-geometry source (shared namespace, SSD, RDMA remote) with the same slot count but smaller segments was an out-of-bounds read or write on the pinned pool — silent, since the pool's mapping is large. Each segment is now checked against the bytes the layout will copy, and a contiguous-layout copy additionally requires a single-segment host block: a multi-segment block stores V in segment 1, which one contiguous copy cannot serve even when segment 0's padding passes the size check. Legitimate save-path blocks always pass (host segments are allocated at padded_segment_bytes >= layout segment bytes). Refs #344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Context
Follow-up to the discussion in #344. Two of the three holes identified there are closed here; the engine-side seal-time fingerprint remains open for discussion in the issue.
Changes
1.
derive_namespace: coverblock_sizeand KV-bearing speculative draftersThe namespace hash missed two factors that change stored-block compatibility:
block_size— deployments differing only in tokens-per-block derived the same namespace. In practice vLLM's block-hash grouping kept them from ever matching, but that is protection by accident, not by construction.N-gram-style methods (
ngram,ngram_gpu,suffix) have no drafter KV and keep sharing the namespace. Unknown methods conservatively split — a needless split is a cold cache, a missed split is cross-contamination.Upgrade note: all existing namespaces change. This is a one-time cold cache: the SSD index is in-memory, metaserver entries expire via TTL, resident blocks age out through LRU. A golden-value test now pins the hash so future factor changes are explicit decisions instead of refactoring side effects.
2.
build_copy_descs: validate stored block segments before CUDA copyCopy sizes come from the instance's GPU layout, but the host block may come from a foreign source (shared namespace, SSD, RDMA remote) with different geometry. Previously there was zero size validation — an undersized stored segment meant an out-of-bounds read/write on the pinned pool, silent because the pool mapping is large.
Now every segment is checked against the bytes the layout will copy, and a contiguous-layout copy requires a single-segment host block (a multi-segment block stores V in segment 1, which one contiguous copy cannot serve even when segment 0's padding passes the size check). Legitimate save-path blocks always pass: host segments are allocated at
padded_segment_bytes >= layout segment bytes.Testing
cd python && uv run --extra test pytest— 174 passed (newtest_namespace.pycovers MTP/EAGLE/unknown split, ngram share, block_size split, golden pin)uv run --isolated --no-project --with pytest --with numpy --with 'requests>=2.26.0' pytest) — 174 passedcargo test -p pegaflow-core --no-default-features --features cuda-13— all greenRefs #344
🤖 Generated with Claude Code