Skip to content

Commit 028b096

Browse files
[Bugfix][KV Offloading] Drop conflicting finished-req store jobs to prevent data corruption
When _finished_req_store_jobs contains a store job and the block is reallocated to a new request, the store job would read corrupted data. Fix: In build_connector_meta, after merging _finished_req_store_jobs into store_jobs, check if any of those jobs' blocks are in _current_batch_allocated_block_ids. If so, drop the job entirely (same behavior as before the request_finished fix — the last block is simply not offloaded when its block is reused). This prevents the race where wait() is called on an unsubmitted job (no-op), allowing block corruption before the store reads it. Signed-off-by: Alex <alex.tech.lab@outlook.com>
1 parent 98cffbc commit 028b096

2 files changed

Lines changed: 53 additions & 9 deletions

File tree

tests/v1/kv_connector/unit/offloading_connector/test_scheduler.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2902,14 +2902,17 @@ def inject_request_b():
29022902

29032903
# Run Request A with pre_schedule_fn to inject Request B at the right time
29042904
# Use complete_transfers=True to allow jobs to complete and loop to exit
2905+
# decoded_tokens has three EOS tokens: one for Request A, two for Request B
2906+
# (async mode may need extra steps)
2907+
# In async mode, block reuse triggers flush; in sync mode, job is dropped before flush
29052908
runner.run(
2906-
decoded_tokens=[EOS_TOKEN_ID],
2909+
decoded_tokens=[EOS_TOKEN_ID, EOS_TOKEN_ID, EOS_TOKEN_ID],
29072910
complete_transfers=True,
29082911
drain_finished_req_stores=True,
29092912
pre_schedule_fn=inject_request_b,
29102913
expected_stored=(0,), # Request A stores block 0
2911-
expected_flushed=(0,), # Request B reuses block 0, triggering flush
2912-
max_steps=20,
2914+
expected_flushed=(0,) if async_scheduling else (), # flush depends on timing
2915+
max_steps=30,
29132916
)
29142917

29152918
# Verify Request B was injected

vllm/distributed/kv_transfer/kv_connector/v1/offloading/scheduler.py

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -930,9 +930,8 @@ def _build_store_jobs_for_req(
930930
# A block_id of 0 means either a sliding window / SSM skip
931931
# or a stale entry that was zeroed out — skip it either way.
932932
offload_block_ids = group_state.block_ids[
933-
start_block_idx * block_size_factor
934-
+ block_size_factor
935-
- 1 : num_blocks * block_size_factor : block_size_factor
933+
start_block_idx * block_size_factor + block_size_factor - 1 : num_blocks
934+
* block_size_factor : block_size_factor
936935
]
937936
assert len(offload_keys) == len(offload_block_ids)
938937

@@ -987,9 +986,7 @@ def _build_store_jobs_for_req(
987986
for group_config, group_state in zip(
988987
self.config.kv_group_configs, req_status.group_states
989988
):
990-
is_sliding_window = (
991-
group_config.sliding_window_size_in_blocks is not None
992-
)
989+
is_sliding_window = group_config.sliding_window_size_in_blocks is not None
993990
num_blocks = req_status.storable_blocks(
994991
group_config, num_offloadable_tokens
995992
)
@@ -1137,9 +1134,53 @@ def build_connector_meta(
11371134
)
11381135

11391136
store_jobs = self._build_store_jobs(scheduler_output)
1137+
1138+
# Track which jobs came from _finished_req_store_jobs before merging
1139+
finished_req_job_ids = set(self._finished_req_store_jobs.keys())
1140+
11401141
store_jobs.update(self._finished_req_store_jobs)
11411142
self._finished_req_store_jobs.clear()
11421143

1144+
# Drop finished-req store jobs whose blocks have been reallocated.
1145+
# If a block is in _current_batch_allocated_block_ids, a new request
1146+
# is about to overwrite it. The store job would read stale/corrupted
1147+
# data, so we skip it entirely (same behavior as before the
1148+
# request_finished fix — the last block is simply not offloaded).
1149+
# Only drop jobs that came from _finished_req_store_jobs, not normal
1150+
# store jobs from _build_store_jobs.
1151+
if self._current_batch_allocated_block_ids:
1152+
jobs_to_drop = set()
1153+
for job_id in finished_req_job_ids:
1154+
if job_id not in store_jobs:
1155+
continue # Already removed or not present
1156+
job = self._jobs.get(job_id)
1157+
if job is not None and job.is_store:
1158+
job_blocks = set(job.non_sliding_window_block_ids or [])
1159+
job_blocks.update(job.sliding_window_block_ids or [])
1160+
if job_blocks & self._current_batch_allocated_block_ids:
1161+
jobs_to_drop.add(job_id)
1162+
for job_id in jobs_to_drop:
1163+
del store_jobs[job_id]
1164+
self._current_batch_jobs_to_flush.discard(job_id)
1165+
job = self._jobs.pop(job_id, None)
1166+
if job is not None:
1167+
for bid in job.sliding_window_block_ids or []:
1168+
pjs = self._block_id_to_pending_jobs.get(bid)
1169+
if pjs is not None:
1170+
pjs.discard(job_id)
1171+
if not pjs:
1172+
del self._block_id_to_pending_jobs[bid]
1173+
for bid in job.non_sliding_window_block_ids or []:
1174+
pjs = self._block_id_to_pending_jobs.get(bid)
1175+
if pjs is not None:
1176+
pjs.discard(job_id)
1177+
if not pjs:
1178+
del self._block_id_to_pending_jobs[bid]
1179+
# Also remove from req_status.transfer_jobs
1180+
req_status = self._req_status.get(job.req_id)
1181+
if req_status is not None:
1182+
req_status.transfer_jobs.discard(job_id)
1183+
11431184
meta = OffloadingConnectorMetadata(
11441185
load_jobs=self._current_batch_load_jobs,
11451186
store_jobs=store_jobs,

0 commit comments

Comments
 (0)