roachtest: harmonize GCE and AWS machine types by srosenberg · Pull Request #111140 · cockroachdb/cockroach

srosenberg · 2023-09-22T21:11:03Z

Previously, same (performance) roachtest executed in GCE and AWS
may have used a different memory (per CPU) multiplier and/or
cpu family, e.g., cascade lake vs ice lake. In the best case,
this resulted in different performance baselines on an otherwise
equivalent machine type. In the worst case, this resulted in OOMs
due to VMs in AWS having 2x less memory per CPU.

This change harmozines GCE and AWS machine types by making them
as isomorphic as possible, wrt memory, cpu family and price.
The following heuristics are used depending on specified MemPerCPU:
Standard yields 4GB/cpu, High yields 8GB/cpu,
Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu.
Low is supported only in GCE.
Consequently, n2-standard maps to m6i, n2-highmem maps to r6i,
n2-custom maps to c6i, modulo local SSDs in which case m6id is
used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake;
isomorphic AWS machine types are exclusively on Ice Lake.

Roachprod is extended to show cpu family and architecture on List.
Cost estimation now correctly deals with custom machine types.
Finally, we change the default zone allocation in GCE from exclusively
us-east1-b to ~25% us-central1-b and ~75% us-east1-b. This is
inteded to balance the quotas for local SSDs until we eventually
switch to PD-SSDs.

Epic: none
Fixes: #106570

Release note: None

cockroach-teamcity · 2023-09-22T21:11:11Z

This change is

srosenberg · 2023-09-24T02:29:50Z

CI smoke test in progress: SELECT_PROBABILITY=0.4

erikgrinaker

WIll the global Ice Lake bump affect roachperf benchmarks? If so, should we hold this until after the 23.2 branch cut and performance regression work, to give us a stable baseline for comparisons?

erikgrinaker · 2023-09-25T08:31:33Z

pkg/cmd/roachtest/cluster_test.go

 		{"n2-standard-32", 32},
 		{"n2-standard-64", 64},
 		{"n2-standard-96", 96},
+		// GCE machine types


Stray comment? There is already a comment above for "GCE machine types".

Thanks, removing.

srosenberg · 2023-09-25T12:59:13Z

WIll the global Ice Lake bump affect roachperf benchmarks? If so, should we hold this until after the 23.2 branch cut and performance regression work, to give us a stable baseline for comparisons?

It might have a small bump on the cpu-bound workloads using smaller vCPU density (those are most likely cascade lake, previous generation.) Do you really think it would be that disruptive to hold until after 23.2? I was just planning to add an annotation, since the other improvements seem to outweigh the small (cpu) gain.

erikgrinaker · 2023-09-25T13:07:03Z

If the effect would be small, them probably not. Otherwise, we'll have to directly compare the release roachperf graphs with each other to detect regressions, we can't simply look at the single graph for master -- but if we're ok with the extra manual work then that's fine too.

herkolategan

Reviewed 13 of 13 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @renatolabs, @smg260, and @srosenberg)

-- commits line 4 at r1:
silly nit: "some"

-- commits line 26 at r1:
silly nit: "intended"

srosenberg · 2023-09-25T15:54:10Z

If the effect would be small, them probably not. Otherwise, we'll have to directly compare the release roachperf graphs with each other to detect regressions, we can't simply look at the single graph for master -- but if we're ok with the extra manual work then that's fine too.

Right. My guess the effect is small, possibly negligible. Otherwise, I'll revert the change and postpone it until after branch cut.

renatolabs

Thanks for taking the time to make sure GCE and AWS are consistent!

renatolabs · 2023-09-26T17:21:10Z

pkg/cmd/roachtest/spec/machine_type.go

+
+		}
+		if shouldSupportLocalSSD {
+			family = family + "d"


Can we simplify this function by only having one check at the end for if shouldSupportLocalSSH { family += "d" }?

renatolabs · 2023-09-26T17:24:27Z

pkg/roachprod/vm/gce/gcloud.go

 	}
 	MachineType string
+	// CPU platform corresponding to machine type; see https://cloud.google.com/compute/docs/cpu-platforms
+	CpuPlatform string


Nit: probably more idiomatic to call this CPUPlaform: https://github.com/golang/go/wiki/CodeReviewComments#initialisms (and also more consistent with CPUArch and CPUFamily).

pkg/roachprod/vm/vm.go

renatolabs · 2023-09-26T17:32:26Z

pkg/roachprod/vm/gce/gcloud.go

+			if rand.Float64() < 0.75 {
+				zones = []string{defaultZones[0]}
+			} else {
+				zones = []string{defaultZones[1]}


This won't be great if we get unlucky and run a test that takes large backups and stores them in our backup testing buckets, which are only in us-east1 (not multiregion).

srosenberg

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @herkolategan, @renatolabs, and @smg260)

