Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions tests/v1/core/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -4474,3 +4474,81 @@ def test_ec_connector_pending_prefetch_only_checks_future_mm_features():
f"Expected only {HASH_FUTURE!r} from future mm feature filtering, "
f"got {future_hashes!r}. Past/boundary features must be filtered out."
)


def test_async_load_reservation_prevents_wedge_e2e():
"""Same wedge scenario as PR #40968's lateral-preemption e2e test, but
resolved by reservation-based admission control instead of preemption.

A (8 blocks) and B (5 blocks) both want an async KV load, sharing a 4-block
prefix, in a 10-block pool (9 usable). Admitting both loads would wedge:
once their recvs finish neither can complete its local prefill (8+5 > 9).

Here the reservation gate refuses to admit B's load while A's full sequence
is still reserved, so B never holds blocks and A is free to complete - no
deadlock, and (unlike lateral preemption) B is never preempted.
"""
BLOCK_SIZE = 16
A_TOKENS = BLOCK_SIZE * 8 # bigger request
B_TOKENS = BLOCK_SIZE * 5 # smaller request
MATCHED_TOKENS = BLOCK_SIZE * 4 # 4-block prefix loaded for both
NUM_BLOCKS = 10 # 9 usable; both prefixes fit, but not both full sequences

scheduler = create_scheduler(
block_size=BLOCK_SIZE,
num_blocks=NUM_BLOCKS,
max_num_seqs=4,
max_num_batched_tokens=A_TOKENS * 2,
use_kv_connector=mock_kv(matched_tokens=MATCHED_TOKENS, is_async=True),
)

[a] = create_requests(
num_requests=1, num_tokens=A_TOKENS, block_size=BLOCK_SIZE, req_ids=["a"]
)
[b] = create_requests(
num_requests=1, num_tokens=B_TOKENS, block_size=BLOCK_SIZE, req_ids=["b"]
)
scheduler.add_request(a)
scheduler.add_request(b)

EMPTY_OUTPUT = ModelRunnerOutput(
req_ids=[],
req_id_to_index={},
sampled_token_ids=[],
logprobs=None,
prompt_logprobs_dict={},
pooler_output=[],
)

req_to_blocks = scheduler.kv_cache_manager.coordinator.single_type_managers[
0
].req_to_blocks

# Step 1: A's load is admitted; B's is held back by the reservation (B never
# holds blocks, so the wedge precondition - both holding prefixes - is gone).
out1 = scheduler.schedule()
assert a.status == RequestStatus.WAITING_FOR_REMOTE_KVS
assert a.num_computed_tokens == MATCHED_TOKENS
assert b.status == RequestStatus.WAITING
assert b.request_id not in req_to_blocks
assert len(scheduler.running) == 0
scheduler.update_from_output(out1, EMPTY_OUTPUT)

# Step 2: nothing changes until A's recv lands.
out2 = scheduler.schedule()
assert len(scheduler.running) == 0
a_finished = dataclasses.replace(
EMPTY_OUTPUT,
kv_connector_output=KVConnectorOutput(finished_recving=[a.request_id]),
)
scheduler.update_from_output(out2, a_finished)

# Step 3: A makes forward progress straight to RUNNING - no preemption was
# needed because B never wedged it.
out3 = scheduler.schedule()
assert a.status == RequestStatus.RUNNING
assert a in scheduler.running
assert a.request_id in {req.req_id for req in out3.scheduled_new_reqs}
assert b.status == RequestStatus.WAITING
assert b.num_preemptions == 0
assert b.request_id not in req_to_blocks
2 changes: 2 additions & 0 deletions tests/v1/e2e/general/test_mamba_prefix_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ def fake_allocate_slots_fn(
delay_cache_blocks: bool = False,
num_encoder_tokens: int = 0,
full_sequence_must_fit: bool = False,
reserved_blocks: int = 0,
):
ret = original_allocate_slots_fn(
self,
Expand All @@ -192,6 +193,7 @@ def fake_allocate_slots_fn(
delay_cache_blocks,
num_encoder_tokens,
full_sequence_must_fit,
reserved_blocks,
)
if cur_step_action is not None:
cur_block_ids = self.coordinator.single_type_managers[0].req_to_blocks[
Expand Down
79 changes: 79 additions & 0 deletions tests/v1/kv_connector/unit/test_remote_prefill_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,3 +655,82 @@ def test_p_side_chunked_prefill_mamba(mock_platform):
outputs = engine_core_outputs[0].outputs
assert len(outputs) == 1
assert outputs[0].finish_reason == FinishReason.LENGTH


def test_async_load_reserves_blocks_for_inflight():
"""A second async KV-connector load is not admitted if its initial
allocation would consume blocks reserved for an already in-flight sequence.

req_a gets a 1-block prefix (full sequence = 4 blocks), reserving 3 more.
req_b would need a 4-block initial allocation, but only
(free - req_a's 3-block reservation) = 3 blocks are available to it, so it is
held back in WAITING (holding no blocks) rather than wedging req_a.
"""
vllm_config = create_vllm_config()
BLOCK_SIZE = vllm_config.cache_config.block_size
scheduler = create_scheduler(vllm_config, num_blocks=8) # usable = 7

req_a = create_request(
request_id=1,
block_size=BLOCK_SIZE,
num_tokens=BLOCK_SIZE * 4,
do_remote_prefill=True,
num_remote_blocks=1,
)
req_b = create_request(
request_id=2,
block_size=BLOCK_SIZE,
num_tokens=BLOCK_SIZE * 5,
do_remote_prefill=True,
num_remote_blocks=1,
)
scheduler.add_request(req_a)
scheduler.add_request(req_b)

# Partial external matches: req_a loads 1 block, req_b loads 4 blocks.
with patch.object(
scheduler.connector,
"get_num_new_matched_tokens",
side_effect=[(BLOCK_SIZE, True), (BLOCK_SIZE * 4, True)],
):
scheduler.schedule()

assert req_a.status == RequestStatus.WAITING_FOR_REMOTE_KVS
assert req_b.status == RequestStatus.WAITING

req_to_blocks = scheduler.kv_cache_manager.coordinator.single_type_managers[
0
].req_to_blocks
assert req_a.request_id in req_to_blocks
assert req_b.request_id not in req_to_blocks


def test_async_loads_both_admitted_when_pool_fits():
"""Sanity: with a pool large enough, the reservation gate admits both async
loads (it is not over-conservative)."""
vllm_config = create_vllm_config()
BLOCK_SIZE = vllm_config.cache_config.block_size
scheduler = create_scheduler(vllm_config, num_blocks=64)

reqs = [
create_request(
request_id=i,
block_size=BLOCK_SIZE,
num_tokens=BLOCK_SIZE * 5,
do_remote_prefill=True,
num_remote_blocks=1,
)
for i in (1, 2)
]
for req in reqs:
scheduler.add_request(req)

with patch.object(
scheduler.connector,
"get_num_new_matched_tokens",
side_effect=[(BLOCK_SIZE, True), (BLOCK_SIZE, True)],
):
scheduler.schedule()

for req in reqs:
assert req.status == RequestStatus.WAITING_FOR_REMOTE_KVS
1 change: 1 addition & 0 deletions tests/v1/kv_connector/unit/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def assert_scheduler_empty(scheduler: Scheduler):
assert len(scheduler.running) == 0
assert len(scheduler.finished_req_ids) == 0
assert len(scheduler.finished_recving_kv_req_ids) == 0
assert len(scheduler._inflight_prefills) == 0

# EncoderCacheManager.
assert len(scheduler.encoder_cache_manager.freed) == 0
Expand Down
9 changes: 8 additions & 1 deletion vllm/v1/core/kv_cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ def allocate_slots(
delay_cache_blocks: bool = False,
num_encoder_tokens: int = 0,
full_sequence_must_fit: bool = False,
reserved_blocks: int = 0,
) -> KVCacheBlocks | None:
"""Add slots for a request with new tokens to append.

Expand All @@ -271,6 +272,11 @@ def allocate_slots(
free blocks to hold the full sequence, accounting for prefix cache hits
and sliding window. Used as an admission gate to prevent over-admitting
requests when chunked prefill would otherwise only check the first chunk
reserved_blocks: Number of free blocks that must be left available for
other in-flight sequences to complete. The actual allocation is only
made if it fits within (free blocks - reserved_blocks). Used to gate
async KV-connector loads so their initial allocation cannot consume
blocks an already in-flight (prefilling) sequence is relying on.

Blocks layout:
```
Expand Down Expand Up @@ -386,7 +392,8 @@ def allocate_slots(
num_tokens_main_model=num_tokens_main_model,
)

if num_blocks_to_allocate > self.block_pool.get_num_free_blocks():
available_blocks = self.block_pool.get_num_free_blocks() - reserved_blocks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This estimation is pessimistic, so it might reduce utilization? I'm not sure of the trade-off vs preemption (which only happens at the peak usage).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. This is correct for PD case and might affect throughput for non-PD on some edge cases. But those cases are rare and should run in PD anyway.

if num_blocks_to_allocate > available_blocks:
# Cannot allocate new blocks
return None

Expand Down
46 changes: 46 additions & 0 deletions vllm/v1/core/sched/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,10 @@ def __init__(

self._pause_state: PauseState = PauseState.UNPAUSED

# In-flight requests still prefilling (prefill chunks + in-progress
# async KV loads). Their remaining-block reservation gates async loads.
self._inflight_prefills: set[Request] = set()

def _mamba_block_aligned_split(
self,
request: Request,
Expand Down Expand Up @@ -746,6 +750,14 @@ def schedule(self) -> SchedulerOutput:
for i in encoder_inputs_to_schedule
)

reserved_blocks = 0
if load_kv_async:
# An async load holds its blocks for the whole transfer with
# no forward progress and isn't preemptible here. Admit it
# only if it fits in (free - other in-flight reservations), to
# avoid deadlock and predictable preemptions.
reserved_blocks = self._inflight_prefill_reserved_blocks()

new_blocks = self.kv_cache_manager.allocate_slots(
request,
num_new_tokens,
Expand All @@ -756,6 +768,7 @@ def schedule(self) -> SchedulerOutput:
delay_cache_blocks=load_kv_async,
num_encoder_tokens=num_encoder_tokens,
full_sequence_must_fit=self.scheduler_reserve_full_isl,
reserved_blocks=reserved_blocks,
)

if new_blocks is None:
Expand Down Expand Up @@ -807,6 +820,7 @@ def schedule(self) -> SchedulerOutput:
# _update_waiting_for_remote_kv will then cache
# only the successfully loaded tokens.
request.num_computed_tokens = num_computed_tokens
self._inflight_prefills.add(request)
continue

self.running.append(request)
Expand All @@ -830,6 +844,9 @@ def schedule(self) -> SchedulerOutput:
token_budget -= num_new_tokens
request.status = RequestStatus.RUNNING
request.num_computed_tokens = num_computed_tokens
# Only track requests that will still be prefilling after this chunk.
if num_computed_tokens + num_new_tokens < request.num_tokens:
self._inflight_prefills.add(request)
# Encoder-related.
if encoder_inputs_to_schedule:
scheduled_encoder_inputs[request_id] = encoder_inputs_to_schedule
Expand Down Expand Up @@ -965,6 +982,7 @@ def _preempt_request(self, request: Request, timestamp: float) -> None:
)
self.kv_cache_manager.free(request)
self.encoder_cache_manager.free(request)
self._inflight_prefills.discard(request)
request.status = RequestStatus.PREEMPTED
request.num_computed_tokens = 0
if request.spec_token_ids:
Expand Down Expand Up @@ -996,6 +1014,9 @@ def _update_after_schedule(self, scheduler_output: SchedulerOutput) -> None:
scheduler_output.has_structured_output_requests |= (
request.use_structured_output and not request.is_prefill_chunk
)
# Drop from the in-flight-prefill set once it's no longer prefilling.
if not request.is_prefill_chunk:
self._inflight_prefills.discard(request)

# Snapshot block IDs for routed experts before forward starts.
# A concurrent schedule() may preempt requests and free blocks
Expand Down Expand Up @@ -1869,6 +1890,7 @@ def _free_request(
) -> dict[str, Any] | None:
assert request.is_finished()

self._inflight_prefills.discard(request)
connector_delay_free_blocks, kv_xfer_params = self._connector_finished(request)
self.encoder_cache_manager.free(request)
request_id = request.request_id
Expand Down Expand Up @@ -2105,6 +2127,30 @@ def _connector_finished(

return self.connector.request_finished_all_groups(request, block_ids)

def _request_remaining_blocks(self, request: Request) -> int:
"""Blocks `request` still needs to allocate to hold its full sequence."""
full_num_tokens = min(request.num_tokens, self.max_model_len)
return self.kv_cache_manager.coordinator.get_num_blocks_to_allocate(
request_id=request.request_id,
num_tokens=full_num_tokens,
new_computed_blocks=self.kv_cache_manager.empty_kv_cache_blocks.blocks,
num_encoder_tokens=0,
total_computed_tokens=request.num_computed_tokens,
num_tokens_main_model=full_num_tokens,
apply_admission_cap=True,
)

def _inflight_prefill_reserved_blocks(self) -> int:
"""Blocks in-flight prefills still need to finish (their reservation).

Sums remaining full-ISL blocks over `self._inflight_prefills` (running
prefills + in-progress async loads). The candidate async load isn't yet
in the set, so it's naturally excluded.
"""
return sum(
self._request_remaining_blocks(req) for req in self._inflight_prefills
)

def _update_waiting_for_remote_kv(self, request: Request) -> None:
"""
KV Connector: update request state after async recv is finished.
Expand Down
Loading