Merge #109010

109010: sql,backfill: deflake TestValidationWithProtectedTS r=rafiss a=rafiss This test was slow enough to the point of flaking. I tracked down the problem to the metamorphic constant `kv-batch-size`. Now we set a testing knob to make sure that the value is always the production value. This also required a fix to plumb the testing knob through in one more place. fixes #106960 informs #108539 Release note: None Co-authored-by: Rafi Shamim <[email protected]>
cockroachdb · Aug 18, 2023 · cf87437 · cf87437
2 parents c7d3514 + 7a701ec
commit cf87437
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 6 deletions.
diff --git a/pkg/sql/backfill/backfill.go b/pkg/sql/backfill/backfill.go
@@ -850,11 +850,12 @@ func (ib *IndexBackfiller) BuildIndexEntriesChunk(
 	if err := fetcher.Init(
 		ctx,
 		row.FetcherInitArgs{
-			Txn:        txn,
-			Alloc:      &ib.alloc,
-			MemMonitor: ib.mon,
-			Spec:       &spec,
-			TraceKV:    traceKV,
+			Txn:                        txn,
+			Alloc:                      &ib.alloc,
+			MemMonitor:                 ib.mon,
+			Spec:                       &spec,
+			TraceKV:                    traceKV,
+			ForceProductionKVBatchSize: ib.evalCtx.TestingKnobs.ForceProductionValues,
 		},
 	); err != nil {
 		return nil, nil, 0, err

diff --git a/pkg/sql/backfill_protected_timestamp_test.go b/pkg/sql/backfill_protected_timestamp_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigptsreader"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
@@ -54,6 +55,9 @@ func TestValidationWithProtectedTS(t *testing.T) {
 	indexScanQuery := regexp.MustCompile(`SELECT count\(1\) FROM \[\d+ AS t\]@\[2\]`)
 	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
 		Knobs: base.TestingKnobs{
+			SQLEvalContext: &eval.TestingKnobs{
+				ForceProductionValues: true,
+			},
 			SQLExecutor: &sql.ExecutorTestingKnobs{
 				BeforeExecute: func(ctx context.Context, sql string, descriptors *descs.Collection) {
 					if indexScanQuery.MatchString(sql) {

diff --git a/pkg/testutils/serverutils/test_server_shim.go b/pkg/testutils/serverutils/test_server_shim.go
@@ -52,7 +52,7 @@ import (
 //
 // To investigate, consider using "COCKROACH_TEST_TENANT=true" to force-enable
 // just the secondary tenant in all runs (or, alternatively, "false" to
-// force-disable), or use "COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=false"
+// force-disable), or use "COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true"
 // to disable all random test variables altogether.`
 
 const defaultTestTenantMessage = `test server using tenant; see comment at top of test_server_shim.go for details.`

diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel
@@ -177,6 +177,7 @@ go_test(
         "//pkg/sql/schemachanger/scplan",
         "//pkg/sql/sem/builtins/builtinconstants",
         "//pkg/sql/sem/catid",
+        "//pkg/sql/sem/eval",
         "//pkg/sql/sem/tree",
         "//pkg/sql/types",
         "//pkg/storage",

diff --git a/pkg/upgrade/upgrades/role_members_ids_migration_test.go b/pkg/upgrade/upgrades/role_members_ids_migration_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
@@ -51,6 +52,9 @@ func runTestRoleMembersIDMigration(t *testing.T, numUsers int) {
 	tc := testcluster.StartTestCluster(t, 1 /* nodes */, base.TestClusterArgs{
 		ServerArgs: base.TestServerArgs{
 			Knobs: base.TestingKnobs{
+				SQLEvalContext: &eval.TestingKnobs{
+					ForceProductionValues: true,
+				},
 				Server: &server.TestingKnobs{
 					BootstrapVersionKeyOverride:    clusterversion.V22_2,
 					DisableAutomaticVersionUpgrade: make(chan struct{}),