Skip to content

[CI] Stop requiring both API servers to record weight sync metrics - #59521

Merged
aoshen02 merged 2 commits into
vllm-project:mainfrom
ROCm:rocm/fix-weight-sync-metrics-flake
Oct 4, 2026
Merged

aoshen02 merged 2 commits into
vllm-project:mainfrom
ROCm:rocm/fix-weight-sync-metrics-flake

Conversation

@aarushjain29

@aarushjain29 aarushjain29 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

The test asserted that both API servers recorded a weight operation.

The API servers share one listening socket, so the kernel can send every connection to one server. It was sending all 64 syncs allowed by #59507 to one server and still failed.

Adding up metrics across processes is already covered by test_in_flight_is_summed_across_api_server_processes, so this drops the check and keeps the count and in-flight checks with a fixed 8 syncs.

Verified locally on MI300X: 6/6 passes, including a run where only one API server handled requests

@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.

@aarushjain29
aarushjain29 force-pushed the rocm/fix-weight-sync-metrics-flake branch from 5ae77da to 901be4a Compare October 1, 2026 00:23
@mergify

mergify Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @aarushjain29.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

test_weight_sync_metrics_aggregate_across_api_servers asserted that both
API servers recorded a weight operation. The API servers accept from one
shared listening socket, so the kernel can hand every connection to one
process: AMD CI nightly build 13954 sent all 64 syncs allowed by vllm-project#59507 to
the same server and still failed.

Which server handles a request is not something the test controls, and
summing across processes is already covered deterministically by
test_in_flight_is_summed_across_api_server_processes. Send a fixed 8 syncs
and keep the count and in-flight checks, which hold whichever server
records.

Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
@aarushjain29
aarushjain29 force-pushed the rocm/fix-weight-sync-metrics-flake branch from 901be4a to bc68c25 Compare October 1, 2026 16:29
@aarushjain29 aarushjain29 changed the title [CI][Test] Fix flaky weight sync metrics multi-API-server test [CI] Stop requiring both API servers to record weight sync metrics Oct 1, 2026
@mergify mergify Bot removed the needs-rebase label Oct 1, 2026
@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92457 for commit ce0db0a661dd.

@AndreasKaratzas

Copy link
Copy Markdown
Member

We tested this one 30 times in a loop cause we got again this one today:

cc @DarkLight1337

@aoshen02 aoshen02 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 4, 2026
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ @aarushjain29, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Your branch must contain every commit currently on its upstream target branch. Merge or rebase onto the latest target branch, then rerun the command. Append --allow-stale to a run command to test an outdated branch at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@aoshen02
aoshen02 enabled auto-merge (squash) October 4, 2026 14:10
@aoshen02
aoshen02 merged commit bdd31c3 into vllm-project:main Oct 4, 2026
27 of 28 checks passed
@AndreasKaratzas
AndreasKaratzas deleted the rocm/fix-weight-sync-metrics-flake branch October 4, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants