Merge #109823

109823: ccl,roachtest: remove uses of storage.mvcc.range_tombstones.enabled r=bananabrick a=jbowens Remove uses of the `storage.mvcc.range_tombstones.enabled` cluster setting. This cluster setting is ignored in cluster version 23.1+. Epic: none Informs #97869. Release note: none Co-authored-by: Jackson Owens <[email protected]>
cockroachdb · Sep 9, 2023 · b5836a5 · b5836a5
2 parents 46feddd + fe6a83d
commit b5836a5
Show file tree

Hide file tree

Showing 7 changed files with 10 additions and 47 deletions.
diff --git a/pkg/ccl/backupccl/tenant_backup_nemesis_test.go b/pkg/ccl/backupccl/tenant_backup_nemesis_test.go
@@ -70,9 +70,6 @@ func TestTenantBackupWithCanceledImport(t *testing.T) {
 	)
 	defer hostClusterCleanupFn()
 
-	hostSQLDB.Exec(t, "SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true")
-	hostSQLDB.Exec(t, "ALTER TENANT ALL SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true")
-
 	tenant10, err := tc.Servers[0].StartTenant(ctx, base.TestTenantArgs{
 		TenantID: roachpb.MustMakeTenantID(10),
 		TestingKnobs: base.TestingKnobs{
@@ -143,10 +140,6 @@ func TestTenantBackupNemesis(t *testing.T) {
 	)
 	defer hostClusterCleanupFn()
 
-	// Range tombstones must be enabled for tenant backups to work correctly.
-	hostSQLDB.Exec(t, "SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true")
-	hostSQLDB.Exec(t, "ALTER TENANT ALL SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true")
-
 	tenant10, err := tc.Servers[0].StartTenant(ctx, base.TestTenantArgs{
 		TenantID: roachpb.MustMakeTenantID(10),
 		TestingKnobs: base.TestingKnobs{

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/in-progress-import-rollback b/pkg/ccl/backupccl/testdata/backup-restore/in-progress-import-rollback
@@ -7,13 +7,11 @@
 # - roll it back it back non-mvcc
 # - run an inc backup and ensure we reintroduce the table spans
 
-# disabled to run within tenant as they don't have access to the
-# storage.mvcc.range_tombstones.enabled cluster setting
-new-cluster name=s1 beforeVersion=23_1_MVCCTombstones disable-tenant
+new-cluster name=s1
 ----
 
 ###########
-# Case 1: an incremental backup captures a non-mvcc rollback
+# set up
 ###########
 
 exec-sql
@@ -36,12 +34,6 @@ exec-sql
 SET CLUSTER SETTING kv.bulkio.write_metadata_sst.enabled = false;
 ----
 
-
-exec-sql
-SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = false;
-----
-
-
 exec-sql
 EXPORT INTO CSV 'nodelocal://1/export1/' FROM SELECT * FROM baz;
 ----
@@ -163,8 +155,8 @@ ORDER BY
 ----
 d foo table 3 full
 d foofoo table 4 full
-d foo table 0 incremental
-d foofoo table 1 incremental
+d foo table 3 incremental
+d foofoo table 7 incremental
 
 query-sql
 SELECT
@@ -178,8 +170,8 @@ ORDER BY
 ----
 d foo table 3 full
 d foofoo table 4 full
-d foo table 0 incremental
-d foofoo table 1 incremental
+d foo table 3 incremental
+d foofoo table 7 incremental
 
 
 query-sql
@@ -194,8 +186,8 @@ ORDER BY
 ----
 d foo table 3 full
 d foofoo table 4 full
-d foo table 0 incremental
-d foofoo table 1 incremental
+d foo table 3 incremental
+d foofoo table 7 incremental
 
 
 # To verify the incremental backed up the pre-import state table, restore d and ensure all tables
@@ -242,11 +234,6 @@ exec-sql
 SET CLUSTER SETTING jobs.debug.pausepoints = 'import.after_ingest';
 ----
 
-
-exec-sql
-SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true;
-----
-
 exec-sql
 SET CLUSTER SETTING kv.bulkio.write_metadata_sst.enabled = false;
 ----

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-fast-drop b/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-fast-drop
@@ -19,10 +19,6 @@ BACKUP INTO 'nodelocal://1/cluster_backup';
 new-cluster name=s2 nodes=1 share-io-dir=s1 disable-tenant
 ----
 
-exec-sql
-SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true;
-----
-
 # Restore's OnFailOrCancel deletes descriptors which requires us to wait for no
 # versions of that descriptor to be leased before proceeding. Since our test fails
 # the job after the descriptors have been published, it's possible for them to be leased

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-retry b/pkg/ccl/backupccl/testdata/backup-restore/restore-on-fail-or-cancel-retry
@@ -1,5 +1,4 @@
-# disabled for tenants as they can't set storage.mvcc.range_tombstones.enabled
-new-cluster name=s1 nodes=1 disable-tenant
+new-cluster name=s1 nodes=1
 ----
 
 subtest restore-retry
@@ -17,11 +16,7 @@ exec-sql
 BACKUP INTO 'nodelocal://1/cluster_backup';
 ----
 
-new-cluster name=s2 nodes=1 share-io-dir=s1 disable-tenant
-----
-
-exec-sql
-SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true;
+new-cluster name=s2 nodes=1 share-io-dir=s1
 ----
 
 exec-sql

diff --git a/pkg/ccl/changefeedccl/changefeed_test.go b/pkg/ccl/changefeedccl/changefeed_test.go
@@ -3611,7 +3611,6 @@ func TestChangefeedFailOnTableOffline(t *testing.T) {
 	cdcTestNamedWithSystem(t, "reverted import fails changefeed with earlier cursor", func(t *testing.T, s TestServerWithSystem, f cdctest.TestFeedFactory) {
 		sysSQLDB := sqlutils.MakeSQLRunner(s.SystemDB)
 		sysSQLDB.Exec(t, "SET CLUSTER SETTING kv.bulk_io_write.small_write_size = '1'")
-		sysSQLDB.Exec(t, "SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = true")
 
 		sqlDB := sqlutils.MakeSQLRunner(s.DB)
 		sqlDB.Exec(t, `CREATE TABLE for_import (a INT PRIMARY KEY, b INT)`)

diff --git a/pkg/cmd/roachtest/tests/backup.go b/pkg/cmd/roachtest/tests/backup.go
@@ -954,10 +954,6 @@ func runBackupMVCCRangeTombstones(
 	t.Status("configuring cluster")
 	_, err := conn.Exec(`SET CLUSTER SETTING kv.bulk_ingest.max_index_buffer_size = '2gb'`)
 	require.NoError(t, err)
-	if config.tenantName == "" {
-		_, err = conn.Exec(`SET CLUSTER SETTING storage.mvcc.range_tombstones.enabled = 't'`)
-		require.NoError(t, err)
-	}
 	_, err = conn.Exec(`SET CLUSTER SETTING server.debug.default_vmodule = 'txn=2,sst_batcher=4,
 revert=2'`)
 	require.NoError(t, err)

diff --git a/pkg/cmd/roachtest/tests/mvcc_gc.go b/pkg/cmd/roachtest/tests/mvcc_gc.go
@@ -95,9 +95,6 @@ func runMVCCGC(ctx context.Context, t test.Test, c cluster.Cluster) {
 		execSQLOrFail(fmt.Sprintf(`SET CLUSTER SETTING %s = $1`, name), value)
 	}
 
-	// Explicitly enable range tombstones. Can be removed once ranges tombstones
-	// are enabled by default.
-	setClusterSetting("storage.mvcc.range_tombstones.enabled", true)
 	// Protected timestamps prevent GC from collecting data, even with low ttl
 	// we need to wait for protected ts to be moved. By reducing this interval
 	// we ensure that data will always be collectable after ttl + 5s.