Merge #111140 #111442 #111475

111140: roachtest: harmonize GCE and AWS machine types r=erikgrinaker,herkolategan,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.
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

111442: server,kvcoord: change x-locality log from vevent to vinfo r=arulajmani a=kvoli

The X-locality log events were added in #104585 to the Node batch
receive path, to alert when localities were misconfigured. In some
clusters, especially test clusters, these events are unnecessarily
verbose in traces.

Change the log from `VEvent(5)` to `VInfo(5)` in the node batch path.

The X-locality log events were added in #103963 for the dist sender, to
alert when localities were misconfigured. In some clusters, especially
test clusters, these events are unnecessarily verbose in traces.

Change the log from `VEvent(5)` to `VInfo(5)` in the dist sender path.

Resolves: #110648
Epic: none
Release note: None

111475: server,settingswatcher: fix the local persisted cache r=stevendanna,aliher1911 a=knz

There's two commits here, fixing 2 separate issues.
Epic: CRDB-6671

### server,settingswatcher: properly evict entries from the local persisted cache

Fixes #70567.
Supersedes #101472.

(For context, on each node there is a local persisted cache of cluster
setting customizations. This exists to ensure that configured values
can be used even before a node has fully started up and can start
reading customizations from `system.settings`.)

Prior to this patch, entries were never evicted from the local
persisted cache: when a cluster setting was reset, any previously
saved entry in the cache would remain there.

This is a very old bug, which was long hidden and was recently
revealed when commit 2f5d717 was
merged. In a nutshell, before this recent commit the code responsible
to load the entries from the cache didn't fully work and so the stale
entries were never restored from the cache. That commit fixed the
loader code, and so the stale entries became active, which made the
old bug visible.

To fix the old bug, this present commit modifies the settings watcher
to preserve KV deletion events, and propagates them to the persisted
cache.

(There is no release note because there is no user-facing release where
the bug was visible.)

### settingswatcher: write-through to the persisted cache

Fixes #111422.
Fixes #111328.

Prior to this patch, the rangefeed watcher over `system.settings` was
updating the in-RAM value store before it propagated the updates to
the persisted local cache.

In fact, the update to the persisted local cache was lagging quite a
bit behind, because the rangefeed watcher would buffer updates and
only flush them after a while.

As a result, the following sequence was possible:

1. client updates a cluster setting.
2. server is immediately shut down. The persisted cache
   has not been updated yet.
3. server is restarted. For a short while (until the settings watcher
   has caught up), the old version of the setting remains active.

This recall of ghost values of a setting was simply a bug. This patch
fixes that, by ensuring that the persisted cache is written through
before the in-RAM value store.

By doing this, we give up on batching updates to the persisted local
store. This is deemed acceptable because cluster settings are not
updated frequently.



Co-authored-by: Stan Rosenberg <[email protected]>
Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>

pkg/cmd/roachprod/main.go

@@ -279,12 +279,22 @@ hosts file.
 			}
 		} else {
 			machineType := func(clusterVMs vm.List) string {
-				res := clusterVMs[0].MachineType
-				// Display CPU architecture, other than amd64 (default).
-				if arch := clusterVMs[0].Labels["arch"]; arch != "" && arch != string(vm.ArchAMD64) {
-					res += fmt.Sprintf(" [%s]", arch)
+				return clusterVMs[0].MachineType
+			}
+			cpuArch := func(clusterVMs vm.List) string {
+				// Display CPU architecture and family.
+				if clusterVMs[0].CPUArch == "" {
+					// N.B. Either a local cluster or unsupported cloud provider.
+					return ""
+				}
+				if clusterVMs[0].CPUFamily != "" {
+					return clusterVMs[0].CPUFamily
+				}
+				if clusterVMs[0].CPUArch != vm.ArchAMD64 {
+					return string(clusterVMs[0].CPUArch)
 				}
-				return res
+				// AMD64 is the default, so don't display it.
+				return ""
 			}
 			// Align columns left and separate with at least two spaces.
 			tw := tabwriter.NewWriter(os.Stdout, 0, 8, 2, ' ', tabwriter.AlignRight)
@@ -293,14 +303,14 @@ hosts file.
 			// [1] https://github.com/golang/go/issues/12073
 
 			// Print header.
-			fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t\n",
-				"Cluster", "Clouds", "Size", "VM",
+			fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t\n",
+				"Cluster", "Clouds", "Size", "VM", "Arch",
 				color.HiWhiteString("$/hour"), color.HiWhiteString("$ Spent"),
 				color.HiWhiteString("Uptime"), color.HiWhiteString("TTL"),
 				color.HiWhiteString("$/TTL"))
 			// Print separator.
-			fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t\n",
-				"", "", "",
+			fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t\n",
+				"", "", "", "",
 				color.HiWhiteString(""), color.HiWhiteString(""),
 				color.HiWhiteString(""), color.HiWhiteString(""),
 				color.HiWhiteString(""))
