From 1a8fb576f5d982509cd5dbaa68c9fabb27b49e58 Mon Sep 17 00:00:00 2001 From: sumeerbhola Date: Mon, 13 Dec 2021 16:59:06 -0500 Subject: [PATCH] storage: disable BlockPropertyCollectors for SSTWriter There is a todo to enable it for MakeIngestionSSTWriter, which will require plumbing either the cluster version or a reference to the Engine to the function, so one can determine that it is safe to do so. Without this change, tests that mix Pebble old and new versions fail because old versions cannot parse sstable index entries containing block properties. Fixes #73485,#73708 Release note: None --- pkg/storage/sst_writer.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/storage/sst_writer.go b/pkg/storage/sst_writer.go index 841791b6b80c..8080d06f12da 100644 --- a/pkg/storage/sst_writer.go +++ b/pkg/storage/sst_writer.go @@ -56,6 +56,8 @@ func (noopSyncCloser) Close() error { // are typically only ever iterated in their entirety. func MakeBackupSSTWriter(f io.Writer) SSTWriter { opts := DefaultPebbleOptions().MakeWriterOptions(0) + // Don't need BlockPropertyCollectors for backups. + opts.BlockPropertyCollectors = nil opts.TableFormat = sstable.TableFormatRocksDBv2 // Disable bloom filters since we only ever iterate backups. @@ -75,6 +77,10 @@ func MakeBackupSSTWriter(f io.Writer) SSTWriter { // format set to RocksDBv2. func MakeIngestionSSTWriter(f writeCloseSyncer) SSTWriter { opts := DefaultPebbleOptions().MakeWriterOptions(0) + // TODO(sumeer): we should use BlockPropertyCollectors here if the cluster + // version permits (which is also reflected in the store's roachpb.Version + // and pebble.FormatMajorVersion). + opts.BlockPropertyCollectors = nil opts.TableFormat = sstable.TableFormatRocksDBv2 opts.MergerName = "nullptr" sst := sstable.NewWriter(f, opts)