Skip to content

Commit a7d85da

Browse files
craig[bot]msbutlerjbowens
committed
153474: backupdest: remove old manifest format from 20.1 r=jeffswenson a=msbutler backup: rename BackupManifestName DeprecatedBackupManifestName Informs #139159 Release note: none --- backup: always write slim manifest Previously, backups would write the slim manifest if the a default true cluster setting was true. This patch removes this cluster setting, so backups will always write the slim manifest. This work is part of an effort to remove deprecated backup manifest versions. Since this patch will take effect on 25.4 clusters, backup _reads_ on 26.2 clusters can exclusively use the slim manifest without any failback logic to the deprecated fat manifest. Since there still exists logic to read the deprecated manifest exclusively (e.g. SHOW BACKUPS), we cannot remove the deprecated manifest _write_ path until all these read paths have fallback logic to the new slim manifest-- work for an upcoming PR. Informs #139159 Release note: none --- backup: remove old manifest format from 20.1 Informs: #139159 Release note: none 153531: workload/ycsb: use crrand.Perm64 for pseudorandom keys r=RaduBerinde a=jbowens This commit reworks the random ycsb key generation to use a pseudorandom permutation of 64-bit integers constructed deterministically from the seed, rather than hashing the key index. The pseudorandom permutation is provided by the crrand.Perm64 type. The per-key computation is likely lighterweight than the previous fnv hash, but more importantly, it avoids duplicates. This ensures that a large IMPORT of random keys will not fail due to a key uniqueness violation. This commit also accordingly renames the --insert-hash flag to --insert-random (leaving an alias with the old name). Epic: none Release note: none Co-authored-by: Michael Butler <[email protected]> Co-authored-by: Jackson Owens <[email protected]>
3 parents 277f3f5 + 3b69608 + 16e2453 commit a7d85da

21 files changed

+99
-219
lines changed

DEPS.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,10 +1740,10 @@ def go_deps():
17401740
name = "com_github_cockroachdb_crlib",
17411741
build_file_proto_mode = "disable_global",
17421742
importpath = "github.com/cockroachdb/crlib",
1743-
sha256 = "4fb4d32f15fb3c63decb9625406f291874f975518d5937493a7a9d1f3a644f18",
1744-
strip_prefix = "github.com/cockroachdb/[email protected]20250718215705-7ff5051265b9",
1743+
sha256 = "3481f073a07aad90b3e66e357ed9b1212fb649b2653d6ce91a7d6a345632a9e4",
1744+
strip_prefix = "github.com/cockroachdb/[email protected]20250914170308-eb2d55dbae2e",
17451745
urls = [
1746-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlib/com_github_cockroachdb_crlib-v0.0.0-20250718215705-7ff5051265b9.zip",
1746+
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlib/com_github_cockroachdb_crlib-v0.0.0-20250914170308-eb2d55dbae2e.zip",
17471747
],
17481748
)
17491749
go_repository(

build/bazelutil/distdir_files.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ DISTDIR_FILES = {
345345
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cmux/com_github_cockroachdb_cmux-v0.0.0-20250514152509-914d3bf9ec58.zip": "c1cf4cd99a1ad6a00f2ccd4188cbcf004cb0d56895670b2c171061ce564cd791",
346346
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.4.1.zip": "ba646db91152f3121a6812c7b74d12d8c0e126f7b4d3b927618b159692ceb424",
347347
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlfmt/com_github_cockroachdb_crlfmt-v0.0.0-20221214225007-b2fc5c302548.zip": "fedc01bdd6d964da0425d5eaac8efadc951e78e13f102292cc0774197f09ab63",
348-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlib/com_github_cockroachdb_crlib-v0.0.0-20250718215705-7ff5051265b9.zip": "4fb4d32f15fb3c63decb9625406f291874f975518d5937493a7a9d1f3a644f18",
348+
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlib/com_github_cockroachdb_crlib-v0.0.0-20250914170308-eb2d55dbae2e.zip": "3481f073a07aad90b3e66e357ed9b1212fb649b2653d6ce91a7d6a345632a9e4",
349349
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20250407164829-2945557346d5.zip": "251593cd9c040fe84a99a3919de7ce6f85030d522159a37d625dc2dea7a4d17f",
350350
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/drpc/com_github_cockroachdb_drpc-v0.0.0-20250807091527-65dcebaa113e.zip": "ebb4b3ca9a6c5944255506b4cf5091b86b76851e52211940b609c98e8bcc7aab",
351351
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/errors/com_github_cockroachdb_errors-v1.12.0.zip": "f73d8a5f4d8fcbc4ed61db2b47f17e2601d8b32e9a49c0665667489d9d9d6e7c",

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ require (
135135
github.com/cockroachdb/cmux v0.0.0-20250514152509-914d3bf9ec58
136136
github.com/cockroachdb/cockroach-go/v2 v2.4.1
137137
github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548
138-
github.com/cockroachdb/crlib v0.0.0-20250718215705-7ff5051265b9
138+
github.com/cockroachdb/crlib v0.0.0-20250914170308-eb2d55dbae2e
139139
github.com/cockroachdb/datadriven v1.0.3-0.20250407164829-2945557346d5
140140
github.com/cockroachdb/errors v1.12.0
141141
github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,8 +553,8 @@ github.com/cockroachdb/cockroach-go/v2 v2.4.1 h1:ACVT/zXsuK6waRPVYtDQpsM8pPA7IA/
553553
github.com/cockroachdb/cockroach-go/v2 v2.4.1/go.mod h1:9U179XbCx4qFWtNhc7BiWLPfuyMVQ7qdAhfrwLz1vH0=
554554
github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548 h1:i0bnjanlWAvM50wHMT7EFyxlt5HQusznWrkwl+HBIsU=
555555
github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548/go.mod h1:qtkxNlt5i3rrdirfJE/bQeW/IeLajKexErv7jEIV+Uc=
556-
github.com/cockroachdb/crlib v0.0.0-20250718215705-7ff5051265b9 h1:eNjHrT3pPlP1q/4VT/IRGzl8Jiq1XHrSpLJmrGFJDT8=
557-
github.com/cockroachdb/crlib v0.0.0-20250718215705-7ff5051265b9/go.mod h1:Gq51ZeKaFCXk6QwuGM0w1dnaOqc/F5zKT2zA9D6Xeac=
556+
github.com/cockroachdb/crlib v0.0.0-20250914170308-eb2d55dbae2e h1:qn+p2EISsM2HjMDt6Ivx5W3CG/jCz/JSCeLnF9vJju8=
557+
github.com/cockroachdb/crlib v0.0.0-20250914170308-eb2d55dbae2e/go.mod h1:Gq51ZeKaFCXk6QwuGM0w1dnaOqc/F5zKT2zA9D6Xeac=
558558
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
559559
github.com/cockroachdb/datadriven v1.0.2/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU=
560560
github.com/cockroachdb/datadriven v1.0.3-0.20250407164829-2945557346d5 h1:UycK/E0TkisVrQbSoxvU827FwgBBcZ95nRRmpj/12QI=

pkg/backup/backup_job.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -427,24 +427,16 @@ func backup(
427427
}
428428
}
429429

430-
// Write a `BACKUP_MANIFEST` file to support backups in mixed-version clusters
431-
// with 22.2 nodes.
432-
//
433-
// TODO(adityamaru): We can stop writing `BACKUP_MANIFEST` in 23.2
434-
// because a mixed-version cluster with 23.1 nodes will read the
435-
// `BACKUP_METADATA` instead.
436-
if err := backupinfo.WriteBackupManifest(ctx, defaultStore, backupbase.BackupManifestName,
430+
// TODO(msbutler): version gate writing the old manifest once we can guarantee
431+
// a cluster version that will not read the old manifest.
432+
if err := backupinfo.WriteBackupManifest(ctx, defaultStore, backupbase.DeprecatedBackupManifestName,
437433
encryption, &kmsEnv, backupManifest); err != nil {
438434
return roachpb.RowCount{}, 0, err
439435
}
440436

441-
// Write a `BACKUP_METADATA` file along with SSTs for all the alloc heavy
442-
// fields elided from the `BACKUP_MANIFEST`.
443-
if backupinfo.WriteMetadataWithExternalSSTsEnabled.Get(&settings.SV) {
444-
if err := backupinfo.WriteMetadataWithExternalSSTs(ctx, defaultStore, encryption,
445-
&kmsEnv, backupManifest); err != nil {
446-
return roachpb.RowCount{}, 0, err
447-
}
437+
if err := backupinfo.WriteMetadataWithExternalSSTs(ctx, defaultStore, encryption,
438+
&kmsEnv, backupManifest); err != nil {
439+
return roachpb.RowCount{}, 0, err
448440
}
449441

450442
statsTable := getTableStatsForBackup(ctx, execCtx.ExecCfg().InternalDB.Executor(), backupManifest.Descriptors)

pkg/backup/backup_test.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func TestBackupRestoreStatementResult(t *testing.T) {
144144
// have been stored in the GZip compressed format.
145145
t.Run("GZipBackupManifest", func(t *testing.T) {
146146
backupDir := fmt.Sprintf("%s/foo", dir)
147-
backupManifestFile := backupDir + backupPath + "/" + backupbase.BackupManifestName
147+
backupManifestFile := backupDir + backupPath + "/" + backupbase.DeprecatedBackupManifestName
148148
backupManifestBytes, err := os.ReadFile(backupManifestFile)
149149
if err != nil {
150150
t.Fatal(err)
@@ -668,7 +668,7 @@ func TestBackupRestoreAppend(t *testing.T) {
668668
// Find the backup times in the collection and try RESTORE'ing to each, and
669669
// within each also check if we can restore to individual times captured with
670670
// incremental backups that were appended to that backup.
671-
fullBackup1, fullBackup2 := findFullBackupPaths(tmpDir, path.Join(tmpDir, "*/*/*/"+backupbase.BackupManifestName))
671+
fullBackup1, fullBackup2 := findFullBackupPaths(tmpDir, path.Join(tmpDir, "*/*/*/"+backupbase.DeprecatedBackupManifestName))
672672

673673
sqlDB.Exec(t, "DROP DATABASE data CASCADE")
674674
sqlDB.Exec(t, "RESTORE DATABASE data FROM $4 IN ($1, $2, $3) AS OF SYSTEM TIME "+tsBefore,
@@ -740,7 +740,7 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
740740
sqlDB.Exec(t, "BACKUP INTO ($1, $2, $3) AS OF SYSTEM TIME '-1s'", collections...)
741741

742742
// Find the subdirectory created by the full BACKUP INTO statement.
743-
matches, err := filepath.Glob(path.Join(tmpDir, "full/*/*/*/"+backupbase.BackupManifestName))
743+
matches, err := filepath.Glob(path.Join(tmpDir, "full/*/*/*/"+backupbase.DeprecatedBackupManifestName))
744744
require.NoError(t, err)
745745
require.Equal(t, 2, len(matches))
746746
for i := range matches {
@@ -1786,7 +1786,7 @@ func TestBackupRestoreResume(t *testing.T) {
17861786

17871787
// If the backup properly took the (incorrect) checkpoint into account, it
17881788
// won't have tried to re-export any keys within backupCompletedSpan.
1789-
backupManifestFile := path.Join(dir, item.testName, subdir, backupbase.BackupManifestName)
1789+
backupManifestFile := path.Join(dir, item.testName, subdir, backupbase.DeprecatedBackupManifestName)
17901790
backupManifestBytes, err := os.ReadFile(backupManifestFile)
17911791
if err != nil {
17921792
t.Fatal(err)
@@ -4000,7 +4000,7 @@ func TestBackupRestoreChecksum(t *testing.T) {
40004000

40014001
var backupManifest backuppb.BackupManifest
40024002
{
4003-
backupManifestBytes, err := os.ReadFile(filepath.Join(dir, backupPath, backupbase.BackupManifestName))
4003+
backupManifestBytes, err := os.ReadFile(filepath.Join(dir, backupPath, backupbase.DeprecatedBackupManifestName))
40044004
if err != nil {
40054005
t.Fatalf("%+v", err)
40064006
}
@@ -8456,7 +8456,6 @@ func TestIncorrectAccessOfFilesInBackupMetadata(t *testing.T) {
84568456

84578457
_, sqlDB, rawDir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication)
84588458
defer cleanupFn()
8459-
sqlDB.Exec(t, `SET CLUSTER SETTING backup.write_metadata_with_external_ssts.enabled=true`)
84608459
sqlDB.Exec(t, `CREATE DATABASE r1`)
84618460
sqlDB.Exec(t, `CREATE TABLE r1.foo ( id INT PRIMARY KEY)`)
84628461
sqlDB.Exec(t, `INSERT INTO r1.foo VALUES (1)`)
@@ -8502,7 +8501,6 @@ func TestRestoringAcrossVersions(t *testing.T) {
85028501
tc, sqlDB, rawDir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication)
85038502
defer cleanupFn()
85048503

8505-
sqlDB.Exec(t, `SET CLUSTER SETTING backup.write_metadata_with_external_ssts.enabled=true`)
85068504
sqlDB.Exec(t, `CREATE DATABASE r1`)
85078505

85088506
sqlDB.Exec(t, `BACKUP DATABASE r1 INTO 'nodelocal://1/cross_version'`)
@@ -8615,7 +8613,6 @@ func TestManifestBitFlip(t *testing.T) {
86158613
defer leaktest.AfterTest(t)()
86168614
defer log.Scope(t).Close(t)
86178615
_, sqlDB, rawDir, cleanupFn := backupRestoreTestSetup(t, singleNode, 1, InitManualReplication)
8618-
sqlDB.Exec(t, `SET CLUSTER SETTING backup.write_metadata_with_external_ssts.enabled=true`)
86198616
defer cleanupFn()
86208617
sqlDB.Exec(t, `CREATE DATABASE r1; CREATE DATABASE r2; CREATE DATABASE r3;`)
86218618
const checksumError = "checksum mismatch"
@@ -10422,7 +10419,7 @@ func TestBackupDoNotIncludeViewSpans(t *testing.T) {
1042210419
// Read the backup manifest.
1042310420
var backupManifest backuppb.BackupManifest
1042410421
{
10425-
backupManifestBytes, err := os.ReadFile(filepath.Join(dir, "foo", res[0][0], backupbase.BackupManifestName))
10422+
backupManifestBytes, err := os.ReadFile(filepath.Join(dir, "foo", res[0][0], backupbase.DeprecatedBackupManifestName))
1042610423
if err != nil {
1042710424
t.Fatalf("%+v", err)
1042810425
}

pkg/backup/backupbase/constants.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,12 @@ const (
3535
// Also exported for testing backup inspection tooling.
3636
DateBasedIntoFolderName = "/2006/01/02-150405.00"
3737

38-
// BackupOldManifestName is an old name for the serialized BackupManifest
39-
// proto. It is used by 20.1 nodes and earlier.
38+
// DeprecatedBackupManifestName is the file name used for serialized
39+
// BackupManifest protos.
4040
//
41-
// TODO(adityamaru): Remove this in 22.2 as part of disallowing backups
42-
// from >1 major version in the past.
43-
BackupOldManifestName = "BACKUP"
44-
45-
// BackupManifestName is the file name used for serialized BackupManifest
46-
// protos.
47-
//
48-
// TODO(adityamaru): Remove in 23.2 since at that point all nodes will be
49-
// writing a SlimBackupManifest instead.
50-
BackupManifestName = "BACKUP_MANIFEST"
41+
// TODO(msbutler): Remove 26.3 when we're guaranteed that no backup wrote
42+
// exclusively the backup_manifest, and not the slim manifest.
43+
DeprecatedBackupManifestName = "BACKUP_MANIFEST"
5144

5245
// BackupMetadataName is the file name used for serialized BackupManifest
5346
// protos written by 23.1 nodes and later. This manifest has the alloc heavy

pkg/backup/backupdest/backup_destination.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const (
5353
// On some cloud storage platforms (i.e. GS, S3), backups in a base bucket may
5454
// omit a leading slash. However, backups in a subdirectory of a base bucket
5555
// will contain one.
56-
var backupPathRE = regexp.MustCompile("^/?[^\\/]+/[^\\/]+/[^\\/]+/" + backupbase.BackupManifestName + "$")
56+
var backupPathRE = regexp.MustCompile("^/?[^\\/]+/[^\\/]+/[^\\/]+/" + backupbase.DeprecatedBackupManifestName + "$")
5757

5858
// featureFullBackupUserSubdir, when true, will create a full backup at a user
5959
// specified subdirectory if no backup already exists at that subdirectory. As
@@ -68,7 +68,7 @@ var featureFullBackupUserSubdir = settings.RegisterBoolSetting(
6868

6969
// TODO(adityamaru): Move this to the soon to be `backupinfo` package.
7070
func containsManifest(ctx context.Context, exportStore cloud.ExternalStorage) (bool, error) {
71-
r, _, err := exportStore.ReadFile(ctx, backupbase.BackupManifestName, cloud.ReadOptions{NoFileSize: true})
71+
r, _, err := exportStore.ReadFile(ctx, backupbase.DeprecatedBackupManifestName, cloud.ReadOptions{NoFileSize: true})
7272
if err != nil {
7373
if errors.Is(err, cloud.ErrFileDoesNotExist) {
7474
return false, nil
@@ -534,7 +534,7 @@ func ListFullBackupsInCollection(
534534
return nil, err
535535
}
536536
for i, backupPath := range backupPaths {
537-
backupPaths[i] = strings.TrimSuffix(backupPath, "/"+backupbase.BackupManifestName)
537+
backupPaths[i] = strings.TrimSuffix(backupPath, "/"+backupbase.DeprecatedBackupManifestName)
538538
}
539539
return backupPaths, nil
540540
}

pkg/backup/backupdest/backup_destination_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestBackupRestoreResolveDestination(t *testing.T) {
5656
require.NoError(t, err)
5757
reader := bytes.NewReader(manifestBytes)
5858
require.NoError(t, err)
59-
require.NoError(t, cloud.WriteFile(ctx, storage, backupbase.BackupManifestName, reader))
59+
require.NoError(t, cloud.WriteFile(ctx, storage, backupbase.DeprecatedBackupManifestName, reader))
6060
}
6161

6262
// writeLatest writes latestBackupSuffix to the LATEST file in the given

pkg/backup/backupdest/incrementals.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func FindAllIncrementalPaths(
120120

121121
if includeManifest {
122122
paths = util.Map(paths, func(p string) string {
123-
return path.Join(p, backupbase.BackupManifestName)
123+
return path.Join(p, backupbase.DeprecatedBackupManifestName)
124124
})
125125
}
126126
return paths, nil
@@ -141,32 +141,23 @@ func LegacyFindPriorBackups(
141141

142142
var prev []string
143143
if err := store.List(ctx, "", backupbase.ListingDelimDataSlash, func(p string) error {
144-
matchesGlob, err := path.Match(incBackupSubdirGlob+backupbase.BackupManifestName, p)
144+
matchesGlob, err := path.Match(incBackupSubdirGlob+backupbase.DeprecatedBackupManifestName, p)
145145
if err != nil {
146146
return err
147147
} else if !matchesGlob {
148-
matchesGlob, err = path.Match(incBackupSubdirGlobWithSuffix+backupbase.BackupManifestName, p)
148+
matchesGlob, err = path.Match(incBackupSubdirGlobWithSuffix+backupbase.DeprecatedBackupManifestName, p)
149149
if err != nil {
150150
return err
151151
}
152152
}
153153

154154
if matchesGlob {
155155
if !includeManifest {
156-
p = strings.TrimSuffix(p, "/"+backupbase.BackupManifestName)
156+
p = strings.TrimSuffix(p, "/"+backupbase.DeprecatedBackupManifestName)
157157
}
158158
prev = append(prev, p)
159159
return nil
160160
}
161-
162-
if ok, err := path.Match(incBackupSubdirGlob+backupbase.BackupOldManifestName, p); err != nil {
163-
return err
164-
} else if ok {
165-
if !includeManifest {
166-
p = strings.TrimSuffix(p, "/"+backupbase.BackupOldManifestName)
167-
}
168-
prev = append(prev, p)
169-
}
170161
return nil
171162
}); err != nil {
172163
return nil, errors.Wrap(err, "reading previous backup layers")

0 commit comments

Comments
 (0)