@@ -312,8 +322,8 @@ hosts file.
 				} else {
 					// N.B. Tabwriter doesn't support per-column alignment. It looks odd to have the cluster names right-aligned,
 					// so we make it left-aligned.
-					fmt.Fprintf(tw, "%s\t%s\t%d\t%s", name+strings.Repeat(" ", maxClusterName-len(name)), c.Clouds(),
-						len(c.VMs), machineType(c.VMs))
+					fmt.Fprintf(tw, "%s\t%s\t%d\t%s\t%s", name+strings.Repeat(" ", maxClusterName-len(name)), c.Clouds(),
+						len(c.VMs), machineType(c.VMs), cpuArch(c.VMs))
 					if !c.IsLocal() {
 						colorByCostBucket := func(cost float64) func(string, ...interface{}) string {
 							switch {

pkg/cmd/roachtest/cluster.go

@@ -597,6 +597,9 @@ func MachineTypeToCPUs(s string) int {
 		if _, err := fmt.Sscanf(s, "n2-highcpu-%d", &v); err == nil {
 			return v
 		}
+		if _, err := fmt.Sscanf(s, "n2-custom-%d", &v); err == nil {
+			return v
+		}
 		if _, err := fmt.Sscanf(s, "n2-highmem-%d", &v); err == nil {
 			return v
 		}
@@ -650,9 +653,7 @@ func MachineTypeToCPUs(s string) int {
 
 	// TODO(pbardea): Non-default Azure machine types are not supported
 	// and will return unknown machine type error.
-	fmt.Fprintf(os.Stderr, "unknown machine type: %s\n", s)
-	os.Exit(1)
-	return -1
+	panic(fmt.Sprintf("unknown machine type: %s\n", s))
 }
 
 type nodeSelector interface {

pkg/cmd/roachtest/cluster_test.go

@@ -11,6 +11,7 @@
 package main
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -173,6 +174,9 @@ func TestClusterMachineType(t *testing.T) {
 		{"n2-standard-32", 32},
 		{"n2-standard-64", 64},
 		{"n2-standard-96", 96},
+		{"n2-highmem-8", 8},
+		{"n2-highcpu-16-2048", 16},
+		{"n2-custom-32-65536", 32},
 		{"t2a-standard-2", 2},
 		{"t2a-standard-4", 4},
 		{"t2a-standard-8", 8},
@@ -190,6 +194,233 @@ func TestClusterMachineType(t *testing.T) {
 	}
 }
 
+type machineTypeTestCase struct {
+	cpus                int
+	mem                 spec.MemPerCPU
+	localSSD            bool
+	arch                vm.CPUArch
+	expectedMachineType string
+	expectedArch        vm.CPUArch
+}
+
+func TestAWSMachineType(t *testing.T) {
+	testCases := []machineTypeTestCase{}
+
+	xlarge := func(cpus int) string {
+		var size string
+		switch {
+		case cpus <= 2:
+			size = "large"
+		case cpus <= 4:
+			size = "xlarge"
+		case cpus <= 8:
+			size = "2xlarge"
+		case cpus <= 16:
+			size = "4xlarge"
+		case cpus <= 32:
+			size = "8xlarge"
+		case cpus <= 48:
+			size = "12xlarge"
+		case cpus <= 64:
+			size = "16xlarge"
+		case cpus <= 96:
+			size = "24xlarge"
+		default:
+			size = "24xlarge"
+		}
+		return size
+	}
+
+	addAMD := func(mem spec.MemPerCPU) {
+		family := func() string {
+			switch mem {
+			case spec.Auto:
+				return "m6i"
+			case spec.Standard:
+				return "m6i"
+			case spec.High:
+				return "r6i"
+			}
+			return ""
+		}
+
+		for _, arch := range []vm.CPUArch{vm.ArchAMD64, vm.ArchFIPS} {
+			family := family()
+
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, arch,
+				fmt.Sprintf("%s.%s", family, xlarge(1)), arch})
+			testCases = append(testCases, machineTypeTestCase{1, mem, true, arch,
+				fmt.Sprintf("%sd.%s", family, xlarge(1)), arch})
+			for i := 2; i <= 128; i += 2 {
+				if i > 16 && mem == spec.Auto {
+					family = "c6i"
+				}
+				testCases = append(testCases, machineTypeTestCase{i, mem, false, arch,
+					fmt.Sprintf("%s.%s", family, xlarge(i)), arch})
+				testCases = append(testCases, machineTypeTestCase{i, mem, true, arch,
+					fmt.Sprintf("%sd.%s", family, xlarge(i)), arch})
+			}
+		}
+	}
+	addARM := func(mem spec.MemPerCPU) {
+		fallback := false
+		var family string
+
+		switch mem {
+		case spec.Auto:
+			family = "m7g"
+		case spec.Standard:
+			family = "m7g"
+		case spec.High:
+			family = "r6i"
+			fallback = true
+		}
+
+		if fallback {
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, vm.ArchARM64,
+				fmt.Sprintf("%s.%s", family, xlarge(1)), vm.ArchAMD64})
+			testCases = append(testCases, machineTypeTestCase{1, mem, true, vm.ArchARM64,
+				fmt.Sprintf("%sd.%s", family, xlarge(1)), vm.ArchAMD64})
+		} else {
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, vm.ArchARM64,
+				fmt.Sprintf("%s.%s", family, xlarge(1)), vm.ArchARM64})
+			testCases = append(testCases, machineTypeTestCase{1, mem, true, vm.ArchARM64,
+				fmt.Sprintf("%sd.%s", family, xlarge(1)), vm.ArchARM64})
+		}
+		for i := 2; i <= 128; i += 2 {
+			if i > 16 && mem == spec.Auto {
+				family = "c7g"
+			}
+			fallback = fallback || i > 64
+
+			if fallback {
+				if mem == spec.Auto {
+					family = "c6i"
+				} else if mem == spec.Standard {
+					family = "m6i"
+				}
+				// Expect fallback to AMD64.
+				testCases = append(testCases, machineTypeTestCase{i, mem, false, vm.ArchARM64,
+					fmt.Sprintf("%s.%s", family, xlarge(i)), vm.ArchAMD64})
+				testCases = append(testCases, machineTypeTestCase{i, mem, true, vm.ArchARM64,
+					fmt.Sprintf("%sd.%s", family, xlarge(i)), vm.ArchAMD64})
+			} else {
+				testCases = append(testCases, machineTypeTestCase{i, mem, false, vm.ArchARM64,
+					fmt.Sprintf("%s.%s", family, xlarge(i)), vm.ArchARM64})
+				testCases = append(testCases, machineTypeTestCase{i, mem, true, vm.ArchARM64,
+					fmt.Sprintf("%sd.%s", family, xlarge(i)), vm.ArchARM64})
+			}
+		}
+	}
+	for _, mem := range []spec.MemPerCPU{spec.Auto, spec.Standard, spec.High} {
+		addAMD(mem)
+		addARM(mem)
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%d/%s/%t/%s", tc.cpus, tc.mem, tc.localSSD, tc.arch), func(t *testing.T) {
+			machineType, selectedArch := spec.AWSMachineType(tc.cpus, tc.mem, tc.localSSD, tc.arch)
+
+			require.Equal(t, tc.expectedMachineType, machineType)
+			require.Equal(t, tc.expectedArch, selectedArch)
+		})
+	}
+	// spec.Low is not supported.
+	require.Panics(t, func() { spec.AWSMachineType(4, spec.Low, false, vm.ArchAMD64) })
+	require.Panics(t, func() { spec.AWSMachineType(16, spec.Low, false, vm.ArchARM64) })
+}
+
+func TestGCEMachineType(t *testing.T) {
+	testCases := []machineTypeTestCase{}
+
+	addAMD := func(mem spec.MemPerCPU) {
+		series := func() string {
+			switch mem {
+			case spec.Auto:
+				return "standard"
+			case spec.Standard:
+				return "standard"
+			case spec.High:
+				return "highmem"
+			case spec.Low:
+				return "highcpu"
+			}
+			return ""
+		}
+
+		for _, arch := range []vm.CPUArch{vm.ArchAMD64, vm.ArchFIPS} {
+			series := series()
+
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, arch,
+				fmt.Sprintf("n2-%s-%d", series, 2), arch})
+			for i := 2; i <= 128; i += 2 {
+				if i > 16 && mem == spec.Auto {
+					// n2-custom with 2GB per CPU.
+					testCases = append(testCases, machineTypeTestCase{i, mem, false, arch,
+						fmt.Sprintf("n2-custom-%d-%d", i, i*2048), arch})
+				} else {
+					testCases = append(testCases, machineTypeTestCase{i, mem, false, arch,
+						fmt.Sprintf("n2-%s-%d", series, i), arch})
+				}
+			}
+		}
+	}
+	addARM := func(mem spec.MemPerCPU) {
+		fallback := false
+		var series string
+
+		switch mem {
+		case spec.Auto:
+			series = "standard"
+		case spec.Standard:
+			series = "standard"
+		case spec.High:
+			fallback = true
+			series = "highmem"
+		case spec.Low:
+			fallback = true
+			series = "highcpu"
+		}
+
+		if fallback {
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, vm.ArchARM64,
+				fmt.Sprintf("n2-%s-%d", series, 2), vm.ArchAMD64})
+		} else {
+			testCases = append(testCases, machineTypeTestCase{1, mem, false, vm.ArchARM64,
+				fmt.Sprintf("t2a-%s-%d", series, 1), vm.ArchARM64})
+		}
+		for i := 2; i <= 128; i += 2 {
+			fallback = fallback || i > 48 || (i > 16 && mem == spec.Auto)
+
+			if fallback {
+				expectedMachineType := fmt.Sprintf("n2-%s-%d", series, i)
+				if i > 16 && mem == spec.Auto {
+					expectedMachineType = fmt.Sprintf("n2-custom-%d-%d", i, i*2048)
+				}
+				// Expect fallback to AMD64.
+				testCases = append(testCases, machineTypeTestCase{i, mem, false, vm.ArchARM64,
+					expectedMachineType, vm.ArchAMD64})
+			} else {
+				testCases = append(testCases, machineTypeTestCase{i, mem, false, vm.ArchARM64,
+					fmt.Sprintf("t2a-%s-%d", series, i), vm.ArchARM64})
+			}
+		}
+	}
+	for _, mem := range []spec.MemPerCPU{spec.Auto, spec.Standard, spec.High, spec.Low} {
+		addAMD(mem)
+		addARM(mem)
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%d/%s/%s", tc.cpus, tc.mem, tc.arch), func(t *testing.T) {
+			machineType, selectedArch := spec.GCEMachineType(tc.cpus, tc.mem, tc.arch)
+
+			require.Equal(t, tc.expectedMachineType, machineType)
+			require.Equal(t, tc.expectedArch, selectedArch)
+		})
+	}
+}
+
 func TestCmdLogFileName(t *testing.T) {
 	ts := time.Date(2000, 1, 1, 15, 4, 12, 0, time.Local)
 

pkg/cmd/roachtest/spec/cluster_spec.go

@@ -176,10 +176,18 @@ func getGCEOpts(
 	localSSD bool,
 	RAID0 bool,
 	terminateOnMigration bool,
-	minCPUPlatform, volumeType string,
+	minCPUPlatform string,
+	arch vm.CPUArch,
+	volumeType string,
 ) vm.ProviderOpts {
 	opts := gce.DefaultProviderOpts()
 	opts.MachineType = machineType
+	if arch == vm.ArchARM64 {
+		// ARM64 machines don't support minCPUPlatform.
+		opts.MinCPUPlatform = ""
+	} else if minCPUPlatform != "" {
+		opts.MinCPUPlatform = minCPUPlatform
+	}
 	if volumeSize != 0 {
 		opts.PDVolumeSize = volumeSize
 	}
@@ -195,7 +203,6 @@ func getGCEOpts(
 		opts.UseMultipleDisks = !RAID0
 	}
 	opts.TerminateOnMigration = terminateOnMigration
-	opts.MinCPUPlatform = minCPUPlatform
 	if volumeType != "" {
 		opts.PDVolumeType = volumeType
 	}
@@ -262,7 +269,7 @@ func (s *ClusterSpec) RoachprodOpts(
 			// based on the cloud and CPU count.
 			switch s.Cloud {
 			case AWS:
-				machineType, selectedArch = AWSMachineType(s.CPUs, s.Mem, arch)
+				machineType, selectedArch = AWSMachineType(s.CPUs, s.Mem, s.PreferLocalSSD && s.VolumeSize == 0, arch)
 			case GCE:
 				machineType, selectedArch = GCEMachineType(s.CPUs, s.Mem, arch)
 			case Azure:
@@ -328,7 +335,7 @@ func (s *ClusterSpec) RoachprodOpts(
 	case GCE:
 		providerOpts = getGCEOpts(machineType, zones, s.VolumeSize, ssdCount,
 			createVMOpts.SSDOpts.UseLocalSSD, s.RAID0, s.TerminateOnMigration,
-			s.GCEMinCPUPlatform, s.GCEVolumeType,
+			s.GCEMinCPUPlatform, vm.ParseArch(createVMOpts.Arch), s.GCEVolumeType,
 		)
 	case Azure:
 		providerOpts = getAzureOpts(machineType, zones)