Skip to content

Commit 04ef14b

Browse files
committed
backup: do not write backup indexes for custom incremental locations
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 Release note: None
1 parent cd43ab3 commit 04ef14b

File tree

2 files changed

+45
-9
lines changed

2 files changed

+45
-9
lines changed

pkg/backup/backupdest/backup_index.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ func WriteBackupIndexMetadata(
116116
// The store should be rooted at the default collection URI (the one that
117117
// contains the `index/` directory).
118118
//
119-
// Note: v25.4+ backups will always contain an index file. In other words, we
120-
// can remove these checks in v26.2+.
119+
// TODO (kev-cao): v25.4+ backups will always contain an index file. In other
120+
// words, we can remove these checks in v26.2+.
121121
func IndexExists(ctx context.Context, store cloud.ExternalStorage, subdir string) (bool, error) {
122122
var indexExists bool
123123
indexSubdir := path.Join(backupbase.BackupIndexDirectoryPath, flattenSubdirForIndex(subdir))
@@ -159,6 +159,16 @@ func shouldWriteIndex(
159159
return false, nil
160160
}
161161

162+
// As we are going to be deprecating the `incremental_location` option, we
163+
// will avoid writing an index for any backups that specify an `incremental`
164+
// location. Note that if `incremental_location` is explicitly set to the
165+
// default location, then we will have some backups containing an index and
166+
// others not. We are treating this as an unsupported state and the user
167+
// should not use `incremental_location` in this manner.
168+
if len(details.Destination.IncrementalStorage) != 0 {
169+
return false, nil
170+
}
171+
162172
// Full backups can write an index as long as the cluster is on v25.4+.
163173
if details.StartTime.IsEmpty() {
164174
return true, nil

pkg/backup/backupdest/backup_index_test.go

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func TestWriteBackupIndexMetadata(t *testing.T) {
171171
require.Equal(t, subdir, metadata.Path)
172172
}
173173

174-
func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testing.T) {
174+
func TestWriteBackupIndexMetadataWithLocalityAwareBackups(t *testing.T) {
175175
defer leaktest.AfterTest(t)()
176176
defer log.Scope(t).Close(t)
177177

@@ -184,13 +184,9 @@ func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testi
184184

185185
collections := `('nodelocal://1/us-west?COCKROACH_LOCALITY=region%3Dus-west',
186186
'nodelocal://1/us-east?COCKROACH_LOCALITY=default')`
187-
incStorage := `('nodelocal://1/us-west/inc?COCKROACH_LOCALITY=region%3Dus-west',
188-
'nodelocal://1/us-east-inc?COCKROACH_LOCALITY=default')`
189187

190-
sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO %s WITH incremental_location = %s`, collections, incStorage))
191-
sqlDB.Exec(t, fmt.Sprintf(
192-
`BACKUP INTO LATEST IN %s WITH incremental_location = %s`, collections, incStorage,
193-
))
188+
sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO %s`, collections))
189+
sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO LATEST IN %s`, collections))
194190

195191
indexDir := path.Join(tempDir, "us-east", backupbase.BackupIndexDirectoryPath)
196192
fullIndexes, err := os.ReadDir(indexDir)
@@ -226,6 +222,36 @@ func TestWriteBackupIndexMetadataWithLocalityAwareAndIncrementalStorage(t *testi
226222
require.True(t, strings.HasPrefix(incrIndex.Path, fullIndex.Path))
227223
}
228224

225+
func TestWriteBackupindexMetadataWithSpecifiedIncrementalLocation(t *testing.T) {
226+
defer leaktest.AfterTest(t)()
227+
defer log.Scope(t).Close(t)
228+
229+
tempDir, tempDirCleanup := testutils.TempDir(t)
230+
defer tempDirCleanup()
231+
_, sqlDB, _, cleanup := backuptestutils.StartBackupRestoreTestCluster(
232+
t, 1, backuptestutils.WithTempDir(tempDir),
233+
)
234+
defer cleanup()
235+
236+
const collectionURI = "nodelocal://1/backup"
237+
const incLoc = "nodelocal://1/incremental_backup"
238+
239+
sqlDB.Exec(t, "BACKUP INTO $1", collectionURI)
240+
sqlDB.Exec(t, "BACKUP INTO LATEST IN $1 WITH incremental_location=$2", collectionURI, incLoc)
241+
242+
indexDir := path.Join(tempDir, "backup", backupbase.BackupIndexDirectoryPath)
243+
fullIndexes, err := os.ReadDir(indexDir)
244+
require.NoError(t, err)
245+
require.Len(t, fullIndexes, 1)
246+
247+
chainIndexes, err := os.ReadDir(path.Join(indexDir, fullIndexes[0].Name()))
248+
require.NoError(t, err)
249+
250+
// Since we specified an incremental location, we should not see an index
251+
// being written for the incremental backup.
252+
require.Len(t, chainIndexes, 1)
253+
}
254+
229255
func TestDontWriteBackupIndexMetadata(t *testing.T) {
230256
defer leaktest.AfterTest(t)()
231257
defer log.Scope(t).Close(t)

0 commit comments

Comments
 (0)