Skip to content

Commit

Permalink
storage: disable BlockPropertyCollectors for SSTWriter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sumeerbhola committed Dec 13, 2021
1 parent d411bd0 commit 1a8fb57
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/storage/sst_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down

0 comments on commit 1a8fb57

Please sign in to comment.