Repository navigation
[Bugfix] Log CRIU failure details before snapshot cleanup - #59661
Merged
simon-mo merged 2 commits intoOct 3, 2026
Merged
Conversation
Signed-off-by: Nils Matteson <nilsmatteson@icloud.com>
matteso1
requested review from
AndreasKaratzas,
DarkLight1337,
NickLucche,
aarnphm and
robertgshaw2-redhat
as code owners
October 1, 2026 18:03
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This was referenced Oct 1, 2026
simon-mo
approved these changes
Oct 3, 2026
simon-mo
enabled auto-merge (squash)
October 3, 2026 01:21
github-actions
Bot
added
the
ready
Contributor
Author
|
/ci run --allow-stale |
|
✅ Triggered Buildkite CI #92732 for commit
|
Signed-off-by: Nils Matteson <nilsmatteson@icloud.com>
Contributor
Author
|
/ci run |
|
✅ Triggered Buildkite CI #92734 for commit |
Collaborator
CI selector (shadow): 6 test steps (13 jobs) instead of 33 (46 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (6)
Would skip (today's rules run them) (31)
Would add (today's rules do not run them) (4)
AMD mirrors: would skip (30)
AMD mirrors: would add (0)none 2 changed files · base |
6 tasks done
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
3 participants
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.
Overview
Report CRIU failure details before snapshot cleanup removes the logs. This addresses the missing diagnostics in nightly build 92230, which reached
criu dumpafter #59487 corrected the image labels.Claims
Validation
python -m pytest tests/entrypoints/launchers/test_launch_cli.py -q -k 'criu_failure or preserves_bounded_criu_diagnostics or rolls_back_failed_dump': six passed on Mac and Linux CPU. The five new regression cases fail against the unchanged runtime; the existing cleanup test passes in both arms.A separate Linux check used a real failing subprocess in place of CRIU and called
create_snapshot. Its stdout, stderr and dump-log marker appeared in the console. The original exit-23 exception propagated and the incomplete artifact was removed. This checks the diagnostic path without requiring a GPU.All applicable pre-commit hooks passed, including Python 3.10 mypy and the Buildkite test-wiring check.
The full Mac test file has 99 passes and one environment-contract failure: CLI imports add
AITER_SITUV2_A4W4=1. The same failure reproduces on unchanged base083060d046a8. A broader Linux CPU run could not exercise CLI configuration cases that require a detected device; the focused diagnostics tests pass without one.Details
_runcaptures subprocess output, but the old CRIU path did not report that output or the CRIU log when the command failed. Creation then deleted the unpublished artifact. Collecting the file later in the CI exit trap would be too late.The error handler reads the last 8,192 bytes of
dump.logorrestore.logthrough the existing privilege helper, with a five-second timeout. Missing or unreadable logs are reported without replacing the original failure. Captured output is also bounded, and byte-valued timeout output is decoded with replacement.The cause of the nightly CRIU failure remains unknown. This patch makes the next attempt diagnosable; it does not claim to fix checkpoint compatibility. The regular snapshot job in build 92059 passed creation and both restores with identical snapshot source and E2E-driver bytes, on a different host and image.
Open-PR searches for snapshot, CRIU and #59487 found no duplicate diagnostic fix. #59487 changes image labels; this change reports runtime failures. AI assistance was used to prepare the change and its tests.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.