Skip to content

test(vllm): remove unsupported prefill migration test - #13692

Open
alec-flowers wants to merge 1 commit into
codex/vllm-agg-migrationfrom
codex/vllm-prefill-migration
Open

test(vllm): remove unsupported prefill migration test#13692
alec-flowers wants to merge 1 commit into
codex/vllm-agg-migrationfrom
codex/vllm-prefill-migration

Conversation

@alec-flowers

@alec-flowers alec-flowers commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove the permanently skipped vLLM prefill-migration test and its stale timing entry
  • remove 8 matrix nodes that described a capability Dynamo does not implement
  • keep the supported aggregated, KV-transfer, and decode migration tests unchanged

Support evidence and decision

This is intentionally a removal rather than an unskip:

  • Dynamo's Migration operator wraps the complete prefill-to-decode pipeline. On a migratable error, RetryManager clones and reissues the PreprocessedRequest through that pipeline.
  • The migrator accumulates generated response token IDs. During prefill there are no generated response tokens to preserve, and the vLLM prefill path exposes no checkpoint/export/restore mechanism for partially computed prefill KV state.
  • Consequently, a failure during prefill can at most restart and recompute the prompt on another prefill worker. That is request replay/new-request recovery, not migration of in-flight prefill state.
  • The test was introduced in test: Rewrite Request Migration Tests and Add Disagg Scenarios #5448 already marked xfail with Prefill migration not yet supported; it was later converted to an unconditional skip. It never represented a supported regression boundary.
  • The test's assertion only checked eventual request success, so even an xpass could not distinguish full recomputation from state migration.

A permanently skipped hypothetical test has no regression sensitivity and should not remain in the nightly denominator. If partial prefill-state transfer is implemented later, it needs a new test with an observable state-reuse contract.

Validation

  • exact nightly image collection before: 28 tests collected
  • exact nightly image collection after: 20 tests collected (8 unsupported prefill nodes removed)
  • python3 -m py_compile tests/fault_tolerance/migration/test_vllm.py
  • git diff --check
  • scoped pre-commit formatting, lint, spelling, and hygiene hooks passed
  • repository-wide pytest-marker-report reached the changed file and reported tests/fault_tolerance/migration/test_vllm.py: 20, then failed while collecting unrelated suites because the shared test port allocator was exhausted

No GPU run was needed: this change only deletes an unconditionally skipped test, and production-code inspection conclusively shows that its claimed state-transfer capability is absent.

Stack navigation

  1. #13684 — Aggregate migration
  2. #13692 — Prefill removal · DYN-4118
  3. #13693 — Decode migration · DYN-4119
  4. #13694 — KV-transfer migration · DYN-4120

This PR is layer 2 and targets codex/vllm-agg-migration. Review and merge the stack in order.


Open in Devin Review

@alec-flowers
alec-flowers requested a review from a team as a code owner August 23, 2026 16:17
@github-actions github-actions Bot added the test label Aug 23, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Signed-off-by: Alec Flowers <aflowers@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant