Skip to content

Commit

Permalink
scan_internal: don't visit empty external files
Browse files Browse the repository at this point in the history
When truncating external files to our scan bounds, we may find that
the file has Start == End. Other validation in ingest asserts these
are invalid bounds.
  • Loading branch information
stevendanna committed Mar 8, 2024
1 parent 1f7e8ee commit ca67095
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 31 deletions.
17 changes: 0 additions & 17 deletions ingest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1179,23 +1179,6 @@ type ExternalFile struct {
Level uint8
}

func (e *ExternalFile) cloneFromFileMeta(f *fileMetadata) {
*e = ExternalFile{
Bounds: KeyRange{
Start: append([]byte(nil), f.Smallest.UserKey...),
End: append([]byte(nil), f.Largest.UserKey...),
},
HasPointKey: f.HasPointKeys,
HasRangeKey: f.HasRangeKeys,
Size: f.Size,
}
e.SyntheticSuffix = append([]byte(nil), f.SyntheticSuffix...)
if pr := f.PrefixReplacement; pr != nil {
e.ContentPrefix = append([]byte(nil), pr.ContentPrefix...)
e.SyntheticPrefix = append([]byte(nil), pr.SyntheticPrefix...)
}
}

// IngestWithStats does the same as Ingest, and additionally returns
// IngestOperationStats.
func (d *DB) IngestWithStats(paths []string) (IngestOperationStats, error) {
Expand Down
52 changes: 38 additions & 14 deletions scan_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package pebble
import (
"context"
"fmt"
"slices"

"github.com/cockroachdb/errors"
"github.com/cockroachdb/pebble/internal/base"
Expand Down Expand Up @@ -460,27 +461,43 @@ func (d *DB) truncateExternalFile(
level int,
file *fileMetadata,
objMeta objstorage.ObjectMetadata,
) *ExternalFile {
) (*ExternalFile, error) {
cmp := d.cmp
sst := &ExternalFile{}
sst.cloneFromFileMeta(file)
sst.Level = uint8(level)
sst.ObjName = objMeta.Remote.CustomObjectName
sst.Locator = objMeta.Remote.Locator
needsLowerTruncate := cmp(lower, file.Smallest.UserKey) > 0
needsUpperTruncate := cmp(upper, file.Largest.UserKey) < 0 || (cmp(upper, file.Largest.UserKey) == 0 && !file.Largest.IsExclusiveSentinel())
sst := &ExternalFile{
Level: uint8(level),
ObjName: objMeta.Remote.CustomObjectName,
Locator: objMeta.Remote.Locator,
Bounds: KeyRange{},
HasPointKey: file.HasPointKeys,
HasRangeKey: file.HasRangeKeys,
Size: file.Size,
SyntheticSuffix: slices.Clone(file.SyntheticSuffix),
}
if pr := file.PrefixReplacement; pr != nil {
sst.ContentPrefix = slices.Clone(pr.ContentPrefix)
sst.SyntheticPrefix = slices.Clone(pr.SyntheticPrefix)
}

needsLowerTruncate := cmp(lower, file.Smallest.UserKey) > 0
if needsLowerTruncate {
sst.Bounds.Start = sst.Bounds.Start[:0]
sst.Bounds.Start = append(sst.Bounds.Start, lower...)
sst.Bounds.Start = slices.Clone(lower)
} else {
sst.Bounds.Start = slices.Clone(file.Smallest.UserKey)
}

cmpUpper := cmp(upper, file.Largest.UserKey)
needsUpperTruncate := cmpUpper < 0 || (cmpUpper == 0 && !file.Largest.IsExclusiveSentinel())
if needsUpperTruncate {
sst.Bounds.End = sst.Bounds.End[:0]
sst.Bounds.End = append(sst.Bounds.End, upper...)
sst.Bounds.End = slices.Clone(upper)
} else {
sst.Bounds.End = slices.Clone(file.Largest.UserKey)
}

return sst
if cmp(sst.Bounds.Start, sst.Bounds.End) >= 0 {
return nil, errors.AssertionFailedf("pebble: invalid external file bounds after truncation [%q, %q)", sst.Bounds.Start, sst.Bounds.End)
}

return sst, nil
}

// truncateSharedFile truncates a shared file's [Smallest, Largest] fields to
Expand Down Expand Up @@ -693,6 +710,10 @@ func scanInternalImpl(
for level := firstLevelWithRemote; level < numLevels; level++ {
files := current.Levels[level].Iter()
for f := files.SeekGE(cmp, lower); f != nil && cmp(f.Smallest.UserKey, upper) < 0; f = files.Next() {
if cmp(lower, f.Largest.UserKey) == 0 && f.Largest.IsExclusiveSentinel() {
continue
}

var objMeta objstorage.ObjectMetadata
var err error
objMeta, err = provider.Lookup(fileTypeTable, f.FileBacking.DiskFileNum)
Expand Down Expand Up @@ -737,7 +758,10 @@ func scanInternalImpl(
return err
}
} else if objMeta.IsExternal() {
sst := iter.db.truncateExternalFile(ctx, lower, upper, level, f, objMeta)
sst, err := iter.db.truncateExternalFile(ctx, lower, upper, level, f, objMeta)
if err != nil {
return err
}
if err := opts.visitExternalFile(sst); err != nil {
return err
}
Expand Down
16 changes: 16 additions & 0 deletions testdata/ingest_external
Original file line number Diff line number Diff line change
Expand Up @@ -695,3 +695,19 @@ a: (d2-v1, .)
b: (d1-v1, .)
d: (d1-v1, .)
e: (d2-v1, .)

reset
----

build-remote trunctest
set a foo
set b bar
----

ingest-external
trunctest bounds=(a,c)
----

replicate 1 2 c z
----
replicated 0 external SSTs

0 comments on commit ca67095

Please sign in to comment.