Repository navigation
[CI/Build] Fix flaky failures in CPU CI image building - #55317
bigPYJ1151 merged 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 9d2bebfe6dc65418a5b1699468efd081ae446f35 and 7df33c9. 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe CPU build workflow updates disk cleanup and Docker build retries. The CPU Dockerfile adds local-disk sccache support across three stages. The pooling test scopes logging to its server fixture, and the MLA cache comment clarifies tensor shapes. ChangesCPU build maintenance
Test and kernel maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change improves CPU CI build retries, caching, cleanup behavior, and test logging isolation without any identified current-head merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9d2bebf to
f2b6f92
Compare
CPU CI jobs were frequently failing fast on unrelated PRs due to transient DNS/connection errors during `docker build` (base image auth, rustup, rust toolchain, pypi/uv), which normally succeed on a manual rebuild. Retry only the build command with backoff when the output matches known transient-network signatures, leaving the actual test run (`docker run`) untouched so real test failures are never retried or masked. Signed-off-by: jiang1.li <jiang1.li@intel.com>
Local CPU test builds were losing their uv/cargo/apt package caches to docker builder prune whenever disk pressure hit, forcing full re-downloads on the next build. Exclude cache mounts (exec.cachemount) from the prune filter so only regular build-cache records get reclaimed, and raise the retention ceiling from 80GB to 200GB to reduce how often pruning kicks in. Signed-off-by: jiang1.li <jiang1.li@intel.com>
Build CPU image (image_build_cpu.sh) uses USE_SCCACHE with an S3 backend plus a registry-backed BuildKit cache, so it stays fast across agents. The CPU test shards' embedded docker build (run-cpu-test.sh) used neither, rebuilding Rust and C++ from scratch on every run even though the same agent had just compiled the identical source minutes earlier. Add SCCACHE_LOCAL_ONLY, which redirects sccache to a local BuildKit cache mount instead of S3 when set, so it works without AWS credentials. Wire it into the rust-build-cache, vllm-build, and vllm-triton-cpu-build stages, and enable it by default in run-cpu-test.sh. Verified locally: a rebuild with one source file changed (so Docker's own layer cache misses) hit sccache for 485/485 C++ objects and 1692/1696 Rust objects, cutting those stages from ~240s/~283s to ~33s/~148s. The default (no USE_SCCACHE) and S3 (USE_SCCACHE without SCCACHE_LOCAL_ONLY) paths are unchanged. Signed-off-by: jiang1.li <jiang1.li@intel.com>
70% was triggering builder prune too eagerly relative to the new 200GB retention ceiling, discarding warm cache before it was actually needed. Signed-off-by: jiang1.li <jiang1.li@intel.com>
Setting os.environ at module import time leaked into the whole pytest process once CPU generation/pooling/e2e tests were combined into a single sharded invocation (vllm-project#54751), since pytest imports every test file during collection regardless of -k/-m selection. This silently downgraded EngineCore/Worker child-process logging to WARNING for the rest of that session, hiding their INFO-level startup logs in CI. Pass the override via RemoteOpenAIServer's env_dict instead, so it only affects the launched server subprocess. Signed-off-by: jiang1.li <jiang1.li@intel.com>
The RUN script in the vllm-triton-cpu-build stage chains git clone/checkout and uv build with plain ';' and no errexit, so a transient failure partway through (e.g. a network blip cloning triton-cpu) doesn't abort the script: it falls through to 'rm -rf triton-cpu' and exits 0. Docker/BuildKit then caches that layer as a success, with an empty dist/. Because that cache is local and persists across CI jobs, any agent that ever hits this once keeps silently falling back to the requirements/test/cpu.txt-pinned triton==3.6.0 instead of triton-cpu, with no error anywhere (confirmed against build #87100, where two different CPU CI agents had exactly this stale cache). Add 'set -ex' so a failure inside the build aborts the RUN and BuildKit correctly treats the layer as failed instead of caching it. Signed-off-by: jiang1.li <jiang1.li@intel.com>
|
/ci run |
|
✅ Triggered Buildkite CI #87235 for commit |
|
/ci run |
|
✅ Triggered Buildkite CI #87241 for commit |
4c2ec7b to
7df33c9
Compare
|
/ci run |
|
✅ Triggered Buildkite CI #87243 for commit |
The docstring above concat_and_cache_mla_cpu got mangled across lines by a previous clang-format pass, splitting field names mid-sentence. Signed-off-by: jiang1.li <jiang1.li@intel.com>
…55317) Signed-off-by: jiang1.li <jiang1.li@intel.com> Signed-off-by: Jyotirmoy Roy <jyotirmoyroy649@gmail.com>
Summary
VLLM_LOGGING_LEVEL=WARNINGoverride to just its own server subprocess (viaRemoteOpenAIServer'senv_dict) instead of mutatingos.environat module import time. That mutation leaked into the whole pytest process oncetests/models/language/{generation,pooling}and the CPU e2e test got combined into a single sharded invocation ([CI] Shard CPU jobs above the 24h P90 threshold #54751), since pytest imports every test file during collection regardless of-k/-mselection — silently hiding EngineCore/Worker INFO-level startup logs in CI for every test that ran afterward in that session.vllm-triton-cpu-buildDocker stage's RUN step on any internal failure (set -ex) instead of letting a transient error (e.g. a network blip cloning triton-cpu) fall through torm -rf triton-cpu; exit 0. Previously, a single such failure got permanently cached by BuildKit as a "successful" emptydist/, so every future CI job scheduled on that same agent silently fell back torequirements/test/cpu.txt'striton==3.6.0pin instead of triton-cpu, with no error anywhere — reproduced against build #87100, where two independent CPU CI agents had exactly this stale cache.Why this doesn't duplicate an open PR
Searched vllm-project/vllm open/closed PRs for each change (disk-pressure prune threshold, BuildKit cache mounts, local-disk sccache, retry CPU image build, triton-cpu build cache,
VLLM_LOGGING_LEVEL) — no existing PR addresses any of these.Test plan
test_max_tokens_per_doc.pylog-leak fix: reproduced the log-suppression bug live in a locally built CPU test Docker image by running the exact pre-/post-[CI] Shard CPU jobs above the 24h P90 threshold #54751 pytest invocations (pytest tests/models/language/generation tests/models/language/pooling tests/v1/e2e/test_cpu_linear_attn_chunked_prefix.py -m cpu_model --num-shards=N --shard-id=i, filtered tofacebook/opt-125mto keep it fast). Confirmed EngineCore/Worker INFO logs were absent (0 occurrences) in shards that exercised the reranker tests before the fix, and present after, matching unaffected shards' behavior.docker/Dockerfile.cpuset -exfix: built thevllm-triton-cpu-buildstage in isolation twice.docker build --target vllm-triton-cpu-build --build-arg USE_SCCACHE=1 --build-arg SCCACHE_LOCAL_ONLY=1 -f docker/Dockerfile.cpu .): succeeds, producestriton-3.7.0+git270e696d-...whlexactly as before.git checkoutref pointed at a nonexistent ref): before the fix this exits 0 and caches an emptydist/; after the fix,docker buildfails with exit code 1 and a clear error pointing at the failed RUN step.AI assistance disclosure
The root-cause analysis (bisecting the log-suppression bug across shard/test-file combinations, and finding the missing errexit in the triton-cpu build stage) and the fixes above were developed with AI assistance (Claude Code). I reviewed every changed line and ran the test/build commands listed above myself before opening this PR.