Skip to content
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

[DNM] Try two worker processes per host #915

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ntabris
Copy link
Member

@ntabris ntabris commented Jul 25, 2023

I'd like to see how this impacts perf of various benchmarks.

@@ -15,6 +15,7 @@ default:
package_sync: true
wait_for_workers: true
scheduler_vm_types: [m6i.large]
_n_worker_specs_per_host: 2
Copy link
Member

Choose a reason for hiding this comment

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

is this on prod now?

Copy link
Member

Choose a reason for hiding this comment

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

IIUC a fair assessment would require us to double the machine sizes and reduce cluster sizes by half. Otherwise the same-host workers have much less memory to work with and are much more likely to run into spilling and OOM

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Results of this A/B test are interesting but likely require a bit further analysis

image

We can see a couple of tests that regress significantly while others are getting much worse.

My best read without digging deeper is that most/almost all of our tests require a bit of spilling and that all test cases that are spilling behave really poorly, likely because disk is just busy?

@ntabris
Copy link
Member Author

ntabris commented Jul 26, 2023

@fjetter you want to try run with 1/2 the number of m6i.xlarge machines? want me to push that change to this branch?

@fjetter
Copy link
Member

fjetter commented Jul 26, 2023

@ntabris #917 is what I ran for the A/B tests. Clusters were using only half the number of VMs, VMs were twice as large, i.e. same total number of CPUs, same total RAM, half the number of VMs with two workers each

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