Skip to content

fix(vllm): floor max cached blocks at 1 for sub-block-size budgets (#343)#380

Open
SuperMarioYL wants to merge 1 commit into
ovg-project:mainfrom
SuperMarioYL:fix/max-cached-tokens-sub-block-floor
Open

fix(vllm): floor max cached blocks at 1 for sub-block-size budgets (#343)#380
SuperMarioYL wants to merge 1 commit into
ovg-project:mainfrom
SuperMarioYL:fix/max-cached-tokens-sub-block-floor

Conversation

@SuperMarioYL

Copy link
Copy Markdown

Summary

Fixes #343.

_get_max_cached_blocks (kvcached/integration/vllm/patches.py) converts the
unified KVCACHED_MAX_CACHED_TOKENS config into a block budget with:

return MAX_CACHED_TOKENS // block_size

When KVCACHED_MAX_CACHED_TOKENS is a small positive value below one KV
block (e.g. 8 with a 16-token block_size), integer division floors to 0.
That is the exact value the == 0 "disabled — evict on free" sentinel
produces, so a user who asked for a small non-zero cache gets prefix caching
silently turned off instead of a small cap.

Fix

Handle the sentinels explicitly and floor the positive branch at one block:

if MAX_CACHED_TOKENS < 0:
    return -1          # unlimited (unchanged)
if MAX_CACHED_TOKENS == 0:
    return 0           # disabled  (unchanged)
max_cached_blocks = MAX_CACHED_TOKENS // block_size
if max_cached_blocks == 0:
    logger.warning(...)  # no longer silent
    return 1
return max_cached_blocks
  • A positive-but-sub-block budget now keeps prefix caching enabled at the
    smallest viable unit (1 block), which the consumer at
    ElasticBlockPool (max_cached_blocks >= 0 and len(evictable) > max_cached_blocks)
    treats as a valid cap of one evictable block.
  • The clamp is no longer silent — a warning reports the effective
    granularity and points at KVCACHED_MAX_CACHED_TOKENS=0 for explicit disable.
  • < 0 (unlimited), == 0 (disabled), and >= block_size budgets are
    byte-for-byte unchanged; the only behavioral change is the previously
    broken 0 < tokens < block_size case.

Tests

Adds tests/test_get_max_cached_blocks.py — GPU-free (no vmm_ops, GPU, or
installed vLLM), mirroring the existing tests/test_make_cache_key.py. Covers
the -1 / 0 / sub-block / exact-multiple / non-multiple / large boundaries, the
#343 regression (0 < tokens < block_size never collapses to the disabled
sentinel), and the warn-on-clamp / no-warn-on-full-block behavior.

$ pytest tests/test_get_max_cached_blocks.py -q
12 passed

_get_max_cached_blocks derived the block budget as
MAX_CACHED_TOKENS // block_size. A positive KVCACHED_MAX_CACHED_TOKENS
smaller than block_size (e.g. 8 tokens with a 16-token block) floored to
0, which is the same value the == 0 'disabled' sentinel produces, so
prefix caching was silently turned off instead of capped at a small
budget.

Floor the positive branch at one block and log a warning explaining the
effective granularity, so a small non-zero budget keeps prefix caching
enabled. Negative (unlimited), zero (disabled), and >= block_size budgets
are unchanged.

Adds tests/test_get_max_cached_blocks.py (GPU-free, mirrors
test_make_cache_key.py) covering the sentinel boundaries and the
sub-block regression.

Fixes ovg-project#343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_get_max_cached_blocks truncates small positive KVCACHED_MAX_CACHED_TOKENS to the disabled

1 participant