Repository navigation
[OMNIML-6228] Give CI jobs and per-test timeouts headroom against slow runners - #2901
Closed
shengliangxu wants to merge 3 commits into
Closed
shengliangxu wants to merge 3 commits into
shengliangxu wants to merge 3 commits into
Conversation
…cap kills them A job cap close to the typical runtime turns every slow runner into a failed required check that only a manual re-run clears. Over 2026-10-06..10-20, the caps below killed these PR jobs (killed = cancelled within 1.5 min after the cap; minutes): lane old cap median p95 killed / finished new cap gpu 60 41.6 52 9 / 609 75 gpu_trtllm 15 6.5 10 34 / 715 30 regression 15 10.0 14 41 / 630 30 example llm_qat 30 19.3 25 9 / 615 45 example megatron (PR) 30 19.6 25 26 / 589 45 The kills follow the load on the runner fleet: all 34 gpu_trtllm kills and 37 of the 41 regression kills fall on 10-14..10-17, the days with the most PR pushes. For gpu_trtllm, 32 of the 34 hit during container initialization or after one that took more than 12 minutes: pulling the image takes 4 min at the median, while the test step takes 2 min. A slow job is not a hung one, and the per-test timeouts still cut a hung test long before any of these caps. The new caps are 1.8x to 4.6x the median. The nightly megatron example lane gets 90 min: its manually dispatched runs took up to 69 min against a cap of 75. The example torch lanes share one cap, so llm_qat's 45 min applies to the whole matrix. The unit jobs keep their 20 min, which CONTRIBUTING.md documents as the limit that keeps tests/unit fast. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
…m and example groups Besides test_mamba_search_space and test_overfits_a_single_batch, which are fixed on their own, 21 tests hit their per-test timeout in the CI runs of 2026-10-06..10-20: 10 in gpu (120 s), 5 in gpu_megatron (120 s), 5 in the examples (300 s) and 1 in gpu_vllm (60 s). Five of them passed on a re-run. The "slowest durations" of recent passing runs show how little room the slowest tests have before a slow runner pushes them over (call time, longest of the last 4 runs of each lane): test_qwen3_eval_fp8 (examples) 291 s of 300 s, median 228 s test_llama_eagle3[1-False] (examples) 233 s of 300 s test_serving_measurement_matches_production_autotuning (vllm) 49 s of 60 s test_quant_lora_distributed_checkpoint[1] (gpu_megatron) 88 s of 120 s test_unified_hf_export_and_check_safetensors (gpu) 66 s of 120 s (timed out 4 times) The new defaults are at least 2x the slowest of these: examples 600 s, gpu 180 s, gpu_megatron 180 s, gpu_vllm 120 s. A hung test is still cut there rather than by its job cap, and MODELOPT_QA_TEST_TIMEOUT still overrides the examples default. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
The 2-GPU runs of the lane (the nightly and the dispatched runs) take longer than the PR runs on the 1-GPU runner (about 42 min). Of the 17 runs of the standard lane on 2026-10-14..10-20 under the old 60 min cap, 8 were killed by it, including 4 of the 7 nightlies on main, and the 9 that finished took 45 to 58 min (median 51). With a median near 58 min over all 17, the 75 min cap from #2836 leaves 1.3x. The runs that did finish under the 75 min cap (10 dispatched runs on 10-20) took 42 to 60 min, but 10-14..10-17 pushed 8 of 17 runs past 60 min, so a loaded day can still reach 75. 90 min is 1.5x the typical 2-GPU run. PR runs finish in about 42 min and do not use it; a hung job is still cut earlier by the per-test timeout. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly summarizes the main change: increasing CI job and per-test timeouts to provide headroom on slow runners. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Security Anti-Patterns | |
The reviewed range changes only three workflow YAML files and tests/conftest.py. It does not change Python files under the modelopt package or examples, so none of the specified security anti-pa… |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (3 skipped: 3 unsupported.)
Full details: Security Anti-Patterns
-
Explanation
The reviewed range changes only three workflow YAML files and
tests/conftest.py. It does not change Python files under themodeloptpackage orexamples, so none of the specified security anti-patterns is introduced.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2901 +/- ##
==========================================
- Coverage 76.90% 76.90% -0.01%
==========================================
Files 658 659 +1
Lines 74029 74155 +126
==========================================
+ Hits 56932 57026 +94
- Misses 17097 17129 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Type of change: Bug fix (CI configuration)
Gives the GPU, regression and example jobs more room before a job cap kills them, and raises the default per-test timeout of four test groups. Both follow from 14 days of CI data (2026-10-06..10-20, about 4,000 workflow runs and 42,000 jobs; per-job timings and logs from the Actions API).
Job caps (
timeout-minutes). A cap close to the typical runtime turns every slow runner into a failed required check that only a manual re-run clears. PR runs lost these jobs to the cap (killed = cancelled within 1.5 min after the cap; minutes):The kills follow the load on the runner fleet, not the tests: all 34 gpu_trtllm kills and 37 of the 41 regression kills fall on 10-14..10-17, the days with the most PR pushes, and almost none after 10-17. For gpu_trtllm, 32 of the 34 hit during container initialization or after one that took more than 12 minutes: pulling the image takes 4 min at the median, while the test step takes 2 min. A slow job is not a hung one, and the per-test timeouts still cut a hung test long before any of these caps. The new caps are 1.8x to 4.6x the median. The nightly megatron example lane gets 90 min (was 75): its manually dispatched runs took up to 69 min.
The gpu_megatron lane gets 90 min (was 75 since #2836, 60 before) for its 2-GPU runs, the nightly and the dispatched ones, which take longer than the 1-GPU PR runs (about 42 min). Of the 17 runs of the lane on the 2-GPU runner on 10-14..10-20 under the old 60 min cap, 8 were killed by it, including 4 of the 7 nightlies on main; the 9 that finished took 45 to 58 min. The 10 runs that finished under a 75 min cap took 42 to 60 min, but 10-14..10-17 pushed 8 of 17 runs past 60 min, so a loaded day can still reach 75.
Not changed on purpose: the unit jobs keep their 20 min (CONTRIBUTING.md documents it as the limit that keeps
tests/unitfast; the xdist PR #2845 restores the headroom), and the vLLM cap (30 min) that #2836 already raised.Per-test timeouts (
_DEFAULT_TIMEOUTintests/conftest.py). Apart fromtest_mamba_search_spaceandtest_overfits_a_single_batch, which are fixed on their own, 21 tests hit their per-test timeout in the same window: 10 in gpu (120 s), 5 in gpu_megatron (120 s), 5 in the examples (300 s) and 1 in gpu_vllm (60 s); 5 of them passed on a re-run. The "slowest durations" of recent passing runs (longestcalltime of the last 4 runs of each lane) show how little room the slowest tests have:test_qwen3_eval_fp8(examples)test_llama_eagle3[1-False](examples)test_serving_measurement_matches_production_autotuning(gpu_vllm)test_quant_lora_distributed_checkpoint[1](gpu_megatron)test_unified_hf_export_and_check_safetensors(gpu)The new defaults are at least 2x the slowest of these: examples 600 s, gpu 180 s, gpu_megatron 180 s, gpu_vllm 120 s. A hung test is still cut by its own timeout rather than by the job cap, and
MODELOPT_QA_TEST_TIMEOUTstill overrides the examples default.Usage
N/A
Testing
Configuration only. The workflow YAML parses, the pre-commit hooks pass, and every number above comes from the Actions API (job and step start/end times, conclusions, and the "slowest durations" block of each lane's pytest log). This PR's own CI run cannot show the effect: a run that stays under a cap says nothing about the cap.
Before your PR is "Ready for review"
CONTRIBUTING.md: N/ASummary by CodeRabbit