Merge #151681

151681: backup: do not write backup indexes for custom incremental locations r=msbutler a=kev-cao

Due to the plan to deprecate `incremental_location`, we do not want to write a backup index for any incremental backup that was written to a custom incremental location. This keeps the development of backup indexes simpler as we do not have to worry about determining which location the index belongs to.

Epic: [CRDB-47942](https://cockroachlabs.atlassian.net/browse/CRDB-47942)

Release note: None

Co-authored-by: Kevin Cao <[email protected]>

Author: craig[bot]

pkg/backup/backupdest/backup_index.go

@@ -116,8 +116,8 @@ func WriteBackupIndexMetadata(
 // The store should be rooted at the default collection URI (the one that
 // contains the `index/` directory).
 //
-// Note: v25.4+ backups will always contain an index file. In other words, we
-// can remove these checks in v26.2+.
+// TODO (kev-cao): v25.4+ backups will always contain an index file. In other
+// words, we can remove these checks in v26.2+.
 func IndexExists(ctx context.Context, store cloud.ExternalStorage, subdir string) (bool, error) {
 	var indexExists bool
 	indexSubdir := path.Join(backupbase.BackupIndexDirectoryPath, flattenSubdirForIndex(subdir))
@@ -159,6 +159,16 @@ func shouldWriteIndex(
 		return false, nil
 	}
 
+	// As we are going to be deprecating the `incremental_location` option, we
+	// will avoid writing an index for any backups that specify an `incremental`
+	// location. Note that if `incremental_location` is explicitly set to the
+	// default location, then we will have some backups containing an index and
+	// others not. We are treating this as an unsupported state and the user
+	// should not use `incremental_location` in this manner.
+	if len(details.Destination.IncrementalStorage) != 0 {
+		return false, nil
+	}
+
 	// Full backups can write an index as long as the cluster is on v25.4+.
 	if details.StartTime.IsEmpty() {
 		return true, nil

pkg/backup/backupdest/backup_index_test.go

@@ -171,7 +171,7 @@ func TestWriteBackupIndexMetadata(t *testing.T) {
 	require.Equal(t, subdir, metadata.Path)
 }
 
-func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testing.T) {
+func TestWriteBackupIndexMetadataWithLocalityAwareBackups(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
@@ -184,13 +184,9 @@ func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testi
 
 	collections := `('nodelocal://1/us-west?COCKROACH_LOCALITY=region%3Dus-west',
 		'nodelocal://1/us-east?COCKROACH_LOCALITY=default')`
-	incStorage := `('nodelocal://1/us-west/inc?COCKROACH_LOCALITY=region%3Dus-west',
-		'nodelocal://1/us-east-inc?COCKROACH_LOCALITY=default')`
 
-	sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO %s WITH incremental_location = %s`, collections, incStorage))
-	sqlDB.Exec(t, fmt.Sprintf(
-		`BACKUP INTO LATEST IN %s WITH incremental_location = %s`, collections, incStorage,
-	))
+	sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO %s`, collections))
+	sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO LATEST IN %s`, collections))
 
 	indexDir := path.Join(tempDir, "us-east", backupbase.BackupIndexDirectoryPath)
 	fullIndexes, err := os.ReadDir(indexDir)
@@ -226,6 +222,36 @@ func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testi
 	require.True(t, strings.HasPrefix(incrIndex.Path, fullIndex.Path))
 }
 
+func TestWriteBackupindexMetadataWithSpecifiedIncrementalLocation(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	tempDir, tempDirCleanup := testutils.TempDir(t)
+	defer tempDirCleanup()
+	_, sqlDB, _, cleanup := backuptestutils.StartBackupRestoreTestCluster(
+		t, 1, backuptestutils.WithTempDir(tempDir),
+	)
+	defer cleanup()
+
+	const collectionURI = "nodelocal://1/backup"
+	const incLoc = "nodelocal://1/incremental_backup"
+
+	sqlDB.Exec(t, "BACKUP INTO $1", collectionURI)
+	sqlDB.Exec(t, "BACKUP INTO LATEST IN $1 WITH incremental_location=$2", collectionURI, incLoc)
+
+	indexDir := path.Join(tempDir, "backup", backupbase.BackupIndexDirectoryPath)
+	fullIndexes, err := os.ReadDir(indexDir)
+	require.NoError(t, err)
+	require.Len(t, fullIndexes, 1)
+
+	chainIndexes, err := os.ReadDir(path.Join(indexDir, fullIndexes[0].Name()))
+	require.NoError(t, err)
+
+	// Since we specified an incremental location, we should not see an index
+	// being written for the incremental backup.
+	require.Len(t, chainIndexes, 1)
+}
+
 func TestDontWriteBackupIndexMetadata(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)