[205150940]

PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.

Purpose

Test Plan

Test Result

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results
• (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
• (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[205150940]

测试方法
1.在_run_ar的all_reduce后添加
if (parallel_config.times == 5 and dp_rank == 1): raise RuntimeError("FAULT INJECTION: exception after allreduce")
抛出异常后，retry可继续进行推理
2.在sync_cudagraph_and_dp_padding的all_reduce后添加
if (parallel_config.times == 6 and dp_rank == 1): raise RuntimeError("FAULT INJECTION: exception after allreduce")
抛出异常后，retry可继续推理
3.pytest正常通过

注：当前代码在prefill阶段抛出异常，首token 丢失，原因正在排查（已修复，原因为request.async_tokens_to_discard状态刷新错误）

[fangyuchu]

Review: fork PR #260 — [Bugfix] Evict and zero contaminated KV blocks on fault retry

PR: #260
改动只有一个文件：vllm/v1/fault_tolerance/engine_core_sentinel.py 的 on_fault()，+13/-1 行。

一句话结论

问题是真的、方向是对的，但现在的实现"宁可错杀一千"——把所有 running 请求的所有 KV block 都从 prefix cache 里删了，导致 resume 后全部从头重算，resume 省 KV 的意义没了。另外 PR 标题说有 zero，代码里没有；还顺手删了一行与标题无关的代码，没解释。

背景：这个 PR 在修什么

fault 发生时，on_fault 把所有 running 请求 preempt 回 waiting 队列（base PR #253 的 resume 功能）。vLLM v1 里 preempt 释放的 block 不会从 prefix cache 删除，hash 还留着，resume 后请求能命中自己的旧 block、跳过重算——这是 resume 的核心收益。

问题在于：失败的那个 step，schedule 阶段就把刚写满的 block 登记进了 prefix cache，但 forward 还没跑（或跑了一半写了垃圾）。这些 block 里的 KV 是脏的，但 resume 后会被当成好 block 命中。这个 PR 就是要把它们从 cache 里删掉。

逐条问题

1. 驱逐范围太大（最主要的问题）

现在驱逐的是所有 running 请求的全部 block。但真正脏的只有失败 step 写的那一段——每个请求最多尾部 1 个 full block。前面的 block 都是之前成功 step 写的，是干净的。

后果：resume 之后每个请求都从头 prefill，一点 KV 都复用不了。那 resume 相比直接 abort 就只剩"client 不报错"这一个区别了，性能收益归零。还会误删其他请求（已完成的）贡献的共享 prefix block。

精确的写法是只驱逐"还没确认的尾部"：[num_computed_tokens - num_in_flight_tokens, num_computed_tokens) 这段 token 覆盖的 block。num_in_flight_tokens 就是"已调度但 output 还没回来"的 token 数，scheduler.py:2543 已经在用同样的算法算确认边界。

2. 标题说 "zero"，代码里没有 zero

PR 标题是 "Evict and zero"，但 diff 里只有 evict。两种处理方式：

• 改标题；或者
• 真的接上现成的 record_blocks_for_zeroing()（kv_cache_manager.py:817，KV-load-failure 路径已经在用），让这些 block 被重新分配时由 worker 清零。注意要在 preempt 之前调（它依赖 req_to_blocks），且只在 needs_kv_cache_zeroing 开启时生效。

3. 删掉的 async_tokens_to_discard 那行——删得对，但没解释

Base (#253) 里这行是从 reset_prefix_cache 抄来的：

request.async_tokens_to_discard = request.num_output_placeholders

reset_prefix_cache 的场景引擎是健康的，in-flight 的 output frame 一定会回来，所以要计数丢弃。但 fault 场景不一样：失败 step 的 frame 根本不会产生（worker 抛异常了），batch_queue.clear() + worker 侧 execute_model_state=None 之后也不会有残留 frame 送回来。如果留着这行，discard 计数永远消耗不完，resume 后第一个正常的 output frame 会被误丢，scheduler 和 worker 的 token 状态就错位了。

所以删除是对的，但这是和"evict block"无关的第二个改动，混在一个标题下、PR 描述空白、没有测试，review 的人根本看不出意图。

需要验证的反向风险：如果某种边缘情况下迟到的 frame 真的送达了（比如异常发生在 engine 侧而不是 worker 侧），async_scheduler._update_request_with_output 里 num_output_placeholders -= len(new_token_ids) 会变成负数触发 assert，busy loop 会再炸一次。要么确认"frame 不可能到达"这个前提成立，要么给非 RUNNING 请求的迟到 frame 加显式丢弃。

4. 小问题

• collection 循环写在 if/else 分支外面，所以 abort 路径（不开 resume 时）行为也被改了——也会 evict。abort 下 evict 本身合理（防止后续请求命中脏 block），但同样太宽，而且 PR 没提这个影响面。
• 0 <= blk < len(pool.blocks) 这种防御性检查不符合 FT spec 的 "no excessive defensive coding"。is_null 检查对 hybrid group 的 null block padding 可能确实需要，保留可以，范围检查没必要。
• 已经 offload 到 CPU/disk 的脏 KV 召不回来——和 50283 一样的已知盲区，建议在 PR 描述里写明。
• PR 描述是空模板、没有 test plan、没有测试结果，按 AGENTS.md 的要求没法合。至少补一个单测："fault 后 resume 不命中脏 block" + "resume 后第一个 output frame 不被丢弃"。

建议的改法

在 preempt 循环之前，按请求驱逐尾部未确认的 block，顺便把 zero 做了：

for request in engine.scheduler.running:
    confirmed = request.num_computed_tokens - request.num_in_flight_tokens
    start = confirmed - (confirmed % block_size)  # 向下对齐到 block 边界
    if start < request.num_computed_tokens:
        kv_manager.record_blocks_for_zeroing(request.request_id, start)
        suspect = kv_manager.get_zeroing_block_ids_in_range(
            request.request_id, start, request.num_computed_tokens)
        batch_block_ids.update(suspect)

效果：每个请求只驱逐尾部 0~1 个脏 full block，干净的前缀全部保留，resume 后命中自己的旧 block、只重算尾部。

不建议为了"简洁"直接调 reset_prefix_cache(reset_running_requests=True)——它会错误地设置 async_tokens_to_discard（上面第 3 条），而且清空的是整个 prefix cache，驱逐范围比现在还大。

和 upstream PR 50283 的关系

	50283 (NaN FT)	本 PR (fault retry)
触发源	logits 里检测到 NaN	DP/EP 通信异常
处理	abort 单个请求	preempt 全部 running 请求
清理手段	evict 该请求的 block + 重分配时 zero（force_zeroing）	evict 全部 running 请求的 block，无 zero

• 两者的 evict 用的是同一个现成原语（KV-load-failure 路径的 evict_blocks），概念也是同一件事：脏 block 不能留在 prefix cache 里被后面的请求命中。
• 50283 的 force_zeroing（重分配时清零）正好就是本 PR 标题里声称、但代码里没做的那个 zero。如果两个 PR 都要往上游推，应该抽一个共用的"脏 block 清理"helper（evict + record-for-zeroing），别各写一套。
• 互不覆盖：50283 管不了 fault-retry 的 schedule-time-commit 污染，本 PR 也不做 NaN 检测。是互补关系，不是重复。

[fangyuchu]

清除evict当前请求的最后一个block的逻辑写成一个函数吧，不要在on_fault里实现

[fangyuchu]

就算是abort的情况，也只evict脏的kv就行吧，不需要清理所有的kv吧

[205150940]

已更改