Skip to content

[Bugfix][Core] Return False for retryable prefix-cache reset failures - #60573

Open
Vegetog wants to merge 1 commit into
vllm-project:mainfrom
Vegetog:fix-prefix-cache-reset-http
Open

Vegetog wants to merge 1 commit into
vllm-project:mainfrom
Vegetog:fix-prefix-cache-reset-http

Conversation

@Vegetog

@Vegetog Vegetog commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Related to #60552 (partial fix). Return False when a forced prefix-cache reset still has referenced blocks, so /reset_prefix_cache?reset_running_requests=true returns the documented retryable 200 {"success": false} instead of HTTP 500.

Merging this PR alone fixes the reset response, but does not make the full #60552 offloading reproducer pass. success: false means the cache reset did not complete; referenced blocks remain in use and the caller can retry later. When a preempted request resumes, the existing OffloadingConnector assertion can still crash the engine and interrupt generation.

The full reproduced scenario also needs the companion connector fix in #60574, or an equivalent fix for the same bug covered by #57810. This is a requirement for end-to-end recovery, not a required merge order: the two PRs address separate root causes.

Applied fixes Observed effect
This PR alone The reset returns the retryable HTTP response, but repeated runs still encounter the connector preempt/resume assertion.
This PR + #60574 The reset returns the retryable HTTP response, streams complete, and subsequent generation remains healthy in the combined tests below.

A parked streaming-input session (WAITING_FOR_STREAMING_REQ, #60744) is a different holder: it waits on the client rather than on a transfer, so with this PR alone the reset keeps returning False however often it is retried; #60764 reclaims those sessions in the same function, so the two changes are complementary.

Claims

  • Preserve blocks held by in-flight KV transfers and allow a later reset to succeed after the transfer completes.
  • Preserve the early exit before connector reset, and the existing failure checks that prevent cache-clearing pause/sleep from proceeding after an unsuccessful reset.

Validation

Standalone patch on 0f112d180033f3757cf283723f5593a40b02ed82:

.venv/bin/python -m pytest \
  tests/v1/core/test_scheduler.py \
  tests/v1/core/test_async_scheduler.py \
  tests/v1/engine/test_engine_core.py \
  -k 'reset_prefix_cache or inflight_remote_kv or aux_output_reset or reset_connector_cache or kv_cache_release or pause_synchronizes' -v

19 passed. The four new regression cases failed before the fix. Pre-commit passed for both changed files.

The original #60552 reproducer returned HTTP 500 in 5/5 flagged trials on the initial baseline fb2ac824. With this patch alone, the retryable HTTP response was observed, but repeated runs also exposed the pre-existing offloading preempt/resume crash described in #57810. That crash is addressed separately by #60574 and is not fixed by this PR.

Combined serving validation on the same base, with both this patch and #60574 applied: one RTX 3070, Qwen3-0.6B, eager execution with TRITON_ATTN, 320 GPU blocks and 4 GB CPU offload. The Python checkout reused the official v0.31.0+cu129 CUDA extensions.

These are combined-patch serving results, not a claim that this PR alone fixes the separate connector crash.

No model-quality or performance claim is made. The serving checks validate HTTP responses, stream completion and continued generation; the results above are local validation, not a claim of passing upstream test CI.

Details

Preempting self.running does not release blocks owned by WAITING_FOR_REMOTE_KVS requests. The cache manager already reports this condition as False; the scheduler currently converts it into an exception only when reset_running_requests=True.

Return False at the same point without changing preemption or transfer ownership. Keep the early return so a failed forced reset does not start a connector reset. EngineCore._reset_caches() already checks the result, so no pause/sleep caller changes are needed.

The duplicate-work check before opening this PR found no existing fix for this HTTP return-value contract. Companion #60574 and the overlapping fix in #57810 address the separate same-step offloading resume crash. This PR changes only the scheduler and its existing tests.

Developed and validated with OpenAI Codex assistance.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Vegetog <110553275+Vegetog@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working scheduler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant