From 1dd71d45ddd8eefb3b035f77daabf54a1103a665 Mon Sep 17 00:00:00 2001
From: Yahor Yuzefovich <yahor@cockroachlabs.com>
Date: Sat, 11 Nov 2023 02:18:51 +0000
Subject: [PATCH] colfetcher,roachtest: re-enable direct scans metamorphization

This commit makes a couple of tweaks to the backup roachtests which
allows us to make the default value of `sql.distsql.direct_columnar_scans.enabled`
a metamorphic variable again. In particular, we need to disable this
setting for backups on `bank` data set due to pathological case as
described in #113816. This also allows us to reduce `--max-sql-memory`
since we don't expect memory usage to be as high without direct scans.

Fixes: #113816.

Release note: None
---
 pkg/cmd/roachtest/tests/backup.go      | 14 ++++++++++----
 pkg/sql/colfetcher/cfetcher_wrapper.go |  7 ++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/pkg/cmd/roachtest/tests/backup.go b/pkg/cmd/roachtest/tests/backup.go
index 448cba34045a..1547371620c0 100644
--- a/pkg/cmd/roachtest/tests/backup.go
+++ b/pkg/cmd/roachtest/tests/backup.go
@@ -100,7 +100,10 @@ func importBankDataSplit(
 
 	// NB: starting the cluster creates the logs dir as a side effect,
 	// needed below.
-	c.Start(ctx, t.L(), maybeUseMemoryBudget(t, 50), install.MakeClusterSettings())
+	c.Start(ctx, t.L(), option.DefaultStartOptsNoBackups(), install.MakeClusterSettings())
+	// See #113816 for why this is needed for now (probably until #94850 is
+	// resolved).
+	c.Run(ctx, c.Node(1), "./cockroach sql --insecure -e 'SET CLUSTER SETTING sql.distsql.direct_columnar_scans.enabled = false;'")
 	runImportBankDataSplit(ctx, rows, ranges, t, c)
 	return dest
 }
@@ -744,7 +747,7 @@ func runBackupMVCCRangeTombstones(
 ) {
 	if !config.skipClusterSetup {
 		c.Put(ctx, t.DeprecatedWorkload(), "./workload") // required for tpch
-		c.Start(ctx, t.L(), maybeUseMemoryBudget(t, 50), install.MakeClusterSettings())
+		c.Start(ctx, t.L(), option.DefaultStartOptsNoBackups(), install.MakeClusterSettings())
 	}
 	t.Status("starting csv servers")
 	c.Run(ctx, c.All(), `./cockroach workload csv-server --port=8081 &> logs/workload-csv-server.log < /dev/null &`)
@@ -755,8 +758,11 @@ func runBackupMVCCRangeTombstones(
 	t.Status("configuring cluster")
 	_, err := conn.Exec(`SET CLUSTER SETTING kv.bulk_ingest.max_index_buffer_size = '2gb'`)
 	require.NoError(t, err)
-	_, err = conn.Exec(`SET CLUSTER SETTING server.debug.default_vmodule = 'txn=2,sst_batcher=4,
-revert=2'`)
+	_, err = conn.Exec(`SET CLUSTER SETTING server.debug.default_vmodule = 'txn=2,sst_batcher=4,revert=2'`)
+	require.NoError(t, err)
+	// See #113816 for why this is needed for now (probably until #94850 is
+	// resolved).
+	_, err = conn.Exec(`SET CLUSTER SETTING sql.distsql.direct_columnar_scans.enabled = false;`)
 	require.NoError(t, err)
 	// Wait for ranges to upreplicate.
 	require.NoError(t, WaitFor3XReplication(ctx, t, conn))
diff --git a/pkg/sql/colfetcher/cfetcher_wrapper.go b/pkg/sql/colfetcher/cfetcher_wrapper.go
index c897820348e1..f75aa9e5085f 100644
--- a/pkg/sql/colfetcher/cfetcher_wrapper.go
+++ b/pkg/sql/colfetcher/cfetcher_wrapper.go
@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/storage"
+	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/errors"
@@ -40,7 +41,11 @@ var DirectScansEnabled = settings.RegisterBoolSetting(
 	settings.ApplicationLevel,
 	"sql.distsql.direct_columnar_scans.enabled",
 	"set to true to enable the 'direct' columnar scans in the KV layer",
-	// TODO(yuzefovich): make this metamorphic constant once #113816 is fixed.
+	directScansEnabledDefault,
+)
+
+var directScansEnabledDefault = util.ConstantWithMetamorphicTestBool(
+	"direct-scans-enabled",
 	false,
 )