[CI] Add retry for OOM/Kill in run_tests_with_coverage#7602
Conversation
|
/skip-ci ci_iluvatar |
|
Thanks for your contribution! |
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 AI Code Review |
2026-04-24 00:02:14
📋 Review 摘要
PR 概述：为 CI 测试运行阶段引入 OOM/Kill 重试机制，同时删除手动维护的高风险 OOM 测试串行白名单。
变更范围：scripts/coverage_run.sh（CI 脚本）
影响面 Tag：[CI]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🟡 建议 | coverage_run.sh:71 |
max_retries=3 使单测最坏耗时达 40 分钟 |
| ❓ 疑问 | coverage_run.sh:91 |
sleep 5 是否足以等待 GPU 显存回收 |
| ❓ 疑问 | 整体逻辑 | 删除串行列表后，并行 OOM 根因未根治（见下） |
❓ 删除高风险 OOM 串行列表的潜在风险
原始 Rule 5 将这些测试归为 multi_gpu，使其串行执行，根本上避免了并发显存竞争。删除后，这些测试将作为 single_gpu 并行运行：
- 若 OOM 根因是并发资源竞争（多个 Flash Attention / 高显存算子测试同时执行），重试不能解决根因，最终 4 次尝试均会 Killed，反而将 CI 耗时放大 4 倍。
- 建议在 CI 运行一段时间后，监控这批测试的 retry 触发频率，若某些测试频繁 retry 到 max，考虑通过限制并行度而非白名单的方式来控制并发。
总体评价
思路正确，用动态重试替代静态白名单是更可维护的方向。主要关注点是重试对 CI 总时长的影响，以及并行执行对原高风险测试是否仍有 OOM 压力，建议在实际 CI 运行后观察 retry 触发情况再决定是否需要进一步调整。
| @@ -97,14 +69,37 @@ run_test_with_logging() { | |||
| # Set FD_LOG_DIR to isolate logs for each test | |||
| export FD_LOG_DIR="$isolated_log_dir" | |||
|
|
|||
There was a problem hiding this comment.
🟡 建议 max_retries=3 意味着单测最多执行 4 次（1次原始 + 3次重试），加上每次 600s timeout，单个测试最坏耗时可达 40 分钟（4 × 600s）。
建议适当降低 max_retries 或 timeout 以控制 CI 总时长：
local max_retries=2 # 1 original + 2 retries = max 30min per test| retry_count=$((retry_count + 1)) | ||
| continue | ||
| fi | ||
|
|
There was a problem hiding this comment.
❓ 疑问 sleep 5 是否足够等待 GPU 显存回收？OOM Kill 后进程清理 + GPU 显存回收通常需要更长时间（有时 10-30s）。建议适当增加等待时长或动态检测显存状态，避免重试时因显存未完全释放再次触发 OOM。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7602 +/- ##
==========================================
Coverage ? 71.73%
==========================================
Files ? 419
Lines ? 57783
Branches ? 9060
==========================================
Hits ? 41449
Misses ? 13512
Partials ? 2822
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EmmonsCurse
changed the title
4 participants
Motivation
Some tests may intermittently fail due to
OOMor process kill issues, especially under constrained CI resources. Previously, a high-risk OOM test list was maintained to mitigate this, but it increases maintenance overhead.Introducing a retry mechanism provides a more robust and scalable solution to handle transient failures without excluding tests.
Modifications
Run Test With Loggingstage inrun_tests_with_coverageto handleOOM/Killfailures.Usage or Command
N/A
Accuracy Tests
N/A
Checklist
[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]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.