-- commits line 4 at r1:

Previously, herkolategan (Herko Lategan) wrote…

silly nit: "some"

It's actually inteded (sic) to be same (sic), but I'll rephrase :) Basically, one roachtest executed in both clouds would have different multipliers.

-- commits line 26 at r1:

Previously, herkolategan (Herko Lategan) wrote…

silly nit: "intended"

Fixing, thanks.

pkg/cmd/roachtest/spec/machine_type.go line 115 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Can we simplify this function by only having one check at the end for if shouldSupportLocalSSH { family += "d" }?

Yep, good catch!

pkg/roachprod/vm/vm.go line 52 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Would be nice to include some examples of what kinds of inputs this is known to work well: e.g., GCE tools, binary detection tool, etc.

Yep, good idea!

pkg/roachprod/vm/gce/gcloud.go line 131 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

Nit: probably more idiomatic to call this CPUPlaform: https://github.com/golang/go/wiki/CodeReviewComments#initialisms (and also more consistent with CPUArch and CPUFamily).

I struggled with this one; CPUPlatform just hurts my eyes :) I'll comply with the more idiomatic convention.

pkg/roachprod/vm/gce/gcloud.go line 973 at r1 (raw file):

Previously, renatolabs (Renato Costa) wrote…

This won't be great if we get unlucky and run a test that takes large backups and stores them in our backup testing buckets, which are only in us-east1 (not multiregion).

Yep, I am taking this out; to be dealt with in #111371

renatolabs · 2023-09-27T20:57:09Z

pkg/cmd/roachtest/spec/machine_type.go

-	if family == "c7g" && size == "24xlarge" {
-		family = "c6id"
+	// There is no m7gd.24xlarge, fall back to (c|m|r)6id.24xlarge.
+	if family == "m7gd" && size == "24xlarge" {


This should be m7g and m6i below.

Good catch, I shouldn't have hurried!

…l revert This is a backport of the "merged" diff of the following PRs: roachtest: harmonize GCE and AWS machine types cockroachdb#111140 roachtest: revert harmonize GCE and AWS machine types cockroachdb#111633 Release justification: test-only code, keeping roachtest in sync. Epic: none Release note: None

renatolabs · 2023-11-15T15:04:02Z

circle back after the 23.2 branch is cut?

We should revisit this. It still causes the occasional test failure (e.g., #113279).

srosenberg · 2023-11-15T16:51:42Z

circle back after the 23.2 branch is cut?

We should revisit this. It still causes the occasional test failure (e.g., #113279).

Yep, I'll prepare a new PR. We can wait until 23.2.1 since it appears fairly imminent.

TBD [1] cockroachdb#111140 [2] cockroachdb#111633 Epic: none Fixes: cockroachdb#106570 Release note: None

Previously, same (performance) roachtest executed in GCE and AWS may have used a different memory (per CPU) multiplier and/or cpu family, e.g., cascade lake vs ice lake. In the best case, this resulted in different performance baselines on an otherwise equivalent machine type. In the worst case, this resulted in OOMs due to VMs in AWS having 2x less memory per CPU. This change harmozines GCE and AWS machine types by making them as isomorphic as possible, wrt memory, cpu family and price. The following heuristics are used depending on specified MemPerCPU: Standard yields 4GB/cpu, High yields 8GB/cpu, Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu. Low is supported only in GCE. Consequently, n2-standard maps to m6i, n2-highmem maps to r6i, n2-custom maps to c6i, modulo local SSDs in which case m6id is used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake; isomorphic AWS machine types are exclusively on Ice Lake. Roachprod is extended to show cpu family and architecture on List. Cost estimation now correctly deals with custom machine types. Note, this PR essentially resurrects [1], after it was reverted in [2]. Since [1], `SelectAzureMachineType` has been added. MemPerCPU is preserved across all three cloud providers. However, when mem is Auto (default) and cpus > 80, we switch to AMD Milan, both in GCE and AWS, but not Azure. (The latter doesn't support 2GB per AMD CPU.) For complete lists of machine types see `ExampleXXXMachineType`. [1] cockroachdb#111140 [2] cockroachdb#111633 Epic: none Fixes: cockroachdb#106570 Release note: None

117852: roachtest: harmonize GCE, AWS, Azure machine types r=renatolabs a=srosenberg Previously, same (performance) roachtest executed in GCE and AWS may have used a different memory (per CPU) multiplier and/or cpu family, e.g., cascade lake vs ice lake. In the best case, this resulted in different performance baselines on an otherwise equivalent machine type. In the worst case, this resulted in OOMs due to VMs in AWS having 2x less memory per CPU. This change harmozines GCE and AWS machine types by making them as isomorphic as possible, wrt memory, cpu family and price. The following heuristics are used depending on specified MemPerCPU: Standard yields 4GB/cpu, High yields 8GB/cpu, Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu. Low is supported only in GCE. Consequently, n2-standard maps to m6i, n2-highmem maps to r6i, n2-custom maps to c6i, modulo local SSDs in which case m6id is used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake; isomorphic AWS machine types are exclusively on Ice Lake. Roachprod is extended to show cpu family and architecture on List. Cost estimation now correctly deals with custom machine types. Note, this PR essentially resurrects [1], after it was reverted in [2]. Since [1], `SelectAzureMachineType` has been added. MemPerCPU is preserved across all three cloud providers. However, when mem is Auto (default) and cpus > 80, we switch to AMD Milan, both in GCE and AWS, but not Azure. (The latter doesn't support 2GB per AMD CPU.) For complete lists of machine types see `ExampleXXXMachineType`. [1] #111140 [2] #111633 Epic: none Fixes: #106570 Release note: None Co-authored-by: Stan Rosenberg <stan.rosenberg@gmail.com>

Previously, same (performance) roachtest executed in GCE and AWS may have used a different memory (per CPU) multiplier and/or cpu family, e.g., cascade lake vs ice lake. In the best case, this resulted in different performance baselines on an otherwise equivalent machine type. In the worst case, this resulted in OOMs due to VMs in AWS having 2x less memory per CPU. This change harmozines GCE and AWS machine types by making them as isomorphic as possible, wrt memory, cpu family and price. The following heuristics are used depending on specified MemPerCPU: Standard yields 4GB/cpu, High yields 8GB/cpu, Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu. Low is supported only in GCE. Consequently, n2-standard maps to m6i, n2-highmem maps to r6i, n2-custom maps to c6i, modulo local SSDs in which case m6id is used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake; isomorphic AWS machine types are exclusively on Ice Lake. Roachprod is extended to show cpu family and architecture on List. Cost estimation now correctly deals with custom machine types. Note, this PR essentially resurrects [1], after it was reverted in [2]. Since [1], `SelectAzureMachineType` has been added. MemPerCPU is preserved across all three cloud providers. However, when mem is Auto (default) and cpus > 80, we switch to AMD Milan, both in GCE and AWS, but not Azure. (The latter doesn't support 2GB per AMD CPU.) For complete lists of machine types see `ExampleXXXMachineType`. [1] cockroachdb#111140 [2] cockroachdb#111633 Epic: none Fixes: cockroachdb#106570 Release note: None

Previously, same (performance) roachtest executed in GCE and AWS may have used a different memory (per CPU) multiplier and/or cpu family, e.g., cascade lake vs ice lake. In the best case, this resulted in different performance baselines on an otherwise equivalent machine type. In the worst case, this resulted in OOMs due to VMs in AWS having 2x less memory per CPU. This change harmozines GCE and AWS machine types by making them as isomorphic as possible, wrt memory, cpu family and price. The following heuristics are used depending on specified MemPerCPU: Standard yields 4GB/cpu, High yields 8GB/cpu, Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu. Low is supported only in GCE. Consequently, n2-standard maps to m6i, n2-highmem maps to r6i, n2-custom maps to c6i, modulo local SSDs in which case m6id is used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake; isomorphic AWS machine types are exclusively on Ice Lake. Roachprod is extended to show cpu family and architecture on List. Cost estimation now correctly deals with custom machine types. Note, this PR essentially resurrects [1], after it was reverted in [2]. Since [1], `SelectAzureMachineType` has been added. MemPerCPU is preserved across all three cloud providers. However, when mem is Auto (default) and cpus > 80, we switch to AMD Milan, both in GCE and AWS, but not Azure. (The latter doesn't support 2GB per AMD CPU.) For complete lists of machine types see `ExampleXXXMachineType`. [1] #111140 [2] #111633 Epic: none Fixes: #106570 Release note: None

Previously, same (performance) roachtest executed in GCE and AWS may have used a different memory (per CPU) multiplier and/or cpu family, e.g., cascade lake vs ice lake. In the best case, this resulted in different performance baselines on an otherwise equivalent machine type. In the worst case, this resulted in OOMs due to VMs in AWS having 2x less memory per CPU. This change harmozines GCE and AWS machine types by making them as isomorphic as possible, wrt memory, cpu family and price. The following heuristics are used depending on specified MemPerCPU: Standard yields 4GB/cpu, High yields 8GB/cpu, Auto yields 4GB/cpu up to and including 16 vCPUs, then 2GB/cpu. Low is supported only in GCE. Consequently, n2-standard maps to m6i, n2-highmem maps to r6i, n2-custom maps to c6i, modulo local SSDs in which case m6id is used, etc. Note, we also force --gce-min-cpu-platform to Ice Lake; isomorphic AWS machine types are exclusively on Ice Lake. Roachprod is extended to show cpu family and architecture on List. Cost estimation now correctly deals with custom machine types. Note, this PR essentially resurrects [1], after it was reverted in [2]. Since [1], `SelectAzureMachineType` has been added. MemPerCPU is preserved across all three cloud providers. However, when mem is Auto (default) and cpus > 80, we switch to AMD Milan, both in GCE and AWS, but not Azure. (The latter doesn't support 2GB per AMD CPU.) For complete lists of machine types see `ExampleXXXMachineType`. [1] cockroachdb#111140 [2] cockroachdb#111633 Epic: none Fixes: cockroachdb#106570 Release note: None

srosenberg force-pushed the sr/roachtest_harmonize_machine_types branch 2 times, most recently from 4e356ad to e478b80 Compare September 24, 2023 00:38

srosenberg requested review from a team, herkolategan, renatolabs and smg260 and removed request for a team September 24, 2023 01:03

srosenberg force-pushed the sr/roachtest_harmonize_machine_types branch from e478b80 to d315ea8 Compare September 24, 2023 02:24

srosenberg marked this pull request as ready for review September 24, 2023 02:24

srosenberg requested a review from a team as a code owner September 24, 2023 02:24

srosenberg requested a review from erikgrinaker September 24, 2023 02:24

srosenberg added backport-22.2.x backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only labels Sep 24, 2023

srosenberg removed the request for review from a team September 24, 2023 02:27

srosenberg changed the title ~~roachtest: harmonize machine types~~ roachtest: harmonize GCE and AWS machine types Sep 24, 2023

erikgrinaker reviewed Sep 25, 2023

View reviewed changes

erikgrinaker approved these changes Sep 25, 2023

View reviewed changes

herkolategan approved these changes Sep 25, 2023

View reviewed changes

pav-kv mentioned this pull request Sep 25, 2023

roachtest: restore/tpce/8TB/aws/nodes=10/cpus=8 failed #110764

Closed

renatolabs reviewed Sep 26, 2023

View reviewed changes

srosenberg commented Sep 27, 2023

View reviewed changes

srosenberg force-pushed the sr/roachtest_harmonize_machine_types branch 2 times, most recently from 74bab9a to f89da86 Compare September 27, 2023 20:09

renatolabs reviewed Sep 27, 2023

View reviewed changes

srosenberg force-pushed the sr/roachtest_harmonize_machine_types branch from f89da86 to e506755 Compare September 27, 2023 23:06

RaduBerinde mentioned this pull request Nov 2, 2023

release-23.1: roachtest: harmonize GCE and AWS machine types + partial revert #113688

Merged

srosenberg mentioned this pull request Jan 17, 2024

roachtest: harmonize GCE, AWS, Azure machine types #117852

Merged

srosenberg added a commit to srosenberg/cockroach that referenced this pull request Jan 18, 2024

roachtest: harmonize GCE, AWS, Azure machine types

e86b049

TBD [1] cockroachdb#111140 [2] cockroachdb#111633 Epic: none Fixes: cockroachdb#106570 Release note: None

srosenberg mentioned this pull request Feb 13, 2024

release-23.1: roachtest: harmonize GCE, AWS, Azure machine types #119147

Merged

srosenberg mentioned this pull request Feb 14, 2024

release-23.2: roachtest: harmonize GCE, AWS, Azure machine types #119204

Merged

srosenberg mentioned this pull request Feb 15, 2024

release-22.2: roachtest: harmonize GCE, AWS, Azure machine types #119264

Merged

Conversation

srosenberg commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Sep 22, 2023

Uh oh!

srosenberg commented Sep 24, 2023

Uh oh!

erikgrinaker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erikgrinaker Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

srosenberg Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

srosenberg commented Sep 25, 2023

Uh oh!

erikgrinaker commented Sep 25, 2023

Uh oh!

herkolategan left a comment

Choose a reason for hiding this comment

Uh oh!

srosenberg commented Sep 25, 2023

Uh oh!

renatolabs left a comment

Choose a reason for hiding this comment

Uh oh!

renatolabs Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

renatolabs Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

renatolabs Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

srosenberg left a comment

Choose a reason for hiding this comment

Uh oh!

renatolabs Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

srosenberg Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

renatolabs commented Nov 15, 2023

Uh oh!

srosenberg commented Nov 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

srosenberg commented Sep 22, 2023 •

edited

Loading

erikgrinaker left a comment •

edited

Loading