Skip to content

[DNM][CI bisect] Revert #58197 to identify FT E2E detection hang - #58318

Closed
vllm-agent wants to merge 2 commits into
vllm-project:mainfrom
vllm-agent:bisect-ft-revert-58197
Closed

vllm-agent wants to merge 2 commits into
vllm-project:mainfrom
vllm-agent:bisect-ft-revert-58197

Conversation

@vllm-agent

Copy link
Copy Markdown
Contributor

Bisecting the main-branch Fault Tolerance E2E hang (rank 0 never reports UNHEALTHY after injected fault; broke between daily #90513 and #90560).

This PR reverts #58197 (disable JIT warmup in eager mode — the FT lane runs --enforce-eager). Do not merge — bisect probe only; will be closed once the culprit is identified. Sibling probe: #58317 (revert #55891).

@khluu

khluu commented Sep 23, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90636 for commit d543e065c0cf.

@khluu

khluu commented Sep 23, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90788 for commit 9debc2f83492.

@khluu

khluu commented Sep 23, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

❌ This PR is 2 commits behind upstream main. Your branch must contain every commit currently on upstream main. No new CI build was started. Merge or rebase onto the latest main, then rerun /ci run. To test this branch at your own risk, use /ci run --allow-stale.

@khluu

khluu commented Sep 23, 2026

Copy link
Copy Markdown
Member

/ci run

@vllm-agent
vllm-agent force-pushed the bisect-ft-revert-58197 branch from 063327d to c7b1e3a Compare September 23, 2026 23:14
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90793 for commit c7b1e3a84384.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@yewentao256 yewentao256 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CC @mgoin

@vllm-agent

Copy link
Copy Markdown
Contributor Author

Context for a forward fix (per @khluu's preference over this revert):

Empirically proven: reverting #58197 makes the FT lane pass (build #90793, 2/2 green); main fails deterministically since #58197 merged. So the trigger is specifically kernel_config.enable_jit_warmup=False under --enforce-eager.

What enable_jit_warmup=False gates (from code read): the main JIT warmup batch (vllm/model_executor/warmup/kernel_warmup.py:170), plus MLA (mla_attention.py:657) and sparse-indexer (sparse_attn_indexer.py:908) warmup paths.

Observed failure shape: in test_injected_fault_retry_recovers_all_ranks, the faulted engine (DP1) goes UNHEALTHY correctly; the survivor (DP0) shows NIXL-EP dispatch timeouts at ~30s but its busy-loop DP sync never raises, so it never reports UNHEALTHY within the deadline. The lane runs with --enforce-eager + 2×DP + nixl_ep.

Not yet pinned: which specific warmed kernel the survivor's fault-time path depends on (the failing log shows router-GEMM/FlashInfer warmups still ran — so it's specifically the gated batch that matters). A forward fix could be as narrow as: don't disable JIT warmup when parallel_config.enable_fault_tolerance is set — but worth confirming that's sufficient via the FT lane before landing.

@njhill

njhill commented Sep 24, 2026

Copy link
Copy Markdown
Member

I think we can just drop --enforce-eager from that test, I will open a PR for that

@njhill

njhill commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

I opened #58593 which should hopefully forward-fix the test, implementing the bot's suggestion.

A forward fix could be as narrow as: don't disable JIT warmup when parallel_config.enable_fault_tolerance is set — but worth confirming that's sufficient via the FT lane before landing.

@mgoin mgoin closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants