From 71becf96ec82c3e103b6188dfbdc9828f441f296 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 3 Aug 2022 14:38:02 +0000 Subject: [PATCH] backupccl: remove clusterversion.IncrementalBackupSubdir Release note: none. --- pkg/ccl/backupccl/backupdest/BUILD.bazel | 2 - .../backupdest/backup_destination_test.go | 63 ++++--------------- pkg/ccl/backupccl/backupdest/incrementals.go | 6 +- pkg/clusterversion/cockroach_versions.go | 7 --- pkg/clusterversion/key_string.go | 55 ++++++++-------- 5 files changed, 42 insertions(+), 91 deletions(-) diff --git a/pkg/ccl/backupccl/backupdest/BUILD.bazel b/pkg/ccl/backupccl/backupdest/BUILD.bazel index dde48e96258a..f4bab9168bde 100644 --- a/pkg/ccl/backupccl/backupdest/BUILD.bazel +++ b/pkg/ccl/backupccl/backupdest/BUILD.bazel @@ -48,9 +48,7 @@ go_test( "//pkg/ccl/utilccl", "//pkg/cloud", "//pkg/cloud/impl:cloudimpl", - "//pkg/clusterversion", "//pkg/jobs/jobspb", - "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/security/username", diff --git a/pkg/ccl/backupccl/backupdest/backup_destination_test.go b/pkg/ccl/backupccl/backupdest/backup_destination_test.go index 5b13b3d956b7..feee9df61758 100644 --- a/pkg/ccl/backupccl/backupdest/backup_destination_test.go +++ b/pkg/ccl/backupccl/backupdest/backup_destination_test.go @@ -21,9 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils" "github.com/cockroachdb/cockroach/pkg/cloud" _ "github.com/cockroachdb/cockroach/pkg/cloud/impl" // register cloud storage providers - "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/jobs/jobspb" - "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -298,7 +296,6 @@ func TestBackupRestoreResolveDestination(t *testing.T) { inc8Time := full3Time.Add(time.Minute * 30) inc9Time := inc8Time.Add(time.Minute * 30) full4Time := inc9Time.Add(time.Minute * 30) - inc10Time := full4Time.Add(time.Minute * 30) // firstBackupChain is maintained throughout the tests as the history of // backups that were taken based on the initial full backup. @@ -311,14 +308,12 @@ func TestBackupRestoreResolveDestination(t *testing.T) { // separate path relative to the full backup in their chain. Otherwise, // it should be an empty array of strings noIncrementalStorage := []string(nil) - incrementalBackupSubdirEnabled := true - notIncrementalBackupSubdirEnabled := false firstRemoteBackupChain := []string(nil) testCollectionBackup := func(t *testing.T, backupTime time.Time, expectedDefault, expectedSuffix, expectedIncDir string, expectedPrevBackups []string, - appendToLatest bool, subdir string, incrementalTo []string, incrementalBackupSubdirEnabled bool) { + appendToLatest bool, subdir string, incrementalTo []string) { endTime := hlc.Timestamp{WallTime: backupTime.UnixNano()} incrementalFrom := []string(nil) @@ -336,17 +331,6 @@ func TestBackupRestoreResolveDestination(t *testing.T) { _, localityCollections, err = backupdest.GetURIsByLocalityKV(incrementalTo, "") require.NoError(t, err) } - currentVersion := execCfg.Settings.Version.ActiveVersion(ctx) - if !incrementalBackupSubdirEnabled { - // Downgrading is disallowed in normal operation, but fine for testing here. - err = execCfg.Settings.Version.SetActiveVersion(ctx, clusterversion.ClusterVersion{ - Version: roachpb.Version{ - Major: 21, - Minor: 2, - }, - }) - require.NoError(t, err) - } fullBackupExists := false if expectedIncDir != "" { @@ -360,9 +344,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { incrementalFrom, &execCfg, ) - clusterErr := execCfg.Settings.Version.SetActiveVersion(ctx, currentVersion) require.NoError(t, err) - require.NoError(t, clusterErr) localityDests := make(map[string]string, len(localityCollections)) for locality, localityDest := range localityCollections { @@ -389,7 +371,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, fullTime, expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain, - false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled) + false /* intoLatest */, noExplicitSubDir, noIncrementalStorage) firstBackupChain = append(firstBackupChain, expectedDefault) firstRemoteBackupChain = append(firstRemoteBackupChain, expectedDefault) writeManifest(t, expectedDefault) @@ -406,7 +388,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc1Time, expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain, - true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled) + true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo) firstBackupChain = append(firstBackupChain, expectedDefault) writeManifest(t, expectedDefault) } @@ -420,7 +402,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc2Time, expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain, - true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled) + true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo) firstBackupChain = append(firstBackupChain, expectedDefault) writeManifest(t, expectedDefault) } @@ -435,7 +417,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, full2Time, expectedDefault, expectedSuffix, expectedIncDir, []string(nil), - false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled) + false /* intoLatest */, noExplicitSubDir, noIncrementalStorage) writeManifest(t, expectedDefault) // We also wrote a new full backup, so let's update the latest. writeLatest(t, collectionLoc, expectedSuffix) @@ -450,7 +432,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc3Time, expectedDefault, expectedSuffix, expectedIncDir, []string{backup2Location}, - true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo, incrementalBackupSubdirEnabled) + true /* intoLatest */, noExplicitSubDir, defaultIncrementalTo) writeManifest(t, expectedDefault) } @@ -463,7 +445,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc4Time, expectedDefault, expectedSuffix, expectedIncDir, firstBackupChain, - false /* intoLatest */, expectedSubdir, defaultIncrementalTo, incrementalBackupSubdirEnabled) + false /* intoLatest */, expectedSubdir, defaultIncrementalTo) writeManifest(t, expectedDefault) } @@ -479,7 +461,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc5Time, expectedDefault, expectedSuffix, expectedIncDir, firstRemoteBackupChain, - false /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled) + false /* intoLatest */, expectedSubdir, customIncrementalTo) writeManifest(t, expectedDefault) firstRemoteBackupChain = append(firstRemoteBackupChain, expectedDefault) @@ -497,7 +479,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc6Time, expectedDefault, expectedSuffix, expectedIncDir, firstRemoteBackupChain, - false /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled) + false /* intoLatest */, expectedSubdir, customIncrementalTo) writeManifest(t, expectedDefault) } @@ -514,7 +496,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc7Time, expectedDefault, expectedSuffix, expectedIncDir, []string{backup2Location}, - true /* intoLatest */, expectedSubdir, customIncrementalTo, incrementalBackupSubdirEnabled) + true /* intoLatest */, expectedSubdir, customIncrementalTo) writeManifest(t, expectedDefault) } @@ -528,7 +510,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, full3Time, expectedDefault, expectedSuffix, expectedIncDir, []string(nil), - false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, incrementalBackupSubdirEnabled) + false /* intoLatest */, noExplicitSubDir, noIncrementalStorage) writeManifest(t, expectedDefault) // We also wrote a new full backup, so let's update the latest. writeLatest(t, collectionLoc, expectedSuffix) @@ -557,42 +539,23 @@ func TestBackupRestoreResolveDestination(t *testing.T) { testCollectionBackup(t, inc9Time, expectedDefault, expectedSuffix, expectedIncDir, []string{backup3Location, oldStyleDefault}, - true /* intoLatest */, expectedSubdir, noIncrementalStorage, incrementalBackupSubdirEnabled) + true /* intoLatest */, expectedSubdir, noIncrementalStorage) writeManifest(t, expectedDefault) } // A new full backup: BACKUP INTO collection - var backup4Location string { expectedSuffix := "/2020/12/25-120000.00" expectedIncDir := "" expectedDefault := fmt.Sprintf("nodelocal://1/%s%s?AUTH=implicit", t.Name(), expectedSuffix) - backup4Location = expectedDefault testCollectionBackup(t, full4Time, expectedDefault, expectedSuffix, expectedIncDir, []string(nil), - false /* intoLatest */, noExplicitSubDir, noIncrementalStorage, notIncrementalBackupSubdirEnabled) + false /* intoLatest */, noExplicitSubDir, noIncrementalStorage) writeManifest(t, expectedDefault) // We also wrote a new full backup, so let's update the latest. writeLatest(t, collectionLoc, expectedSuffix) } - - // An automatic incremental into the fourth full backup: BACKUP INTO LATEST - // IN collection, BUT simulating an old cluster that doesn't support - // dedicated incrementals subdir by default yet. - { - expectedSuffix := "/2020/12/25-120000.00" - expectedIncDir := "/20201225/123000.00" - expectedSubdir := expectedSuffix - expectedDefault := fmt.Sprintf("nodelocal://1/%s%s%s?AUTH=implicit", - t.Name(), - expectedSuffix, expectedIncDir) - - testCollectionBackup(t, inc10Time, - expectedDefault, expectedSuffix, expectedIncDir, []string{backup4Location}, - true /* intoLatest */, expectedSubdir, noIncrementalStorage, notIncrementalBackupSubdirEnabled) - writeManifest(t, expectedDefault) - } }) }) } diff --git a/pkg/ccl/backupccl/backupdest/incrementals.go b/pkg/ccl/backupccl/backupdest/incrementals.go index b412fb8233d1..3431cfb68222 100644 --- a/pkg/ccl/backupccl/backupdest/incrementals.go +++ b/pkg/ccl/backupccl/backupdest/incrementals.go @@ -18,7 +18,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupbase" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils" "github.com/cockroachdb/cockroach/pkg/cloud" - "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/errors" @@ -166,9 +165,8 @@ func ResolveIncrementalsBackupLocation( "Please choose a location manually with the `incremental_location` parameter.") } - // If the cluster isn't fully migrated, or we have backups in the old default - // location, continue to use the old location. - if len(prevOld) > 0 || !execCfg.Settings.Version.IsActive(ctx, clusterversion.IncrementalBackupSubdir) { + // If we have backups in the old default location, continue to use the old location. + if len(prevOld) > 0 { return resolvedIncrementalsBackupLocationOld, nil } diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 7d98023207a6..72ec94d32ec7 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -247,9 +247,6 @@ const ( // PebbleFormatSplitUserKeysMarked performs a Pebble-level migration and // upgrades the Pebble format major version to FormatSplitUserKeysMarked. PebbleFormatSplitUserKeysMarked - // IncrementalBackupSubdir enables backing up new incremental backups to a - // dedicated subdirectory, to make it easier to apply a different ttl. - IncrementalBackupSubdir // EnableNewStoreRebalancer enables the new store rebalancer introduced in // 22.1. EnableNewStoreRebalancer @@ -465,10 +462,6 @@ var versionsSingleton = keyedVersions{ Key: PebbleFormatSplitUserKeysMarked, Version: roachpb.Version{Major: 21, Minor: 2, Internal: 90}, }, - { - Key: IncrementalBackupSubdir, - Version: roachpb.Version{Major: 21, Minor: 2, Internal: 92}, - }, { Key: EnableNewStoreRebalancer, Version: roachpb.Version{Major: 21, Minor: 2, Internal: 96}, diff --git a/pkg/clusterversion/key_string.go b/pkg/clusterversion/key_string.go index 65ea63385196..2087c66a50a7 100644 --- a/pkg/clusterversion/key_string.go +++ b/pkg/clusterversion/key_string.go @@ -33,37 +33,36 @@ func _() { _ = x[EnableDeclarativeSchemaChanger-22] _ = x[RowLevelTTL-23] _ = x[PebbleFormatSplitUserKeysMarked-24] - _ = x[IncrementalBackupSubdir-25] - _ = x[EnableNewStoreRebalancer-26] - _ = x[ClusterLocksVirtualTable-27] - _ = x[AutoStatsTableSettings-28] - _ = x[SuperRegions-29] - _ = x[EnableNewChangefeedOptions-30] - _ = x[SpanCountTable-31] - _ = x[PreSeedSpanCountTable-32] - _ = x[SeedSpanCountTable-33] - _ = x[V22_1-34] - _ = x[Start22_2-35] - _ = x[LocalTimestamps-36] - _ = x[PebbleFormatSplitUserKeysMarkedCompacted-37] - _ = x[EnsurePebbleFormatVersionRangeKeys-38] - _ = x[EnablePebbleFormatVersionRangeKeys-39] - _ = x[TrigramInvertedIndexes-40] - _ = x[RemoveGrantPrivilege-41] - _ = x[MVCCRangeTombstones-42] - _ = x[UpgradeSequenceToBeReferencedByID-43] - _ = x[SampledStmtDiagReqs-44] - _ = x[AddSSTableTombstones-45] - _ = x[SystemPrivilegesTable-46] - _ = x[EnablePredicateProjectionChangefeed-47] - _ = x[AlterSystemSQLInstancesAddLocality-48] - _ = x[SystemExternalConnectionsTable-49] - _ = x[AlterSystemStatementStatisticsAddIndexRecommendations-50] + _ = x[EnableNewStoreRebalancer-25] + _ = x[ClusterLocksVirtualTable-26] + _ = x[AutoStatsTableSettings-27] + _ = x[SuperRegions-28] + _ = x[EnableNewChangefeedOptions-29] + _ = x[SpanCountTable-30] + _ = x[PreSeedSpanCountTable-31] + _ = x[SeedSpanCountTable-32] + _ = x[V22_1-33] + _ = x[Start22_2-34] + _ = x[LocalTimestamps-35] + _ = x[PebbleFormatSplitUserKeysMarkedCompacted-36] + _ = x[EnsurePebbleFormatVersionRangeKeys-37] + _ = x[EnablePebbleFormatVersionRangeKeys-38] + _ = x[TrigramInvertedIndexes-39] + _ = x[RemoveGrantPrivilege-40] + _ = x[MVCCRangeTombstones-41] + _ = x[UpgradeSequenceToBeReferencedByID-42] + _ = x[SampledStmtDiagReqs-43] + _ = x[AddSSTableTombstones-44] + _ = x[SystemPrivilegesTable-45] + _ = x[EnablePredicateProjectionChangefeed-46] + _ = x[AlterSystemSQLInstancesAddLocality-47] + _ = x[SystemExternalConnectionsTable-48] + _ = x[AlterSystemStatementStatisticsAddIndexRecommendations-49] } -const _Key_name = "V21_2Start22_1PebbleFormatBlockPropertyCollectorProbeRequestPublicSchemasWithDescriptorsEnsureSpanConfigReconciliationEnsureSpanConfigSubscriptionEnableSpanConfigStoreSCRAMAuthenticationUnsafeLossOfQuorumRecoveryRangeLogAlterSystemProtectedTimestampAddColumnEnableProtectedTimestampsForTenantDeleteCommentsWithDroppedIndexesRemoveIncompatibleDatabasePrivilegesAddRaftAppliedIndexTermMigrationPostAddRaftAppliedIndexTermMigrationDontProposeWriteTimestampForLeaseTransfersEnablePebbleFormatVersionBlockPropertiesMVCCIndexBackfillerEnableLeaseHolderRemovalLooselyCoupledRaftLogTruncationChangefeedIdlenessEnableDeclarativeSchemaChangerRowLevelTTLPebbleFormatSplitUserKeysMarkedIncrementalBackupSubdirEnableNewStoreRebalancerClusterLocksVirtualTableAutoStatsTableSettingsSuperRegionsEnableNewChangefeedOptionsSpanCountTablePreSeedSpanCountTableSeedSpanCountTableV22_1Start22_2LocalTimestampsPebbleFormatSplitUserKeysMarkedCompactedEnsurePebbleFormatVersionRangeKeysEnablePebbleFormatVersionRangeKeysTrigramInvertedIndexesRemoveGrantPrivilegeMVCCRangeTombstonesUpgradeSequenceToBeReferencedByIDSampledStmtDiagReqsAddSSTableTombstonesSystemPrivilegesTableEnablePredicateProjectionChangefeedAlterSystemSQLInstancesAddLocalitySystemExternalConnectionsTableAlterSystemStatementStatisticsAddIndexRecommendations" +const _Key_name = "V21_2Start22_1PebbleFormatBlockPropertyCollectorProbeRequestPublicSchemasWithDescriptorsEnsureSpanConfigReconciliationEnsureSpanConfigSubscriptionEnableSpanConfigStoreSCRAMAuthenticationUnsafeLossOfQuorumRecoveryRangeLogAlterSystemProtectedTimestampAddColumnEnableProtectedTimestampsForTenantDeleteCommentsWithDroppedIndexesRemoveIncompatibleDatabasePrivilegesAddRaftAppliedIndexTermMigrationPostAddRaftAppliedIndexTermMigrationDontProposeWriteTimestampForLeaseTransfersEnablePebbleFormatVersionBlockPropertiesMVCCIndexBackfillerEnableLeaseHolderRemovalLooselyCoupledRaftLogTruncationChangefeedIdlenessEnableDeclarativeSchemaChangerRowLevelTTLPebbleFormatSplitUserKeysMarkedEnableNewStoreRebalancerClusterLocksVirtualTableAutoStatsTableSettingsSuperRegionsEnableNewChangefeedOptionsSpanCountTablePreSeedSpanCountTableSeedSpanCountTableV22_1Start22_2LocalTimestampsPebbleFormatSplitUserKeysMarkedCompactedEnsurePebbleFormatVersionRangeKeysEnablePebbleFormatVersionRangeKeysTrigramInvertedIndexesRemoveGrantPrivilegeMVCCRangeTombstonesUpgradeSequenceToBeReferencedByIDSampledStmtDiagReqsAddSSTableTombstonesSystemPrivilegesTableEnablePredicateProjectionChangefeedAlterSystemSQLInstancesAddLocalitySystemExternalConnectionsTableAlterSystemStatementStatisticsAddIndexRecommendations" -var _Key_index = [...]uint16{0, 5, 14, 48, 60, 88, 118, 146, 167, 186, 220, 258, 292, 324, 360, 392, 428, 470, 510, 529, 553, 584, 602, 632, 643, 674, 697, 721, 745, 767, 779, 805, 819, 840, 858, 863, 872, 887, 927, 961, 995, 1017, 1037, 1056, 1089, 1108, 1128, 1149, 1184, 1218, 1248, 1301} +var _Key_index = [...]uint16{0, 5, 14, 48, 60, 88, 118, 146, 167, 186, 220, 258, 292, 324, 360, 392, 428, 470, 510, 529, 553, 584, 602, 632, 643, 674, 698, 722, 744, 756, 782, 796, 817, 835, 840, 849, 864, 904, 938, 972, 994, 1014, 1033, 1066, 1085, 1105, 1126, 1161, 1195, 1225, 1278} func (i Key) String() string { if i < 0 || i >= Key(len(_Key_index)-1) {