backup: add cluster setting to skip compacted backups during restore

This patch adds the cluster setting `restore.compacted_backups.enabled`
to allow skipping compacted backups during a restore operation.

Epic: None

Release note: None

Author: kev-cao

pkg/backup/backupdest/backup_destination.go

@@ -534,7 +534,9 @@ func ListFullBackupsInCollection(
 // that are then expanded into the result layers returned, similar to if those
 // layers had been specified in `from` explicitly. If `includeSkipped` is true,
 // layers that do not actually contribute to the path from the base to the end
-// timestamp are included in the result, otherwise they are elided.
+// timestamp are included in the result, otherwise they are elided. If
+// `includedCompacted` is true, then backups created from compaction will be
+// included in the result, otherwise they are filtered out.
 func ResolveBackupManifests(
 	ctx context.Context,
 	mem *mon.BoundAccount,
@@ -548,6 +550,7 @@ func ResolveBackupManifests(
 	kmsEnv cloud.KMSEnv,
 	user username.SQLUsername,
 	includeSkipped bool,
+	includeCompacted bool,
 ) (
 	defaultURIs []string,
 	// mainBackupManifests contains the manifest located at each defaultURI in the backup chain.
@@ -656,9 +659,9 @@ func ResolveBackupManifests(
 
 	totalMemSize := ownedMemSize
 	ownedMemSize = 0
-
 	validatedDefaultURIs, validatedMainBackupManifests, validatedLocalityInfo, err := backupinfo.ValidateEndTimeAndTruncate(
-		defaultURIs, mainBackupManifests, localityInfo, endTime, includeSkipped)
+		defaultURIs, mainBackupManifests, localityInfo, endTime, includeSkipped, includeCompacted,
+	)
 
 	if err != nil {
 		return nil, nil, nil, 0, err

pkg/backup/backupinfo/BUILD.bazel

@@ -38,6 +38,7 @@ go_library(
         "//pkg/sql/sem/tree",
         "//pkg/sql/stats",
         "//pkg/storage",
+        "//pkg/util",
         "//pkg/util/bulk",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",

pkg/backup/backupinfo/manifest_handling.go

@@ -42,6 +42,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/stats"
+	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/bulk"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
@@ -886,7 +887,16 @@ func ValidateEndTimeAndTruncate(
 	localityInfo []jobspb.RestoreDetails_BackupLocalityInfo,
 	endTime hlc.Timestamp,
 	includeSkipped bool,
+	includeCompacted bool,
 ) ([]string, []backuppb.BackupManifest, []jobspb.RestoreDetails_BackupLocalityInfo, error) {
+	if !includeCompacted {
+		mainBackupManifests = util.Filter(
+			mainBackupManifests,
+			func(manifest backuppb.BackupManifest) bool {
+				return !manifest.IsCompacted
+			},
+		)
+	}
 
 	if endTime.IsEmpty() {
 		if includeSkipped {
@@ -896,7 +906,7 @@ func ValidateEndTimeAndTruncate(
 		if err != nil {
 			return nil, nil, nil, err
 		}
-		if err := validateContinuity(manifests, endTime); err != nil {
+		if err := validateContinuity(manifests); err != nil {
 			return nil, nil, nil, err
 		}
 		return uris, manifests, locality, nil
@@ -944,21 +954,19 @@ func ValidateEndTimeAndTruncate(
 		if err != nil {
 			return nil, nil, nil, err
 		}
-		if err := validateContinuity(manifests, endTime); err != nil {
+		if err := validateContinuity(manifests); err != nil {
 			return nil, nil, nil, err
 		}
 		return uris, manifests, locality, nil
-
 	}
 
 	return nil, nil, nil, errors.Errorf(
 		"invalid RESTORE timestamp: supplied backups do not cover requested time",
 	)
 }
 
-// ValidateContinuity checks that the backups are continuous and cover the
-// requested end time.
-func validateContinuity(manifests []backuppb.BackupManifest, endTime hlc.Timestamp) error {
+// validateContinuity checks that the backups are continuous.
+func validateContinuity(manifests []backuppb.BackupManifest) error {
 	if len(manifests) == 0 {
 		return errors.AssertionFailedf("an empty chain of backups cannot cover an end time")
 	}
@@ -971,15 +979,6 @@ func validateContinuity(manifests []backuppb.BackupManifest, endTime hlc.Timesta
 			)
 		}
 	}
-	if !endTime.IsEmpty() {
-		lastManifest := manifests[len(manifests)-1]
-		if !lastManifest.StartTime.Less(endTime) || !endTime.LessEq(lastManifest.EndTime) {
-			return errors.AssertionFailedf(
-				"requested time %s is not covered by the last backup",
-				endTime,
-			)
-		}
-	}
 	return nil
 }
 
@@ -1000,7 +999,8 @@ func ElideSkippedLayers(
 			j--
 		}
 		// If there are backups between i and j, remove them.
-		if j != i-1 {
+		// If j is less than 0, then no parent was found so nothing to skip.
+		if j != i-1 && j >= 0 {
 			uris = slices.Delete(uris, j+1, i)
 			backups = slices.Delete(backups, j+1, i)
 			loc = slices.Delete(loc, j+1, i)

pkg/backup/backupinfo/manifest_handling_test.go

@@ -376,62 +376,191 @@ func TestMakeBackupCodec(t *testing.T) {
 	}
 }
 
-func TestElideSkippedLayers(t *testing.T) {
+func TestValidateEndTimeAndTruncate(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	m := func(start, end int, compacted bool, revision bool) backuppb.BackupManifest {
+		b := backuppb.BackupManifest{
+			StartTime:   hlc.Timestamp{WallTime: int64(start)},
+			EndTime:     hlc.Timestamp{WallTime: int64(end)},
+			IsCompacted: compacted,
+		}
+		if revision {
+			b.MVCCFilter = backuppb.MVCCFilter_All
+			b.RevisionStartTime = hlc.Timestamp{WallTime: int64(start)}
+		}
+		return b
+	}
+	mNorm := func(start, end int) backuppb.BackupManifest {
+		return m(start, end, false /* compacted */, false /* revision */)
+	}
+	mComp := func(start, end int) backuppb.BackupManifest {
+		return m(start, end, true /* compacted */, false /* revision */)
+	}
+	mRev := func(start, end int) backuppb.BackupManifest {
+		return m(start, end, false /* compacted */, true /* revision */)
+	}
 
-	// Note: The tests here work under the assumption that the input lists are
+	// Note: The tests here work under the assumption that the input manifests are
 	// always sorted in ascending order by end time, and then sorted in ascending
 	// order by start time.
 	for _, tc := range []struct {
-		name     string
-		times    [][]int // len 2 slices of start and end time.
-		expected [][]int // expected start and end times
+		name             string
+		manifests        []backuppb.BackupManifest
+		endTime          int
+		includeCompacted bool
+		err              string
+		expected         [][]int // expected timestamps of returned backups
 	}{
-		{"single", [][]int{{0, 1}}, [][]int{{0, 1}}},
-		{"double", [][]int{{0, 1}, {1, 2}}, [][]int{{0, 1}, {1, 2}}},
 		{
-			"simple chain, no skips",
-			[][]int{{0, 1}, {1, 2}, {2, 3}, {3, 5}, {5, 8}},
-			[][]int{{0, 1}, {1, 2}, {2, 3}, {3, 5}, {5, 8}},
+			name: "single backup",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1),
+			},
+			endTime:  1,
+			expected: [][]int{{0, 1}},
+		},
+		{
+			name: "double backup",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2),
+			},
+			endTime:  2,
+			expected: [][]int{{0, 1}, {1, 2}},
+		},
+		{
+			name: "out of bounds end time",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2),
+			},
+			endTime: 3,
+			err:     "supplied backups do not cover requested time",
 		},
 		{
-			"compaction of two backups",
-			[][]int{{0, 1}, {1, 2}, {1, 3}, {2, 3}, {3, 5}, {5, 8}},
-			[][]int{{0, 1}, {1, 3}, {3, 5}, {5, 8}},
+			name: "revision history restore should fail on non-revision history backups",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 2), mNorm(2, 4),
+			},
+			endTime: 3,
+			err:     "restoring to arbitrary time",
 		},
 		{
-			"compaction of entire chain",
-			[][]int{{0, 1}, {1, 2}, {2, 3}, {3, 5}, {0, 8}, {5, 8}},
-			[][]int{{0, 8}},
+			name: "revision history restore should succeed on revision history backups",
+			manifests: []backuppb.BackupManifest{
+				mRev(0, 2), mRev(2, 4),
+			},
+			endTime:  3,
+			expected: [][]int{{0, 2}, {2, 4}},
 		},
 		{
-			"two compactions of two backups",
-			[][]int{{0, 1}, {1, 2}, {1, 3}, {2, 3}, {3, 5}, {3, 8}, {5, 8}},
-			[][]int{{0, 1}, {1, 3}, {3, 8}},
+			name: "end time in middle of chain should truncate",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mNorm(2, 3),
+				mNorm(3, 5), mNorm(5, 8),
+			},
+			endTime:  3,
+			expected: [][]int{{0, 1}, {1, 2}, {2, 3}},
 		},
 		{
-			"compaction includes a compacted backup in the middle",
-			[][]int{{0, 1}, {1, 2}, {1, 3}, {2, 3}, {3, 5}, {1, 8}, {5, 8}},
-			[][]int{{0, 1}, {1, 8}},
+			name: "non-continuous backup chain should fail",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(2, 3),
+			},
+			endTime: 3,
+			err:     "backups are not continuous",
 		},
 		{
-			"two compactions with the same end time",
-			[][]int{{0, 1}, {1, 2}, {2, 3}, {3, 5}, {1, 8}, {3, 8}, {5, 8}},
-			[][]int{{0, 1}, {1, 8}},
+			name: "ignore compacted backups if includeCompacted is false",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mComp(1, 3), mNorm(2, 3),
+			},
+			endTime:  3,
+			expected: [][]int{{0, 1}, {1, 2}, {2, 3}},
+		},
+		{
+			name: "compaction of two backups",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mComp(1, 3), mNorm(2, 3),
+				mNorm(3, 5), mNorm(5, 8),
+			},
+			endTime:          8,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 3}, {3, 5}, {5, 8}},
+		},
+		{
+			name: "compaction of entire incremental chain",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mNorm(2, 3), mNorm(3, 5),
+				mComp(1, 8), mNorm(5, 8),
+			},
+			endTime:          8,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 8}},
+		},
+		{
+			name: "two separate compactions of two backups",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mComp(1, 3), mNorm(2, 3),
+				mNorm(3, 5), mComp(3, 8), mNorm(5, 8),
+			},
+			endTime:          8,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 3}, {3, 8}},
+		},
+		{
+			name: "compaction includes a compacted backup in the middle",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mComp(1, 3), mNorm(2, 3),
+				mNorm(3, 5), mComp(1, 8), mNorm(5, 8),
+			},
+			endTime:          8,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 8}},
+		},
+		{
+			name: "two compactions with the same end time",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mNorm(2, 3), mNorm(3, 5),
+				mComp(1, 8), mComp(3, 8), mNorm(5, 8),
+			},
+			endTime:          8,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 8}},
+		},
+		{
+			name: "end time in middle of compacted chain should pick base incremental",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mNorm(2, 3),
+				mComp(1, 5), mNorm(3, 5),
+			},
+			endTime:          3,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 2}, {2, 3}},
+		},
+		{
+			name: "overlapping compacted backups",
+			manifests: []backuppb.BackupManifest{
+				mNorm(0, 1), mNorm(1, 2), mComp(1, 3), mNorm(2, 3), mComp(2, 4), mNorm(3, 4),
+			},
+			endTime:          4,
+			includeCompacted: true,
+			expected:         [][]int{{0, 1}, {1, 2}, {2, 4}},
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			chain := make([]backuppb.BackupManifest, len(tc.times))
-			for i, ts := range tc.times {
-				chain[i].StartTime = hlc.Timestamp{WallTime: int64(ts[0])}
-				chain[i].EndTime = hlc.Timestamp{WallTime: int64(ts[1])}
-			}
-			uris, res, locs, err := backupinfo.ElideSkippedLayers(
-				make([]string, len(tc.times)),
-				chain,
-				make([]jobspb.RestoreDetails_BackupLocalityInfo, len(tc.times)),
+			uris, res, locs, err := backupinfo.ValidateEndTimeAndTruncate(
+				make([]string, len(tc.manifests)),
+				tc.manifests,
+				make([]jobspb.RestoreDetails_BackupLocalityInfo, len(tc.manifests)),
+				hlc.Timestamp{WallTime: int64(tc.endTime)},
+				false, /* includeSkipped */
+				tc.includeCompacted,
 			)
-			require.NoError(t, err)
+			if tc.err != "" {
+				require.ErrorContains(t, err, tc.err)
+				return
+			}
 			require.Equal(t, len(tc.expected), len(uris))
 			require.Equal(t, len(tc.expected), len(locs))
 			require.Equal(t, len(tc.expected), len(res))

pkg/backup/compaction_job.go

@@ -692,7 +692,7 @@ func getBackupChain(
 	_, manifests, localityInfo, memReserved, err := backupdest.ResolveBackupManifests(
 		ctx, &mem, baseStores, incStores, mkStore, resolvedBaseDirs,
 		resolvedIncDirs, endTime, baseEncryptionInfo, kmsEnv,
-		user, false,
+		user, false /*includeSkipped */, true, /*includeCompacted */
 	)
 	if err != nil {
 		return nil, nil, nil, nil, err