Merge #145984 #146155

145984: logictest: deflake recent addition to select_for_update r=yuzefovich a=michae2

The testcases I added in #144449 depend on a large enough batch size to match the expected tracing.

Fixes: #144830

Release note: None

146155: explain: deflake TestMaximumMemoryUsage for good r=yuzefovich a=yuzefovich

This commit reverts adeddb5 which attempted to deflake `TestMaximumMemoryUsage` which could fail under metamorphic randomization. Instead, this commit simply forces production values for the relevant batch sizes (I think `kv-batch-size` is the one to blame) because the previous attempt of introducing the retry loop could still flake.

Fixes: #146052.

Release note: None

Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: 3 people

pkg/sql/logictest/testdata/logic_test/select_for_update

@@ -1,4 +1,4 @@
-# LogicTest: !weak-iso-level-configs
+# LogicTest: !weak-iso-level-configs !metamorphic-batch-sizes
 # This test assumes that SERIALIZABLE isolation is being used. See
 # select_for_update_read_committed for READ COMMITTED testing.
 

pkg/sql/opt/exec/explain/BUILD.bazel

@@ -89,7 +89,6 @@ go_test(
         "//pkg/util/log",
         "//pkg/util/treeprinter",
         "@com_github_cockroachdb_datadriven//:datadriven",
-        "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@in_gopkg_yaml_v2//:yaml_v2",

pkg/sql/opt/exec/explain/output_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilitiespb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
@@ -30,7 +31,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/datadriven"
-	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	yaml "gopkg.in/yaml.v2"
@@ -395,7 +395,21 @@ func TestMaximumMemoryUsage(t *testing.T) {
 	skip.UnderRace(t, "multinode cluster setup times out under race")
 
 	const numNodes = 3
-	tc := testcluster.StartTestCluster(t, numNodes, base.TestClusterArgs{})
+	tc := testcluster.StartTestCluster(t, numNodes, base.TestClusterArgs{
+		ServerArgs: base.TestServerArgs{
+			Knobs: base.TestingKnobs{
+				SQLEvalContext: &eval.TestingKnobs{
+					// We disable the randomization of the batch sizes so that
+					// small number of gRPC calls is issued in the query below
+					// (with kv-batch-size=1 we would issue 10k of them which
+					// might result in dropping the ComponentStats proto that
+					// powers "maximum memory usage" from the trace, flaking the
+					// test).
+					ForceProductionValues: true,
+				},
+			},
+		},
+	})
 	ctx := context.Background()
 	defer tc.Stopper().Stop(ctx)
 
@@ -418,29 +432,19 @@ func TestMaximumMemoryUsage(t *testing.T) {
 		return err
 	})
 
-	// In rare cases (due to metamorphic randomization) we might drop the
-	// ComponentStats proto that powers "maximum memory usage" stat from the
-	// trace, so we add a retry loop around this.
-	testutils.SucceedsSoon(t, func() error {
-		rows := db.QueryStr(t, "EXPLAIN ANALYZE SELECT max(v) FROM t GROUP BY bucket;")
-		var output strings.Builder
-		maxMemoryRE := regexp.MustCompile(`maximum memory usage: ([\d\.]+) MiB`)
-		var maxMemoryUsage float64
-		for _, row := range rows {
-			output.WriteString(row[0])
-			output.WriteString("\n")
-			s := strings.TrimSpace(row[0])
-			if matches := maxMemoryRE.FindStringSubmatch(s); len(matches) > 0 {
-				var err error
-				maxMemoryUsage, err = strconv.ParseFloat(matches[1], 64)
-				if err != nil {
-					return err
-				}
-			}
-		}
-		if maxMemoryUsage < 5.0 {
-			return errors.Newf("expected maximum memory usage to be at least 5 MiB, full output:\n\n%s", output.String())
+	rows := db.QueryStr(t, "EXPLAIN ANALYZE SELECT max(v) FROM t GROUP BY bucket;")
+	var output strings.Builder
+	maxMemoryRE := regexp.MustCompile(`maximum memory usage: ([\d\.]+) MiB`)
+	var maxMemoryUsage float64
+	for _, row := range rows {
+		output.WriteString(row[0])
+		output.WriteString("\n")
+		s := strings.TrimSpace(row[0])
+		if matches := maxMemoryRE.FindStringSubmatch(s); len(matches) > 0 {
+			var err error
+			maxMemoryUsage, err = strconv.ParseFloat(matches[1], 64)
+			require.NoError(t, err)
 		}
-		return nil
-	})
+	}
+	require.Greaterf(t, maxMemoryUsage, 5.0, "expected maximum memory usage to be at least 5 MiB, full output:\n\n%s", output.String())
 }