mudler / mudler/vllm.cpp

fix(KV-MAMBA-ALIGN): prefix_cache_retention_interval is consumed but never produced, so retention is permanently dense

Open
#2,692 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-MAMBA-ALIGN (.agents/engine-matrix.md:101); KV-HYBRID-COORD (:100) is the alternative owner if the SWA half is split out.

Found by wave PORTQ-5 (#2679) re-deriving PORT-NOW entry [182], upstream 017e9f4448 vllm#52216. Nothing was executed.

The shape of the gap: the consumer is written, the producer is not

The retention machinery is fully implemented and reached, and then fed nullopt unconditionally:

  • include/vllm/v1/core/kv_cache_coordinator.h:185std::optional<int> retention_interval = std::nullopt;
  • src/vllm/v1/core/kv_cache_coordinator.cpp:166// retention_interval stays nullopt (dense caching); env read deferred. The constructor never assigns it.
  • The three-way semantics upstream defines already exist downstream: src/vllm/v1/core/single_type_kv_cache_manager.cpp:503-508 (!has_value() => dense, == 0 => no periodic segment, > 0 => interval), with the latest-replay-boundary pin at :523-530 and the Mamba arm at :704-715.
  • Both call sites pass it through (kv_cache_coordinator.cpp:237, :461).
  • KVCacheConfig (include/vllm/v1/kv_cache_interface.h:512-538) has no such field.

The deferral is deliberate and recorded at include/vllm/v1/core/kv_cache_coordinator.h:68-70, which is the only occurrence of the string prefix_cache_retention_interval in src/+include/+tests/ (positive control through the identical probe form: retention_interval => 26 hits).

What upstream did

Moved the value off envs.VLLM_PREFIX_CACHE_RETENTION_INTERVAL onto a resolved CacheConfig.prefix_cache_retention_interval (ge=0, deprecated-env fallback), threaded it through arg_utils.py into KVCacheConfig, and changed the default from None to 0, with _validate_prefix_cache_retention_interval gaining an early return for 0 so a model with no SWA/Mamba group no longer raises.

Porting this subsumes a pre-pin hole

At the pin, vllm/v1/core/kv_cache_coordinator.py:152 reads the value from the env — which this tree also does not do. So the tree is behind the pin on the env read and behind the target on the config promotion. Our current default (nullopt = dense) equals upstream's pre-commit default, so the flip to 0 is a real, observable behaviour change for SWA and Mamba groups.

Size

~80-140 lines plus tests:

  1. prefix_cache_retention_interval on KVCacheConfig. Note the tree's own comment at include/vllm/v1/kv_cache_interface.h:535: a default member initializer is required to keep positional aggregate init valid.
  2. Resolution with default 0 (not nullopt) plus the deprecated-env fallback.
  3. _validate_prefix_cache_retention_interval, including the == 0 early return and the % scheduler_block_size check.
  4. Coordinator assignment replacing the hard-wired nullopt at kv_cache_coordinator.cpp:166.
  5. The CLI/ABI knob. There is no CacheConfig class here (include/vllm/config/cache.h holds only dtype helpers), so this lands on vllm_model_params beside enable_prefix_caching (include/vllm.h:449) and is an ABI bump.

Not established

Which shipped gate models exercise a SlidingWindow or Mamba group with prefix caching on. The default flip from dense to 0 changes retained checkpoints for exactly those, and nothing was run — so the flip should not land without that being checked.

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 with the retention_interval declaration and assignment in include/vllm/v1/core/kv_cache_coordinator.h and src/vllm/v1/core/kv_cache_coordinator.cpp, then trace KVCacheConfig in include/vllm/v1/kv_cache_interface.h and vllm_model_params in include/vllm.h. Compare the existing validation and cache-manager semantics with the upstream behavior described in the issue, and identify the affected tests before changing the default. Done means the value is resolved, threaded through configuration and the coordinator, validated, exposed through the CLI/ABI, and the SWA/Mamba default change is checked.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.