mudler / mudler/vllm.cpp

port(KV-SIZING): max_model_len is sized against a pool that includes the unallocatable null block, so the boundary case stalls instead of refusing

Open
#2,719 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
423
Forks
53
Avg merge
20h 26m
Merged PRs (30d)
310

Description

Row: KV-SIZING (.agents/engine-matrix.md:115, re-resolved by line number before filing)

Found by wave PORTQ-6 (#2718) re-deriving PORT-NOW entry [212], upstream 76fb6d210a vllm#47272, of 5559679229..e126687a9a. Nothing was executed. This is a static reading of source.

What upstream does

vllm/v1/core/kv_cache_utils.py, two hunks. check_enough_kv_cache_memory and get_kv_cache_configs now subtract _pool_bytes_per_block(...) from the available memory before the capacity check and before _auto_fit_max_model_len. Allocation still uses the full memory; only the planning arithmetic changes. The reason is that BlockPool permanently withholds the null block, so one block per group is never allocatable.

The helper _pool_bytes_per_block already existed at the pin (git show 5559679229:vllm/v1/core/kv_cache_utils.py:972) and was used for the memory-override path only. The subtraction at these two sites is this commit, so this gap is owed by the pin advance.

What is here

The same over-count, in the loader:

// src/vllm/entrypoints/model_loader.cpp:1933-1935
const int64_t bytes_per_block = vllm::v1::KVBytesPerBlock(kv_cfg);
const int64_t available =
    static_cast<int64_t>(kv_cfg.num_blocks) * bytes_per_block;

That available is the argument to both arms — the pinned-length check at model_loader.cpp:1946-1949 (check_enough_kv_cache_memory plus estimate_max_model_len) and the auto-fit at model_loader.cpp:1961-1962. It is exactly the pair of call sites upstream corrected.

The null block is real and is drawn from the same count. src/vllm/v1/core/kv_cache_coordinator.cpp:130 constructs block_pool(this->kv_cache_config.num_blocks, ...), and src/vllm/v1/core/block_pool.cpp:53-57 pops block 0 out of the free queue so "it can never be allocated". Usable blocks are num_blocks - 1; the sizing math uses num_blocks.

KVBytesPerBlock (src/vllm/v1/kv_cache_interface.cpp:241-266) is already the all-group per-block byte total, this tree's _pool_bytes_per_block, so the correction is one term.

Nothing compensates elsewhere: LoadedEngine::ResolveNumBlocks (model_loader.cpp:1722-1785) returns the raw override, the budget quotient, or the fallback 256, with no +1 for the null block.

There is no second sizing path to also fix. A search for callers of check_enough_kv_cache_memory, auto_fit_max_model_len, estimate_max_model_len and kv_memory_needed_bytes over src/, include/ and tests/ returns exactly one product caller, model_loader.cpp:1946-1962.

Consequence: a silent stall, not a refusal

A pool of N blocks accepts a max_model_len that needs all N. The request prefills and then cannot get its last block, and src/vllm/v1/core/sched/scheduler.cpp:935-941 breaks out of the waiting loop, leaving it queued with no error. --num-blocks 1 today builds an engine that reports a max_model_len it can never allocate. It is off by exactly one block, so it bites at the boundary — which is where auto-fit deliberately lands.

Size

~5-10 product lines: subtract one block's bytes from available before both arms, ideally through a named PoolBytesPerBlock helper in kv_cache_utils so the spelling mirrors upstream.

~50-70 test lines. Five existing cases in tests/vllm/entrypoints/test_loaded_engine_dense.cpp set params.num_blocks = 1 and assert the current arithmetic (e.g. :671-680 asserts auto-fit yields 32 from a one-block pool). After the fix a one-block pool has zero usable blocks and auto_fit_max_model_len must throw. The port moves those five boundaries and adds a red-first case at the N vs N-1 edge.

Zero, with its control

_pool_bytes_per_block / PoolBytesPerBlock over src/ and include/ returns 0. Positive control through the identical probe form and scope: null_block returns 22 hits across block_pool.cpp, single_type_kv_cache_manager.cpp and block_pool.h.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/vllm/entrypoints/model_loader.cpp around LoadedEngine::ResolveNumBlocks and the sizing calls at lines 1946-1962. Read the null-block handling in src/vllm/v1/core/block_pool.cpp and the existing boundary cases in tests/vllm/entrypoints/test_loaded_engine_dense.cpp, then run those tests. Done means the sizing checks account for only allocatable blocks and the one-block boundary refuses or throws instead of stalling.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, performance
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.