Skip to content

e2e/qa: pad short device lists with the last ready device - #4537

Open
ben-dz wants to merge 1 commit into
mainfrom
bdz/adhoc-421
Open

ben-dz wants to merge 1 commit into
mainfrom
bdz/adhoc-421

Conversation

@ben-dz

@ben-dz ben-dz commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

  • This PR makes AssignDevicesToClients pad a host's shorter device list with the last device in that list that can accept users (Ready()).
  • Before, it padded with the last device, even when that device had max_users=0.
  • The all-devices QA test then connected the host to that device again in every padded batch.
  • A host with no device that can accept users still pads with its last device.
  • In the mainnet-beta run at 20:00 UTC on 2026-11-14, aws-sin-mn-qa2 had dz-sg1-sw01 (max_users=0) last in its list and timed out on it 11 times.
  • Each status wait timed out after about 85 seconds, so batches 6 onward took 130 to 155 seconds instead of 50 to 65.
  • That run took about 37 minutes, against the usual 22 to 29.
  • DetermineClientsToConnect keeps a host connected when its device is the same as in the previous batch and its status is up.
  • So padded batches do not connect the host again while its status stays up.
  • The exception is a list that ends with a device that cannot accept users, where the first padded batch connects once to the ready device.
  • The ready device now gets the retests of the padded batches, and it passes if any one of its tests succeeds.

Testing Verification

  • A new TestAssignDevicesToClients case gives a host the list dev1, dev2, drained, and checks that both padded batches use dev2.
  • This case fails on main, where both padded batches use drained.
  • A second case gives a host only devices with max_users=0, and checks that the host still pads with its last device.
  • This change did not run against the QA hosts.
  • The effect on run time shows only after merge, in the next mainnet-beta all-devices runs.

@ben-dz
ben-dz requested review from a team and a balanced review from Copilot November 14, 2026 21:58

Copilot AI 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.

🟢 Approval recommended

No active review issues remain for the change to prefer ready devices when padding host assignments.

No open issues 🎉

What changed in this PR

This PR reduces repeated connection timeouts in the all-devices QA test by changing how hosts pad shorter device lists.

Changes:

  • AssignDevicesToClients now pads with the last ready device in the host’s assigned list.
  • Hosts with no ready device still pad with their last assigned device.
  • Two regression tests cover ready-device selection and the fallback when no device is ready.
File Description
e2e/​internal/​qa/​device_assignment.go Selects the last ready device for padding while preserving the fallback.
e2e/​internal/​qa/​device_assignment_test.go Adds regression cases for both padding paths.
CHANGELOG.md Documents the padding change and the QA timeout context.

Review effort: Balanced

This branch has not been deployed

No deployments
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.

2 participants