Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sstable: fix nil pointer with DisableValueBlocks set #4146

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
sstable: fix nil pointer with DisableValueBlocks set
Previously, the EstimatedSize method of the columnar sstable writer would panic
when the writer was configured with DisableValueBlocks enabled.
jbowens committed Nov 8, 2024
commit ee334ff86c896c4d4bef97e04e680d2304aaaf2b
12 changes: 7 additions & 5 deletions sstable/colblk_writer.go
Original file line number Diff line number Diff line change
@@ -203,11 +203,13 @@ func (w *RawColumnWriter) EstimatedSize() uint64 {
if w.rangeKeyBlock.KeyCount() > 0 {
sz += uint64(w.rangeKeyBlock.Size())
}
for _, blk := range w.valueBlock.blocks {
sz += uint64(blk.block.LengthWithTrailer())
}
if w.valueBlock.buf != nil {
sz += uint64(len(w.valueBlock.buf.b))
if w.valueBlock != nil {
for _, blk := range w.valueBlock.blocks {
sz += uint64(blk.block.LengthWithTrailer())
}
if w.valueBlock.buf != nil {
sz += uint64(len(w.valueBlock.buf.b))
}
}
// TODO(jackson): Include an estimate of the properties, filter and meta
// index blocks sizes.
70 changes: 37 additions & 33 deletions sstable/data_test.go
Original file line number Diff line number Diff line change
@@ -103,39 +103,8 @@ func runBuildMemObjCmd(
_ = w.Close()
}
}()
for _, data := range strings.Split(td.Input, "\n") {
err := func() (err error) {
defer func() {
if r := recover(); r != nil {
err = errors.Errorf("%v", r)
}
}()

switch {
case strings.HasPrefix(data, "EncodeSpan:"):
return w.EncodeSpan(keyspan.ParseSpan(strings.TrimPrefix(data, "EncodeSpan:")))
default:
forceObsolete := strings.HasPrefix(data, "force-obsolete:")
if forceObsolete {
data = strings.TrimSpace(strings.TrimPrefix(data, "force-obsolete:"))
}
j := strings.Index(data, ":")
key := base.ParseInternalKey(data[:j])
value := []byte(data[j+1:])
switch key.Kind() {
case InternalKeyKindRangeDelete:
if forceObsolete {
return errors.Errorf("force-obsolete is not allowed for RANGEDEL")
}
return w.AddWithForceObsolete(key, value, false /* forceObsolete */)
default:
return w.AddWithForceObsolete(key, value, forceObsolete)
}
}
}()
if err != nil {
return nil, nil, err
}
if err := writeKVs(w, td.Input); err != nil {
return nil, nil, err
}
if err := w.Close(); err != nil {
return nil, nil, err
@@ -148,6 +117,41 @@ func runBuildMemObjCmd(
return meta, obj, nil
}

func writeKVs(w RawWriter, input string) (err error) {
defer func() {
if r := recover(); r != nil {
err = errors.Errorf("%v", r)
}
}()
for _, data := range strings.Split(input, "\n") {
switch {
case strings.HasPrefix(data, "EncodeSpan:"):
err = w.EncodeSpan(keyspan.ParseSpan(strings.TrimPrefix(data, "EncodeSpan:")))
default:
forceObsolete := strings.HasPrefix(data, "force-obsolete:")
if forceObsolete {
data = strings.TrimSpace(strings.TrimPrefix(data, "force-obsolete:"))
}
j := strings.Index(data, ":")
key := base.ParseInternalKey(data[:j])
value := []byte(data[j+1:])
switch key.Kind() {
case InternalKeyKindRangeDelete:
if forceObsolete {
return errors.Errorf("force-obsolete is not allowed for RANGEDEL")
}
err = w.AddWithForceObsolete(key, value, false /* forceObsolete */)
default:
err = w.AddWithForceObsolete(key, value, forceObsolete)
}
}
if err != nil {
return err
}
}
return err
}

func runBuildCmd(
td *datadriven.TestData, writerOpts *WriterOptions, cacheSize int,
) (*WriterMetadata, *Reader, error) {
15 changes: 15 additions & 0 deletions sstable/testdata/writer_v5
Original file line number Diff line number Diff line change
@@ -327,3 +327,18 @@ rocksdb.property.collectors: [obsolete-key]
pebble.raw.range-key.key.size: 6
pebble.raw.range-key.value.size: 9
obsolete-key: hex:0074

open-writer disable-value-blocks
----

write-kvs
a@2.SET.1:a2
a@1.SET.1:a1
b@2.SET.1:b2
----
EstimatedSize()=53

close
----
point: [a@2#1,SET-b@2#1,SET]
seqnums: [1-1]
52 changes: 46 additions & 6 deletions sstable/writer_test.go
Original file line number Diff line number Diff line change
@@ -120,20 +120,24 @@ func formatWriterMetadata(td *datadriven.TestData, m *WriterMetadata) string {

func runDataDriven(t *testing.T, file string, tableFormat TableFormat, parallelism bool) {
var r *Reader
var w RawWriter
var obj *objstorage.MemObj
defer func() {
closeReaderAndWriter := func() {
if r != nil {
require.NoError(t, r.Close())
}
}()
if w != nil {
require.NoError(t, w.Close())
}
r = nil
w = nil
}
defer closeReaderAndWriter()

datadriven.RunTest(t, file, func(t *testing.T, td *datadriven.TestData) string {
switch td.Cmd {
case "build":
if r != nil {
_ = r.Close()
r = nil
}
closeReaderAndWriter()
var meta *WriterMetadata
var err error
wopts := &WriterOptions{
@@ -152,6 +156,7 @@ func runDataDriven(t *testing.T, file string, tableFormat TableFormat, paralleli
return formatWriterMetadata(td, meta)

case "build-raw":
closeReaderAndWriter()
if r != nil {
_ = r.Close()
r = nil
@@ -167,6 +172,41 @@ func runDataDriven(t *testing.T, file string, tableFormat TableFormat, paralleli
}
return formatWriterMetadata(td, meta)

case "open-writer":
if r != nil {
_ = r.Close()
r = nil
}
wopts := &WriterOptions{
Comparer: testkeys.Comparer,
DisableValueBlocks: td.HasArg("disable-value-blocks"),
TableFormat: tableFormat,
Parallelism: parallelism,
}
obj := &objstorage.MemObj{}
if err := optsFromArgs(td, wopts); err != nil {
return err.Error()
}
w = NewRawWriter(obj, *wopts)
return ""

case "write-kvs":
if err := writeKVs(w, td.Input); err != nil {
return err.Error()
}
return fmt.Sprintf("EstimatedSize()=%d", w.EstimatedSize())

case "close":
if err := w.Close(); err != nil {
return err.Error()
}
meta, err := w.Metadata()
if err != nil {
return err.Error()
}
w = nil
return formatWriterMetadata(td, meta)

case "scan":
iter, err := r.NewIter(NoTransforms, nil /* lower */, nil /* upper */)
if err != nil {