Merge cockroachdb#147225

147225: roachtest: added buffered write/sender cluster setting mutators to mixedversion r=DarrylWong,srosenberg,herkolategan a=Dev-Kyle

Previously `kv.rangefeed.buffered_sender.enabled` and `kv.transaction.write_buffering.enabled` were disabled in mixedversion - this patch allows the mixedversion tests to enable these settings on supported version.

Informs: cockroachdb#146415
Fixes: cockroachdb#147009

Release note: None
TeamCity: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_RoachtestNightlyGceBazel?buildTypeTab=overview&branch=147225

Co-authored-by: Kyle <kyle.li@cockroachlabs.com>

Author: craig[bot]

pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go

@@ -521,6 +521,28 @@ func DisableMutators(names ...string) CustomOption {
 	}
 }
 
+// DisableAllMutators will disable all available mutators.
+func DisableAllMutators() CustomOption {
+	return func(opts *testOptions) {
+		names := []string{}
+		for _, m := range planMutators {
+			names = append(names, m.Name())
+		}
+		DisableMutators(names...)(opts)
+	}
+}
+
+// DisableAllClusterSettingMutators will disable all available cluster setting mutators.
+func DisableAllClusterSettingMutators() CustomOption {
+	return func(opts *testOptions) {
+		names := []string{}
+		for _, m := range clusterSettingMutators {
+			names = append(names, m.Name())
+		}
+		DisableMutators(names...)(opts)
+	}
+}
+
 // WithTag allows callers give the mixedversion test instance a
 // `tag`. The tag is used as prefix in the log messages emitted by
 // this upgrade test. This is only useful when running multiple

pkg/cmd/roachtest/roachtestutil/mixedversion/planner.go

@@ -211,11 +211,9 @@ const (
 	spanConfigTenantLimit = 50000
 )
 
-// planMutators includes a list of all known `mutator`
-// implementations. A subset of these mutations might be enabled in
-// any mixedversion test plan.
-var planMutators = []mutator{
-	preserveDowngradeOptionRandomizerMutator{},
+// clusterSettingMutators includes a list of all
+// cluster setting mutator implementations.
+var clusterSettingMutators = []mutator{
 	newClusterSettingMutator(
 		"kv.expiration_leases_only.enabled",
 		[]bool{true, false},
@@ -231,8 +229,33 @@ var planMutators = []mutator{
 		[]string{"snappy", "zstd"},
 		clusterSettingMinimumVersion("v24.1.0-alpha.0"),
 	),
+	// These two settings are newly introduced, so their probabilities
+	// are temporarily raised to increase testing on them specifically.
+	// The 50% matches the metamorphic probability of these settings
+	// being enabled in non mixed version tests.
+	newClusterSettingMutator(
+		"kv.transaction.write_buffering.enabled",
+		[]bool{true, false},
+		clusterSettingMinimumVersion("v25.2.0"),
+		clusterSettingProbability(0.5),
+	),
+	newClusterSettingMutator(
+		"kv.rangefeed.buffered_sender.enabled",
+		[]bool{true, false},
+		clusterSettingMinimumVersion("v25.2.0"),
+		clusterSettingProbability(0.5),
+	),
 }
 
+// planMutators includes a list of all known `mutator`
+// implementations. A subset of these mutations might be enabled in
+// any mixedversion test plan.
+var planMutators = append([]mutator{
+	preserveDowngradeOptionRandomizerMutator{},
+},
+	clusterSettingMutators...,
+)
+
 // Plan returns the TestPlan used to upgrade the cluster from the
 // first to the final version in the `versions` field. The test plan
 // roughly translates to the sequence of steps below, annotated with

pkg/cmd/roachtest/roachtestutil/mixedversion/planner_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/version"
@@ -326,10 +327,10 @@ func Test_maxNumPlanSteps(t *testing.T) {
 
 	// There is in fact no "basic upgrade" test plan with fewer than 13 steps.
 	// The smallest plan is,
-	//planner_test.go:314: Seed:               12345
-	//Upgrades:           v24.1.1 → <current>
+	// planner_test.go:314: Seed:               12345
+	// Upgrades:           v24.1.1 → <current>
 	//		Deployment mode:    system-only
-	//Plan:
+	// Plan:
 	//	├── start cluster at version "v24.1.1" (1)
 	//	├── wait for all nodes (:1-4) to acknowledge cluster version '24.1' on system tenant (2)
 	//	└── upgrade cluster from "v24.1.1" to "<current>"
@@ -893,6 +894,36 @@ func dummyHook(context.Context, *logger.Logger, *rand.Rand, *Helper) error {
 	return nil
 }
 
+func Test_DisableAllMutators(t *testing.T) {
+	mvt := newTest(DisableAllMutators())
+
+	rng, seed := randutil.NewTestRand()
+	mvt.seed = seed
+	mvt.prng = rng
+
+	plan, err := mvt.plan()
+	require.NoError(t, err)
+	require.Nil(t, plan.enabledMutators)
+
+}
+
+func Test_DisableAllClusterSettingMutators(t *testing.T) {
+	mvt := newTest(DisableAllClusterSettingMutators())
+
+	rng, seed := randutil.NewTestRand()
+	mvt.seed = seed
+	mvt.prng = rng
+
+	plan, err := mvt.plan()
+	require.NoError(t, err)
+
+	for _, enabled := range plan.enabledMutators {
+		if _, ok := enabled.(clusterSettingMutator); ok {
+			t.Errorf("cluster setting mutator %q was not disabled", enabled.Name())
+		}
+	}
+}
+
 // This is a regression test to ensure that separate process deployments
 // correctly default to using latest predecessors.
 func Test_SeparateProcessUsesLatestPred(t *testing.T) {

pkg/cmd/roachtest/test_runner.go

@@ -940,8 +940,8 @@ func (r *testRunner) runWorker(
 					// 50% change of enabling buffered writes.
 					//
 					// Disabled by default. Disabled for mixed-version tests
-					// since these cluster settings are not supported in all
-					// versions.
+					// because they use a separate mechanism for metamorphic
+					// cluster settings.
 					for _, tc := range []struct {
 						setting string
 						label   string

pkg/cmd/roachtest/tests/mixed_version_backup.go

@@ -2821,11 +2821,7 @@ func registerBackupMixedVersion(r registry.Registry) {
 				// TODO(renato): don't disable these mutators when the
 				// framework exposes some utility to provide mutual exclusion
 				// of concurrent steps.
-				mixedversion.DisableMutators(
-					mixedversion.ClusterSettingMutator("kv.expiration_leases_only.enabled"),
-					mixedversion.ClusterSettingMutator("storage.ingest_split.enabled"),
-					mixedversion.ClusterSettingMutator("storage.sstable.compression_algorithm"),
-				),
+				mixedversion.DisableAllClusterSettingMutators(),
 			)
 			testRNG := mvt.RNG()