From 7ea3af4eef9b2a2daa5a0da01ee0f2700b6f8120 Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Wed, 24 Aug 2022 15:11:15 -0400 Subject: [PATCH] backupccl: set kv.bulkio.write_metadata_sst.enabled to default false This patch sets write_metadata_sst cluster setting to false in prep for the 22.2 branch cut, as there's additional worked required before this feature gets used in production. Unit tests may continue to write the MetadataSST because of a new MetamorphicTestBool. Setting this to false will also stop the roachtest in #86289 from consistently failing due to #86806. Fixes #86289 Release note: none Release justification: prevents using an experimental feature by default --- pkg/ccl/backupccl/backup_job.go | 2 +- pkg/ccl/backupccl/backupinfo/BUILD.bazel | 1 + pkg/ccl/backupccl/backupinfo/manifest_handling.go | 10 +++++++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/ccl/backupccl/backup_job.go b/pkg/ccl/backupccl/backup_job.go index 62cc97912d19..8c1e9f22fbb6 100644 --- a/pkg/ccl/backupccl/backup_job.go +++ b/pkg/ccl/backupccl/backup_job.go @@ -353,7 +353,7 @@ func backup( return roachpb.RowCount{}, err } - if backupinfo.WriteMetadataSST.Get(&settings.SV) { + if backupinfo.WriteMetadataSST.Get(&settings.SV) || backupinfo.TestWriteMetadataSST { if err := backupinfo.WriteBackupMetadataSST(ctx, defaultStore, encryption, &kmsEnv, backupManifest, tableStatistics); err != nil { err = errors.Wrap(err, "writing forward-compat metadata sst") diff --git a/pkg/ccl/backupccl/backupinfo/BUILD.bazel b/pkg/ccl/backupccl/backupinfo/BUILD.bazel index a7ca79d41cfc..5d359950deec 100644 --- a/pkg/ccl/backupccl/backupinfo/BUILD.bazel +++ b/pkg/ccl/backupccl/backupinfo/BUILD.bazel @@ -33,6 +33,7 @@ go_library( "//pkg/sql/sem/tree", "//pkg/sql/stats", "//pkg/storage", + "//pkg/util", "//pkg/util/ctxgroup", "//pkg/util/encoding", "//pkg/util/hlc", diff --git a/pkg/ccl/backupccl/backupinfo/manifest_handling.go b/pkg/ccl/backupccl/backupinfo/manifest_handling.go index e45022ed3c60..e97ed5736d3d 100644 --- a/pkg/ccl/backupccl/backupinfo/manifest_handling.go +++ b/pkg/ccl/backupccl/backupinfo/manifest_handling.go @@ -39,6 +39,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/ctxgroup" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -82,15 +83,18 @@ const ( // WriteMetadataSST controls if we write the experimental new format BACKUP // metadata file. -// kv.bulkio.rite_metadata_sst.enabled set to false by default due to -// https://github.com/cockroachdb/cockroach/issues/85564. var WriteMetadataSST = settings.RegisterBoolSetting( settings.TenantWritable, "kv.bulkio.write_metadata_sst.enabled", "write experimental new format BACKUP metadata file", - true, + false, ) +// TestWriteMetadataSST causes unit tests that run a backup to write the metadata sst with some +// probability. +var TestWriteMetadataSST = util.ConstantWithMetamorphicTestBool("write-metadata-sst", + WriteMetadataSST.Default()) + // IsGZipped detects whether the given bytes represent GZipped data. This check // is used rather than a standard implementation such as http.DetectContentType // since some zipped data may be mis-identified by that method. We've seen