Skip to content

Commit

Permalink
colfetcher,roachtest: re-enable direct scans metamorphization
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yuzefovich committed Nov 15, 2023
1 parent 0456c39 commit 1dd71d4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
14 changes: 10 additions & 4 deletions pkg/cmd/roachtest/tests/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 &`)
Expand All @@ -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))
Expand Down
7 changes: 6 additions & 1 deletion pkg/sql/colfetcher/cfetcher_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
)

Expand Down

0 comments on commit 1dd71d4

Please sign in to comment.