Skip to content

[Cherry-Pick][BugFix] Fix bugs in /v1/abort_requests interface from PR(#6992) (#7176)#7551

Merged
Jiang-Jia-Jun merged 3 commits into
PaddlePaddle:release/2.6from
qwes5s5:release/2.6-cp
Apr 22, 2026
Merged

[Cherry-Pick][BugFix] Fix bugs in /v1/abort_requests interface from PR(#6992) (#7176)#7551
Jiang-Jia-Jun merged 3 commits into
PaddlePaddle:release/2.6from
qwes5s5:release/2.6-cp

Conversation

@qwes5s5
Copy link
Copy Markdown
Collaborator

@qwes5s5 qwes5s5 commented Apr 22, 2026

Motivation

💡 If this PR is a Cherry Pick, the PR title needs to follow the format by adding the [Cherry-Pick] label at the very beginning and appending the original PR ID at the end. For example, [Cherry-Pick][CI] Add check trigger and logic(#5191)

💡 如若此PR是Cherry Pick,PR标题需遵循格式,在最开始加上[Cherry-Pick]标签,以及最后面加上原PR ID,例如[Cherry-Pick][CI] Add check trigger and logic(#5191)

Modifications

Usage or Command

Accuracy Tests

Checklist

  • Add at least a tag in the PR title.
    • Tag list: [[FDConfig],[APIServer],[Engine], [Scheduler], [PD Disaggregation], [Executor], [Graph Optimization], [Speculative Decoding], [RL], [Models], [Quantization], [Loader], [OP], [KVCache], [DataProcessor], [BugFix], [Docs], [CI], [Optimization], [Feature], [Benchmark], [Others], [XPU], [HPU], [GCU], [DCU], [Iluvatar], [Metax]]
    • You can add new tags based on the PR content, but the semantics must be clear.
  • Format your code, run pre-commit before commit.
  • Add unit tests. Please write the reason in this PR if no unit tests.
  • Provide accuracy results.
  • If the current PR is submitting to the release branch, make sure the PR has been submitted to the develop branch, then cherry-pick it to the release branch with the [Cherry-Pick] PR tag.

@paddle-bot
Copy link
Copy Markdown

paddle-bot Bot commented Apr 22, 2026

Thanks for your contribution!

@qwes5s5 qwes5s5 requested a review from Jiang-Jia-Jun April 22, 2026 03:21
Copy link
Copy Markdown

@PaddlePaddle-bot PaddlePaddle-bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Code Review | 2026-04-22 11:37:08

📋 Review 摘要

PR 概述:修复 /v1/abort_requests 接口中的多个 Bug,包括 EOS token 访问方式、abort 等待逻辑中的竞态处理以及 finish_reason 枚举值缺失问题。
变更范围engine/common_engine.pyengine/sched/resource_manager_v1.pyentrypoints/openai/protocol.pytests/engine/test_common_engine.py
影响面 Tag[Engine] [APIServer] [Scheduler]

📝 PR 规范检查

PR 描述中 Motivation / Modifications 均未填写,无法从 PR body 了解具体 Bug 背景和变更影响面,请补充以下内容:

Motivation 建议(可直接复制):

## Motivation
修复 `/v1/abort_requests` 接口中的多个 Bug:
1. `_control_abort_requests` 通过 `self.data_processor.eos_token_ids[0]` 获取 EOS token 在某些情况下会抛出 AttributeError;
2. `_wait_abort_complete` 在请求已自然结束时仍等待 abort 完成,导致不必要的超时;
3. `recycle_abort_task` / `_trigger_abort` 中使用 `set.remove` 在 key 不存在时抛出 KeyError;
4. `finish_requests` 未清理 abort 相关集合,可能导致后续误判;
5. OpenAI 协议中 `finish_reason` 缺少 `"abort"` 枚举值导致 Pydantic 校验报错。

## Modifications
- 使用 `getattr(request, "eos_token_ids", [0])` 安全获取 EOS token;
- `_wait_abort_complete` 入口及循环内增加"已完成请求"快速跳过逻辑;
- 将 `set.remove` 替换为 `set.discard` 避免 KeyError;
- `finish_requests` 中同步清理 abort 相关集合;
- 四个 Response/Choice 模型中补充 `"abort"` 枚举值;
- 更新相关单元测试以匹配新逻辑。

问题

级别 文件 概述
🟡 建议 common_engine.py:1590 getattr 默认值 [0] 使用魔法数字,建议添加注释或 warning 日志说明 fallback 原因
❓ 疑问 common_engine.py:1641 循环内无锁读取 alive_reqs 并直接操作 abort 集合,线程安全性待确认

总体评价

本 PR 修复了 abort 接口中多处潜在的 KeyError 和属性访问异常,逻辑方向正确,测试也同步更新。核心逻辑中的无锁并发访问点建议作者确认线程安全性,同时 PR 描述需补充 Motivation/Modifications 以满足项目规范。

Comment thread fastdeploy/engine/common_engine.py
Comment thread fastdeploy/engine/common_engine.py
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (release/2.6@b0fde16). Learn more about missing BASE report.

Files with missing lines Patch % Lines
fastdeploy/engine/common_engine.py 40.00% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##             release/2.6    #7551   +/-   ##
==============================================
  Coverage               ?   73.75%           
==============================================
  Files                  ?      376           
  Lines                  ?    53082           
  Branches               ?     8298           
==============================================
  Hits                   ?    39152           
  Misses                 ?    11184           
  Partials               ?     2746           
Flag Coverage Δ
GPU 73.75% <66.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qwes5s5
Copy link
Copy Markdown
Collaborator Author

qwes5s5 commented Apr 22, 2026

/re-run stable_tests

@Jiang-Jia-Jun Jiang-Jia-Jun merged commit 9c91ecb into PaddlePaddle:release/2.6 Apr 22, 2026
48 of 55 checks passed
@qwes5s5 qwes5s5 deleted the release/2.6-cp branch April 28, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants