Skip to content

Commit

Permalink
manifest: add methods for extending table bounds
Browse files Browse the repository at this point in the history 
Currently, `FileMetadata` exposes three pairs of keys representing
bounds for a table - one each for point keys, range keys and the overall
bounds for a table. The pair for the overall bounds for the table are a
function of the point and range pairs and could technically be derived,
but instead these overall bounds are materialized on the `FileMetadata`
struct directly for performance reasons (it is faster to dereference the
struct fields than making a function call to perform one or more
comparisons).

With the addition of two pairs of bounds in cockroachdb#1521 for the point and
range bounds, there is a risk that a caller forgets to set a pair of
bounds, violating the invariant that the overall point and range bounds
are internally consistent with the point and range key bounds.

To mitigate this risk, introduce methods on `manifest.FileMetadata` that
can be used to "extend" the point or range key bounds for a table. These
methods will maintain the invariant that the overall bounds for the
table are computed directly from the point and range bounds.

The methods, `MaybeExtend{Point,Range}KeyBounds`, are intended for
"optimistic" use - a table's point, range and / or overall bounds may
not necessarily be extended by a call to the respective methods if the
existing bounds are smaller or larger than the provided bounds. This
confines the comparisons to these method calls, as opposed to having the
caller perform the checks to determine when bounds should be updated. As
these fields are set once and read multiple times, this is considered a
reasonable compromise.

Update all existing call sites that directly set the existing
`FileMetadata` smallest / largest bound fields to instead use the new
methods for extending bounds. The only remaining call sites that set
fields directly are limited to the `manifest` package and one call site
in `ingest.go` that needs to mutate sequence numbers. These calls sites
that store values in the fields directly are explicitly commented to
indicate why the fields are being set directly.

One alternative considered was to unexport the smallest / largest fields
and gate access behind methods. However, as mentioned above, for
performance reasons it beneficial to keep these fields exported.
  • Loading branch information
@nicktrav
nicktrav committed Feb 24, 2022
1 parent 894b57a commit b1b77bc
Show file tree
Hide file tree
Showing 22 changed files with 1,153 additions and 803 deletions.
15 changes: 9 additions & 6 deletions compaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2233,12 +2233,15 @@ func (d *DB) runCompaction(
)
}

meta.SmallestPointKey = writerMeta.SmallestPointKey(d.cmp)
meta.LargestPointKey = writerMeta.LargestPointKey(d.cmp)
meta.SmallestRangeKey = writerMeta.SmallestRangeKey
meta.LargestRangeKey = writerMeta.LargestRangeKey
meta.Smallest = writerMeta.Smallest(d.cmp)
meta.Largest = writerMeta.Largest(d.cmp)
if writerMeta.HasPointKeys {
meta.MaybeExtendPointKeyBounds(d.cmp, writerMeta.SmallestPoint, writerMeta.LargestPoint)
}
if writerMeta.HasRangeDelKeys {
meta.MaybeExtendPointKeyBounds(d.cmp, writerMeta.SmallestRangeDel, writerMeta.LargestRangeDel)
}
if writerMeta.HasRangeKeys {
meta.MaybeExtendRangeKeyBounds(d.cmp, writerMeta.SmallestRangeKey, writerMeta.LargestRangeKey)
}

// Verify that the sstable bounds fall within the compaction input
// bounds. This is a sanity check that we don't have a logic error
Expand Down
82 changes: 51 additions & 31 deletions compaction_picker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,11 @@ func loadVersion(d *datadriven.TestData) (*version, *Options, [numLevels]int64,
key = base.MakeInternalKey([]byte(fmt.Sprintf("%04d", i)), i, InternalKeyKindSet)
}
m := &fileMetadata{
Smallest: key,
Largest: key,
SmallestSeqNum: key.SeqNum(),
LargestSeqNum: key.SeqNum(),
Size: 1,
}
m.MaybeExtendPointKeyBounds(opts.Comparer.Compare, key, key)
if size >= 100 {
// If the requested size of the level is very large only add a single
// file in order to avoid massive blow-up in the number of files in
Expand Down Expand Up @@ -366,8 +365,11 @@ func TestCompactionPickerIntraL0(t *testing.T) {
if len(parts) != 2 {
t.Fatalf("malformed table spec: %s", s)
}
m.Smallest = base.ParseInternalKey(strings.TrimSpace(parts[0]))
m.Largest = base.ParseInternalKey(strings.TrimSpace(parts[1]))
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(parts[0])),
base.ParseInternalKey(strings.TrimSpace(parts[1])),
)
m.SmallestSeqNum = m.Smallest.SeqNum()
m.LargestSeqNum = m.Largest.SeqNum()
if m.SmallestSeqNum > m.LargestSeqNum {
Expand Down Expand Up @@ -437,6 +439,9 @@ func TestCompactionPickerIntraL0(t *testing.T) {
}

func TestCompactionPickerL0(t *testing.T) {
opts := (*Options)(nil).EnsureDefaults()
opts.Experimental.L0CompactionConcurrency = 1

parseMeta := func(s string) (*fileMetadata, error) {
parts := strings.Split(s, ":")
fileNum, err := strconv.Atoi(parts[0])
Expand All @@ -448,18 +453,17 @@ func TestCompactionPickerL0(t *testing.T) {
if len(parts) != 2 {
return nil, errors.Errorf("malformed table spec: %s", s)
}
m := &fileMetadata{
FileNum: base.FileNum(fileNum),
Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])),
Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])),
}
m := &fileMetadata{FileNum: base.FileNum(fileNum)}
m.MaybeExtendRangeKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(parts[0])),
base.ParseInternalKey(strings.TrimSpace(parts[1])),
)
m.SmallestSeqNum = m.Smallest.SeqNum()
m.LargestSeqNum = m.Largest.SeqNum()
return m, nil
}

opts := (*Options)(nil).EnsureDefaults()
opts.Experimental.L0CompactionConcurrency = 1
var picker *compactionPickerByScore
var inProgressCompactions []compactionInfo
var pc *pickedCompaction
Expand Down Expand Up @@ -645,6 +649,9 @@ func TestCompactionPickerL0(t *testing.T) {
}

func TestCompactionPickerConcurrency(t *testing.T) {
opts := (*Options)(nil).EnsureDefaults()
opts.Experimental.L0CompactionConcurrency = 1

parseMeta := func(s string) (*fileMetadata, error) {
parts := strings.Split(s, ":")
fileNum, err := strconv.Atoi(parts[0])
Expand All @@ -657,11 +664,14 @@ func TestCompactionPickerConcurrency(t *testing.T) {
return nil, errors.Errorf("malformed table spec: %s", s)
}
m := &fileMetadata{
FileNum: base.FileNum(fileNum),
Size: 1028,
Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])),
Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])),
FileNum: base.FileNum(fileNum),
Size: 1028,
}
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(parts[0])),
base.ParseInternalKey(strings.TrimSpace(parts[1])),
)
for _, p := range fields[1:] {
if strings.HasPrefix(p, "size=") {
v, err := strconv.Atoi(strings.TrimPrefix(p, "size="))
Expand All @@ -676,8 +686,6 @@ func TestCompactionPickerConcurrency(t *testing.T) {
return m, nil
}

opts := (*Options)(nil).EnsureDefaults()
opts.Experimental.L0CompactionConcurrency = 1
var picker *compactionPickerByScore
var inProgressCompactions []compactionInfo

Expand Down Expand Up @@ -875,11 +883,14 @@ func TestCompactionPickerPickReadTriggered(t *testing.T) {
return nil, errors.Errorf("malformed table spec: %s. usage: <file-num>:start.SET.1-end.SET.2", s)
}
m := &fileMetadata{
FileNum: base.FileNum(fileNum),
Size: 1028,
Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])),
Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])),
FileNum: base.FileNum(fileNum),
Size: 1028,
}
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(parts[0])),
base.ParseInternalKey(strings.TrimSpace(parts[1])),
)
for _, p := range fields[1:] {
if strings.HasPrefix(p, "size=") {
v, err := strconv.Atoi(strings.TrimPrefix(p, "size="))
Expand Down Expand Up @@ -1027,11 +1038,14 @@ func TestPickedCompactionSetupInputs(t *testing.T) {
t.Fatalf("malformed table spec: %s", s)
}
m := &fileMetadata{
Smallest: base.ParseInternalKey(strings.TrimSpace(tableParts[0])),
Largest: base.ParseInternalKey(strings.TrimSpace(tableParts[1])),
Compacting: compacting,
Size: fileSize,
}
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(tableParts[0])),
base.ParseInternalKey(strings.TrimSpace(tableParts[1])),
)
m.SmallestSeqNum = m.Smallest.SeqNum()
m.LargestSeqNum = m.Largest.SeqNum()
return m
Expand Down Expand Up @@ -1138,10 +1152,13 @@ func TestPickedCompactionExpandInputs(t *testing.T) {
if len(parts) != 2 {
t.Fatalf("malformed table spec: %s", s)
}
return &fileMetadata{
Smallest: base.ParseInternalKey(parts[0]),
Largest: base.ParseInternalKey(parts[1]),
}
m := &fileMetadata{}
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(parts[0]),
base.ParseInternalKey(parts[1]),
)
return m
}

datadriven.RunTest(t, "testdata/compaction_expand_inputs",
Expand Down Expand Up @@ -1217,11 +1234,14 @@ func TestCompactionOutputFileSize(t *testing.T) {
return nil, errors.Errorf("malformed table spec: %s. usage: <file-num>:start.SET.1-end.SET.2", s)
}
m := &fileMetadata{
FileNum: base.FileNum(fileNum),
Size: 1028,
Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])),
Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])),
FileNum: base.FileNum(fileNum),
Size: 1028,
}
m.MaybeExtendPointKeyBounds(
opts.Comparer.Compare,
base.ParseInternalKey(strings.TrimSpace(parts[0])),
base.ParseInternalKey(strings.TrimSpace(parts[1])),
)
for _, p := range fields[1:] {
if strings.HasPrefix(p, "size=") {
v, err := strconv.Atoi(strings.TrimPrefix(p, "size="))
Expand Down
Loading

0 comments on commit b1b77bc

Please sign in to comment.