From fe6a83d0ab9d68bf3c9fc947b995e3f38d1660d8 Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Thu, 31 Aug 2023 12:20:56 -0400 Subject: [PATCH] ccl,roachtest: remove uses of storage.mvcc.range_tombstones.enabled 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 --- .../backupccl/tenant_backup_nemesis_test.go | 7 ----- .../in-progress-import-rollback | 29 +++++-------------- .../restore-on-fail-or-cancel-fast-drop | 4 --- .../restore-on-fail-or-cancel-retry | 9 ++---- pkg/ccl/changefeedccl/changefeed_test.go | 1 - pkg/cmd/roachtest/tests/backup.go | 4 --- pkg/cmd/roachtest/tests/mvcc_gc.go | 3 -- 7 files changed, 10 insertions(+), 47 deletions(-) diff --git a/pkg/ccl/backupccl/tenant_backup_nemesis_test.go b/pkg/ccl/backupccl/tenant_backup_nemesis_test.go index a66eb817d145..34ffb620c2fb 100644 --- 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 index 2eaf1bbcf3fa..aa2f86ca26f3 100644 --- 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 index 54500bb53c34..300bf4648847 100644 --- 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 index 5cc6e27d183e..5dc618d082d3 100644 --- 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 index 270d6a4bd2c7..543a178b964d 100644 --- a/pkg/ccl/changefeedccl/changefeed_test.go +++ b/pkg/ccl/changefeedccl/changefeed_test.go @@ -3491,7 +3491,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 index e50d467e9421..accf1a474119 100644 --- 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 index 326c18dfb380..5d3ce2e57720 100644 --- 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.