Skip to content

Commit

Permalink
manifest: Added Virtual SSTable Metadata Bounds Checking
Browse files Browse the repository at this point in the history
Added a check inside of `FileMetadata.ValidateVirtual()` to ensure that
the keys returned from a virtual sstable are within the bounds of the
Smallest and Largest metadata keys.

Informs: #2593
  • Loading branch information
raggar committed Jul 19, 2023
1 parent 628c410 commit 4959a91
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 0 deletions.
36 changes: 36 additions & 0 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -2503,3 +2503,39 @@ func (d *DB) SetCreatorID(creatorID uint64) error {
func (d *DB) ObjProvider() objstorage.Provider {
return d.objProvider
}

func (d *DB) checkVirtualBounds(m *fileMetadata) {
if !invariants.Enabled {
return
}

it, rangeIter, err := d.newIters(context.TODO(), m, nil, internalIterOpts{})
if err != nil {
panic(fmt.Sprintf("pebble: error creating iterator :%s", err.Error()))
}

// Check that virtual sstable bounds are tight from both ends.
first, _ := it.First()
if d.cmp(first.UserKey, m.Smallest.UserKey) != 0 {
panic(fmt.Sprintf("pebble: virtual sstable %s lower bound is not tight: %s != %s",
m.FileNum, d.opts.Comparer.FormatKey(m.Smallest.UserKey), d.opts.Comparer.FormatKey(first.UserKey)))
}
last, _ := it.Last()
if d.cmp(last.UserKey, m.Largest.UserKey) != 0 {
panic(fmt.Sprintf("pebble: virtual sstable %s upper bound is not tight: %s != %s",
m.FileNum, d.opts.Comparer.FormatKey(m.Largest.UserKey), d.opts.Comparer.FormatKey(last.UserKey)))
}

// Check that iterator keys are within bounds.
for key, _ := it.First(); key != nil; key, _ = it.Next() {
if d.cmp(key.UserKey, m.Smallest.UserKey) < 0 || d.cmp(key.UserKey, m.Largest.UserKey) > 0 {
panic("pebble: virtual sstable key is not within bounds")
}
}
if err := it.Close(); err != nil {
panic(fmt.Sprintf("pebble: error closing virtual sstable iterator: %s", err.Error()))
}
if rangeIter != nil {
rangeIter.Close()
}
}
7 changes: 7 additions & 0 deletions ingest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,9 @@ func (d *DB) ingest(
if err := ingestLink(jobID, d.opts, d.objProvider, loadResult, shared); err != nil {
return IngestOperationStats{}, err
}
for _, sharedMeta := range loadResult.sharedMeta {
d.checkVirtualBounds(sharedMeta)
}
// Make the new tables durable. We need to do this at some point before we
// update the MANIFEST (via logAndApply), otherwise a crash can have the
// tables referenced in the MANIFEST, but not present in the provider.
Expand Down Expand Up @@ -1490,6 +1493,8 @@ func (d *DB) excise(
if err := leftFile.Validate(d.cmp, d.opts.Comparer.FormatKey); err != nil {
return nil, err
}
leftFile.ValidateVirtual(m)
d.checkVirtualBounds(leftFile)
ve.NewFiles = append(ve.NewFiles, newFileEntry{Level: level, Meta: leftFile})
ve.CreatedBackingTables = append(ve.CreatedBackingTables, leftFile.FileBacking)
backingTableCreated = true
Expand Down Expand Up @@ -1597,6 +1602,8 @@ func (d *DB) excise(
// for it here.
rightFile.Size = 1
}
rightFile.ValidateVirtual(m)
d.checkVirtualBounds(rightFile)
ve.NewFiles = append(ve.NewFiles, newFileEntry{Level: level, Meta: rightFile})
if !backingTableCreated {
ve.CreatedBackingTables = append(ve.CreatedBackingTables, rightFile.FileBacking)
Expand Down
2 changes: 2 additions & 0 deletions table_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,9 @@ func TestVirtualReadsWiring(t *testing.T) {
v2.SmallestPointKey = v2.Smallest

v1.ValidateVirtual(parentFile)
d.checkVirtualBounds(v1)
v2.ValidateVirtual(parentFile)
d.checkVirtualBounds(v2)

// Write the version edit.
fileMetrics := func(ve *versionEdit) map[int]*LevelMetrics {
Expand Down
4 changes: 4 additions & 0 deletions version_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ func TestLatestRefCounting(t *testing.T) {
m2.SmallestPointKey = m2.Smallest

m1.ValidateVirtual(f)
d.checkVirtualBounds(m1)
m2.ValidateVirtual(f)
d.checkVirtualBounds(m2)

fileMetrics := func(ve *versionEdit) map[int]*LevelMetrics {
metrics := newFileMetrics(ve.NewFiles)
Expand Down Expand Up @@ -282,7 +284,9 @@ func TestVirtualSSTableManifestReplay(t *testing.T) {
m2.Stats.NumEntries = 1

m1.ValidateVirtual(f)
d.checkVirtualBounds(m1)
m2.ValidateVirtual(f)
d.checkVirtualBounds(m2)

fileMetrics := func(ve *versionEdit) map[int]*LevelMetrics {
metrics := newFileMetrics(ve.NewFiles)
Expand Down

0 comments on commit 4959a91

Please sign in to comment.