Skip to content

[CI/Build] Fix CPU coverage collection with multiple workers - #9785

Merged
NickCao merged 2 commits into
vllm-project:mainfrom
LinzeShi:fix/cpu-coverage-xdist-collection
Nov 3, 2026
Merged

NickCao merged 2 commits into
vllm-project:mainfrom
LinzeShi:fix/cpu-coverage-xdist-collection

Conversation

@LinzeShi

@LinzeShi LinzeShi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Contributor intent

1. What outcome do you want to achieve?

Remove two collection failures blocking the nightly CPU coverage job.

2. What is the highest-leverage variable for that outcome?

Consistent test IDs and complete generated configs across pytest workers.

3. How will this work move that variable?

Give the random SHM parameter stable IDs and publish CI overlays with an atomic file replacement.

4. Whose customer or user needs does this address, and what are those needs?

Contributors and CI maintainers need the parallel CPU suite to reach test execution and produce meaningful coverage.

Purpose

Fixes #9784.

Nightly build 18653 fails during collection after the switch to four workers in #9392.

  • Keep the random, speech-shaped SHM key, but give its parameter a stable pytest ID so workers agree on the test list.
  • Write each CI overlay to a unique temporary file in the destination directory, close it, then replace the published path. Concurrent readers cannot see the truncated file.
  • Add one regression that checks readers keep seeing the complete old config during a rewrite, then see the complete new config with no temporary file left behind.

Only tests and their config helper change. Runtime behavior, configuration values and user documentation are unchanged.

Test Plan

vLLM Version: 0.32.0; Python 3.12, PyTorch 2.13.0+cu130, pytest 9.1.1, pytest-xdist 3.8.0 and PyYAML 6.0.3. CPU tests only, with GPU visibility and network access disabled.

vLLM-Omni Commit: base 7afd00e982dfa93c365eee5475a5707904b181e6; fix 8396d360678b4a691fbe6d9577803fdce6a51edd.

With the repository test dependencies installed, run from its root:

export CUDA_VISIBLE_DEVICES='' VLLM_TARGET_DEVICE=cpu VLLM_PLUGINS=''
export HF_HUB_OFFLINE=1 TRANSFORMERS_OFFLINE=1 HF_DATASETS_OFFLINE=1
export OMP_NUM_THREADS=2 MKL_NUM_THREADS=2 OPENBLAS_NUM_THREADS=2

python -m pytest tests/helpers/tests/test_ci_overlay.py \
  -m 'core_model and cpu' --run-level core_model -q

python -m pytest tests/distributed/omni_connectors/test_shm_connector.py \
  tests/helpers/tests/test_ci_overlay.py \
  -m 'core_model and cpu' --run-level core_model -n 4 --dist loadgroup -q

Test Result

  • Before: the SHM file produces three xdist collection errors; the retained overlay regression fails against the original helper.
  • After: the two focused files pass under four workers: 47 passed.
  • Four-process overlay probe, 800 reads: 3 failures before, 0 after. No temporary files remain after completion.
  • Changed-file pre-commit checks pass, including Ruff, mypy, test markers and source policies.

The full nightly suite and Buildkite job were not rerun; this validates the two reproduced collection failures, not the entire coverage job.

cc @yenuo26, this follows up on #9392.

Signed-off-by: Linze-Shi <linzeshi0@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-03T00:38:00.093564Z
We couldn’t complete this review. Comment @codex review to try again.
8396d36 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@yenuo26 yenuo26 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Fixes the two nightly CPU-coverage xdist collection failures from #9784 / #9392: stable pytest IDs for the speech-shaped SHM key, and same-dir mkstemp + os.replace (+ finally cleanup) for CI overlay publication. Focused regression covers readers still seeing the previous complete YAML during rewrite and a complete published overlay with no leftover temps. Placement under tests/helpers/tests/ is correct; the final == _CI_OVERLAYS["bagel"] check is a publish integrity round-trip, not redundant config-literal pinning.

Blocking (P0)

None.

Blocking (P1)

None.

Non-blocking

  1. test_shm_connector.py — please add a short comment above the ids=["simple", "speech"] parametrize. The stable IDs are the load-bearing xdist invariant (workers must agree on node IDs while the speech key value stays intentionally random per process). Without that note, the ids= look cosmetic and are easy to drop in a later cleanup. One or two lines is enough.

Signed-off-by: Linze-Shi <linzeshi0@gmail.com>

LinzeShi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, added a comment in ef62294bc explaining why the IDs must stay stable across xdist workers.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR was classified as CI work.

CI owner: @yenuo26 @NickCao

Routing: @yenuo26 via CI owner, CODEOWNERS; @NickCao via CODEOWNERS

@LinzeShi, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

@yenuo26 yenuo26 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Tip-move re-check of ef62294bcd8a (from 8396d360678b). Delta is the short comment above ids=["simple", "speech"] explaining the xdist-stable node-ID invariant — addresses the prior Non-blocking ask. Overlay atomic publish (mkstemp/os.replace + finally cleanup) and tests/helpers/tests/test_ci_overlay.py regression unchanged.

No blocking P0/P1.

LGTM from a review standpoint — I am not approving from this account; please self-approve / wait for a human approve.

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit ef62294bcd8a produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

LinzeShi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Self-review: checked the stable IDs and atomic file replacement. The focused run passed 47 tests on four workers, and the regression fails with the old helper. The follow-up only adds the requested comment. @NickCao, could you take a final look?

@NickCao NickCao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correct fix for the issue, ideally we should make every worker use their private copy of config dir, but that's a larger refactor.

@NickCao NickCao added merge-test label to trigger buildkite merge test CI ready label to trigger buildkite CI labels Nov 3, 2026
@NickCao
NickCao merged commit 74b022e into vllm-project:main Nov 3, 2026
8 of 11 checks passed
wuchun6661 pushed a commit to wuchun6661/vllm-omni that referenced this pull request Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-test label to trigger buildkite merge test CI ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Nightly CPU coverage fails during parallel test collection

4 participants