Skip to content

Commit 0a5989d

Browse files
committed
backup: remove mixed-version check for incremental path suffix
The `start_time` suffix was added to incremental backup paths in 25.2 A mixed version check was added to ensure backups written during the mixed-version state when ugprading to 25.2 would be written correctly. This check is no longer needed. Epic: None Release note: None
1 parent 1aabeeb commit 0a5989d

File tree

2 files changed

+48
-33
lines changed

2 files changed

+48
-33
lines changed

pkg/backup/backupdest/backup_destination.go

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
2222
"github.com/cockroachdb/cockroach/pkg/cloud"
2323
"github.com/cockroachdb/cockroach/pkg/cloud/cloudpb"
24-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
2524
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
2625
"github.com/cockroachdb/cockroach/pkg/roachpb"
2726
"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -237,41 +236,40 @@ func ResolveDest(
237236
}
238237
prevBackupURIs = append([]string{plannedBackupDefaultURI}, prevBackupURIs...)
239238

240-
// Within the chosenSuffix dir, differentiate incremental backups with partName.
241-
partName := endTime.GoTime().Format(backupbase.DateBasedIncFolderName)
242-
if execCfg.Settings.Version.IsActive(ctx, clusterversion.V25_2) {
243-
if startTime.IsEmpty() {
244-
baseEncryptionOptions, err := backupencryption.GetEncryptionFromBase(
245-
ctx, user, execCfg.DistSQLSrv.ExternalStorageFromURI, prevBackupURIs[0],
246-
encryption, kmsEnv,
247-
)
248-
if err != nil {
249-
return ResolvedDestination{}, err
250-
}
239+
// If startTime is not already set, we will find it via the previous backup
240+
// manifest.
241+
if startTime.IsEmpty() {
242+
baseEncryptionOptions, err := backupencryption.GetEncryptionFromBase(
243+
ctx, user, execCfg.DistSQLSrv.ExternalStorageFromURI, prevBackupURIs[0],
244+
encryption, kmsEnv,
245+
)
246+
if err != nil {
247+
return ResolvedDestination{}, err
248+
}
251249

252-
// TODO (kev-cao): Once we have completed the backup directory index work, we
253-
// can remove the need to read an entire backup manifest just to fetch the
254-
// start time. We can instead read the metadata protobuf.
255-
mem := execCfg.RootMemoryMonitor.MakeBoundAccount()
256-
defer mem.Close(ctx)
257-
precedingBackupManifest, size, err := backupinfo.ReadBackupManifestFromURI(
258-
ctx, &mem, prevBackupURIs[len(prevBackupURIs)-1], user,
259-
execCfg.DistSQLSrv.ExternalStorageFromURI, baseEncryptionOptions, kmsEnv,
260-
)
261-
if err != nil {
262-
return ResolvedDestination{}, err
263-
}
264-
if err := mem.Grow(ctx, size); err != nil {
265-
return ResolvedDestination{}, err
266-
}
267-
defer mem.Shrink(ctx, size)
268-
startTime = precedingBackupManifest.EndTime
269-
if startTime.IsEmpty() {
270-
return ResolvedDestination{}, errors.Errorf("empty end time in prior backup manifest")
271-
}
250+
// TODO (kev-cao): Once we have completed the backup directory index work, we
251+
// can remove the need to read an entire backup manifest just to fetch the
252+
// start time. We can instead read the metadata protobuf.
253+
mem := execCfg.RootMemoryMonitor.MakeBoundAccount()
254+
defer mem.Close(ctx)
255+
precedingBackupManifest, size, err := backupinfo.ReadBackupManifestFromURI(
256+
ctx, &mem, prevBackupURIs[len(prevBackupURIs)-1], user,
257+
execCfg.DistSQLSrv.ExternalStorageFromURI, baseEncryptionOptions, kmsEnv,
258+
)
259+
if err != nil {
260+
return ResolvedDestination{}, err
261+
}
262+
if err := mem.Grow(ctx, size); err != nil {
263+
return ResolvedDestination{}, err
264+
}
265+
defer mem.Shrink(ctx, size)
266+
startTime = precedingBackupManifest.EndTime
267+
if startTime.IsEmpty() {
268+
return ResolvedDestination{}, errors.Errorf("empty end time in prior backup manifest")
272269
}
273-
partName = partName + "-" + startTime.GoTime().Format(backupbase.DateBasedIncFolderNameSuffix)
274270
}
271+
272+
partName := ConstructDateBasedIncrementalFolderName(startTime.GoTime(), endTime.GoTime())
275273
defaultIncrementalsURI, urisByLocalityKV, err := GetURIsByLocalityKV(fullyResolvedIncrementalsLocation, partName)
276274
if err != nil {
277275
return ResolvedDestination{}, err

pkg/backup/backupdest/incrementals.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ package backupdest
77

88
import (
99
"context"
10+
"fmt"
1011
"path"
1112
"regexp"
1213
"sort"
1314
"strings"
15+
"time"
1416

1517
"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
1618
"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
@@ -248,3 +250,18 @@ func ResolveDefaultBaseIncrementalStorageLocation(
248250

249251
return defaultURI, nil
250252
}
253+
254+
// ConstructDateBasedIncrementalFolderName constructs the name of a date-based
255+
// incremental backup folder relative to the full subdirectory it belongs to.
256+
//
257+
// /2025/07/30-120000.00/20250730/130000.00-20250730-120000.00
258+
//
259+
// └─────────────────────────────────────┘
260+
// returns this
261+
func ConstructDateBasedIncrementalFolderName(start, end time.Time) string {
262+
return fmt.Sprintf(
263+
"%s-%s",
264+
end.Format(backupbase.DateBasedIncFolderName),
265+
start.Format(backupbase.DateBasedIncFolderNameSuffix),
266+
)
267+
}

0 commit comments

Comments
 (0)