mudler / mudler/vllm.cpp

fix(ENG-RUNNER-MODELSHAPE): BlockTable::move_row leaves the vacated source row populated

Open Beginner friendly
#2,636 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: ENG-RUNNER-MODELSHAPE

Found by re-deriving upstream 6fbbcf2151 (vllm#49757) against this tree in
#2632, wave PORTQ-1.

The defect

BlockTable::move_row copies the source row into the target and updates the
target's block count, and then stops. It never clears the row it vacated, so the
source row keeps a full copy of the block ids it no longer owns and
num_blocks_per_row[src] keeps counting them.

src/vllm/v1/worker/gpu/block_table.cpp:105-113:

void BlockTable::move_row(int src, int tgt) {
  const int num_blocks = num_blocks_per_row[static_cast<size_t>(src)];
  ...
  num_blocks_per_row[static_cast<size_t>(tgt)] = num_blocks;
}

clear_row directly above it (:95-103) already does exactly the clearing that
is missing, so the fix is to call it.

Upstream added the same clearing at vllm/v1/worker/block_table.py:144-151
@ 6fbbcf2151 (a forward reference past the pin 5559679229), for the reason
that a dummy run then wrote Mamba state through the stale row.

What is NOT claimed

The upstream symptom is not reachable here today. This tree has no
_dummy_run, and padded decode rows are built explicitly rather than by
dereferencing whatever the block table happens to hold:
src/vllm/model_executor/models/qwen3_5.cpp:10309 writes -1 into every padded
GDN state index and slot-mapping entry, and include/vllm/v1/worker/gpu/runner.h:598
rewrites the GDN group's block-table column 0 into a compact per-sequence slot
keyed on req_id. So a stale row past num_reqs is never read.

This is a defensive fix and a mirror repair, not a live defect. It is filed
because the divergence is real and cheap to close, and because the structure that
makes it unreachable is the runner's, not the block table's: a future reader of
move_row has no way to know the invariant is upheld elsewhere.

Size

~4 lines in src/vllm/v1/worker/gpu/block_table.cpp plus one red-first unit case
under tests/vllm/v1/worker/. The live caller is
src/vllm/v1/worker/gpu/input_batch.cpp:709.

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 src/vllm/v1/worker/gpu/block_table.cpp, comparing move_row with clear_row, then inspect the live caller at src/vllm/v1/worker/gpu/input_batch.cpp:709. Add the missing source-row clearing and a red-first unit case under tests/vllm/v1/worker/; done means the vacated row has no block IDs and its block count is zero.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.