Skip to content

test: separate host runtime waiter exit from lock deadline - #8370

Draft
andyluo7 wants to merge 1 commit into
vllm-project:mainfrom
andyluo7:fix/host-runtime-waiter-shutdown-20261001
Draft

andyluo7 wants to merge 1 commit into
vllm-project:mainfrom
andyluo7:fix/host-runtime-waiter-shutdown-20261001

Conversation

@andyluo7

@andyluo7 andyluo7 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

The domain-lock recovery test can fail after it has received the expected timeout report because it allows only five seconds for the spawned helper to exit. AMD #13103 at integration commit edc0e5144fe7e6b6b8ee4bccdce8518b3efadbeb reached that exit assertion with process.exitcode=None; its other 4,649 tests passed.

Give the helper the same 60-second process-completion budget already used by the neighboring filesystem-store multiprocessing tests. Keep the tested 0.1-second lock deadline, five-second result-response bound, both required/preferred policies, typed retryable domain error, owner-lock protection, metadata preservation and fresh-runtime recovery assertions. Keep forced cleanup bounded to five seconds and explicitly fail if the child survives. Failure messages include the PID and exit status.

This test is byte-identical between main and the failing integration before this change. The log proves that all policy/lock assertions before the join passed; it does not identify why process exit exceeded five seconds. This change separates the exit allowance from the runtime coordination contract without changing production behavior.

Validation: all changed-file pre-commit checks, including mypy and marker validation, pass. An AST comparison verifies all 17 existing semantic/exit assertions are preserved and one cleanup assertion is added. A real spawned-process control returns its result, remains alive after a five-second join, then exits zero within the completion allowance; every child/pipe is collected. This local control tests process handling, not Torch or ROCm. Exact-head runtime evidence now confirms both parameters, including fresh-runtime recovery, pass in AMD source #13106 and CUDA source #16493. AMD integration #13107 READY and MERGE copies also pass both parameters; each completes 4,650 tests, 68 skips, three subtests, exit0/nonsoft in637.31/645.33s. Source AMD Simple Other passes4,568/68skips/646.09s; CUDA passes4,607/34skips/586.58s. This lane uploads no artifacts; qualification uses terminal logs plus authoritative job exit metadata.

Standalone AMD aggregate failures are separate unchanged-main AuK graph expectations (two failures) and Breeze graph/RNG expectations (five failures), addressed by existing #8342 and #8201 included in the integration. They do not arise in this changed test. Full source/platform checks and human approval remain; this PR stays draft until review evidence is complete.

Current runtime evidence

Both actual recovery-policy parameters pass on AMD #13106 Simple Other (4568 passed, 68 skipped, three subtests, 646.09s) and CUDA #16493 Simple Other (4607 passed, 34 skipped, three subtests, 586.58s), exit0/nonsoft. CUDA source completes 18/18. Both d926 integration READY/MERGE Simple Other copies execute and pass the repair (4650 passed/68 skipped each).

AMD #13106 completes 22/27 actual passes with four failures and one timeout. Terminal logs identify Ulysses invalid device ordinal/AuK replay on shard2, AuK replay on shard3 and Diffusion Model, Breeze graph-cache/RNG assertions in R2-01, and shard4 timeout at test_tiled_decode_matches_the_whole_decode. These match existing #8165/#8342/#8201 repairs included in the integration. Source aggregate red does not establish a new host-runtime regression. Current-main reconciliation, review and human approval remain separate gates.

Signed-off-by: andyluo7 <andy.luo@amd.com>
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

@andyluo7 this PR is labeled ready + high priority, but CI is failing on the latest commit:

Could you please take a look and push an update to get CI green? Once the checks pass we can proceed with review/merge. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants