Skip to content

Load balance the test file timings across N workers instead of applying a bin packing algorithm #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 22, 2025

Conversation

kylekthompson
Copy link
Member

@kylekthompson kylekthompson commented Apr 21, 2025

Targeting total runtime / partitions for bin capacity can lead to many scenarios where you have empty bins. This is not ideal for test partitioning because we want to distribute the tests across a specified number of workers as evenly as possible. Notably, we don't want to try to fill bins as full as possible leaving any empty.

Instead of bin packing, this problem more closely relates to load balancing. A simple "shortest queue first" approach works very well and ensures all partitions get tests (so long as there are more files than partitions). In this case, queue depth is the total runtime we've added so far.

@kylekthompson kylekthompson self-assigned this Apr 21, 2025
@kylekthompson kylekthompson marked this pull request as draft April 21, 2025 21:07
tonywok
tonywok previously approved these changes Apr 21, 2025
@@ -24,6 +23,5 @@ func (p TestPartition) AddFilePath(filepath string) TestPartition {
}

func (p TestPartition) String() string {
percent := 100 - (float64(p.RemainingCapacity) / float64(p.TotalCapacity) * 100)
return fmt.Sprintf("[PART %d (%0.2f)]", p.Index, percent)
return fmt.Sprintf("[PART %d (%0.2fs)]", p.Index, p.Runtime.Seconds())
Copy link
Member

Choose a reason for hiding this comment

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

Ah, was wondering why these all turned to 0.00s in the tests. They are just too small.

@kylekthompson kylekthompson marked this pull request as ready for review April 22, 2025 12:58
@kylekthompson kylekthompson merged commit 68af0ce into main Apr 22, 2025
1 check passed
@kylekthompson kylekthompson deleted the kt/fix-partitioning branch April 22, 2025 13:01
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.

3 participants