From b1b77bc4b742a3975866bfd80b351bee1b38d652 Mon Sep 17 00:00:00 2001 From: Nick Travers Date: Tue, 22 Feb 2022 19:07:00 -0800 Subject: [PATCH] manifest: add methods for extending table bounds 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 #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. --- compaction.go | 15 +- compaction_picker_test.go | 82 +- compaction_test.go | 825 +++++++++--------- data_test.go | 11 +- db.go | 4 +- flush_external.go | 22 +- get_iter_test.go | 9 +- ingest.go | 66 +- ingest_test.go | 68 +- internal/manifest/btree_test.go | 7 +- internal/manifest/l0_sublevels_test.go | 17 +- internal/manifest/level_metadata_test.go | 11 +- .../manifest/testdata/file_metadata_bounds | 85 ++ internal/manifest/version.go | 155 +++- internal/manifest/version_edit.go | 16 +- internal/manifest/version_edit_test.go | 78 +- internal/manifest/version_test.go | 236 +++-- level_checker_test.go | 8 +- level_iter_test.go | 32 +- merging_iter_test.go | 15 +- sstable/suffix_rewriter.go | 8 +- sstable/writer.go | 186 ++-- 22 files changed, 1153 insertions(+), 803 deletions(-) create mode 100644 internal/manifest/testdata/file_metadata_bounds diff --git a/compaction.go b/compaction.go index 44dac8f2db..227690af9b 100644 --- a/compaction.go +++ b/compaction.go @@ -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 diff --git a/compaction_picker_test.go b/compaction_picker_test.go index 5500ab29dd..2b1fd4d13c 100644 --- a/compaction_picker_test.go +++ b/compaction_picker_test.go @@ -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 @@ -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 { @@ -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]) @@ -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 @@ -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]) @@ -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=")) @@ -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 @@ -875,11 +883,14 @@ func TestCompactionPickerPickReadTriggered(t *testing.T) { return nil, errors.Errorf("malformed table spec: %s. usage: :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=")) @@ -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 @@ -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", @@ -1217,11 +1234,14 @@ func TestCompactionOutputFileSize(t *testing.T) { return nil, errors.Errorf("malformed table spec: %s. usage: :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=")) diff --git a/compaction_test.go b/compaction_test.go index 15c69c1623..0e2bf57132 100644 --- a/compaction_test.go +++ b/compaction_test.go @@ -118,6 +118,15 @@ func TestPickCompaction(t *testing.T) { } opts := (*Options)(nil).EnsureDefaults() + newFileMeta := func(fileNum FileNum, size uint64, smallest, largest base.InternalKey) *fileMetadata { + m := &fileMetadata{ + FileNum: fileNum, + Size: size, + } + m.MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest, largest) + return m + } + testCases := []struct { desc string version *version @@ -128,12 +137,12 @@ func TestPickCompaction(t *testing.T) { desc: "no compaction", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("j.SET.102"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("j.SET.102"), + ), }, }), want: "", @@ -143,12 +152,12 @@ func TestPickCompaction(t *testing.T) { desc: "1 L0 file", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("j.SET.102"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("j.SET.102"), + ), }, }), picker: compactionPickerForTesting{ @@ -163,18 +172,18 @@ func TestPickCompaction(t *testing.T) { desc: "2 L0 files (0 overlaps)", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("j.SET.102"), - }, - { - FileNum: 110, - Size: 1, - Smallest: base.ParseInternalKey("k.SET.111"), - Largest: base.ParseInternalKey("l.SET.112"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("j.SET.102"), + ), + newFileMeta( + 110, + 1, + base.ParseInternalKey("k.SET.111"), + base.ParseInternalKey("l.SET.112"), + ), }, }), picker: compactionPickerForTesting{ @@ -189,18 +198,18 @@ func TestPickCompaction(t *testing.T) { desc: "2 L0 files, with ikey overlap", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("p.SET.102"), - }, - { - FileNum: 110, - Size: 1, - Smallest: base.ParseInternalKey("j.SET.111"), - Largest: base.ParseInternalKey("q.SET.112"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("p.SET.102"), + ), + newFileMeta( + 110, + 1, + base.ParseInternalKey("j.SET.111"), + base.ParseInternalKey("q.SET.112"), + ), }, }), picker: compactionPickerForTesting{ @@ -215,18 +224,18 @@ func TestPickCompaction(t *testing.T) { desc: "2 L0 files, with ukey overlap", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("i.SET.102"), - }, - { - FileNum: 110, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.111"), - Largest: base.ParseInternalKey("i.SET.112"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("i.SET.102"), + ), + newFileMeta( + 110, + 1, + base.ParseInternalKey("i.SET.111"), + base.ParseInternalKey("i.SET.112"), + ), }, }), picker: compactionPickerForTesting{ @@ -241,26 +250,26 @@ func TestPickCompaction(t *testing.T) { desc: "1 L0 file, 2 L1 files (0 overlaps)", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("i.SET.102"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("i.SET.102"), + ), }, 1: { - { - FileNum: 200, - Size: 1, - Smallest: base.ParseInternalKey("a.SET.201"), - Largest: base.ParseInternalKey("b.SET.202"), - }, - { - FileNum: 210, - Size: 1, - Smallest: base.ParseInternalKey("y.SET.211"), - Largest: base.ParseInternalKey("z.SET.212"), - }, + newFileMeta( + 200, + 1, + base.ParseInternalKey("a.SET.201"), + base.ParseInternalKey("b.SET.202"), + ), + newFileMeta( + 210, + 1, + base.ParseInternalKey("y.SET.211"), + base.ParseInternalKey("z.SET.212"), + ), }, }), picker: compactionPickerForTesting{ @@ -275,52 +284,52 @@ func TestPickCompaction(t *testing.T) { desc: "1 L0 file, 2 L1 files (1 overlap), 4 L2 files (3 overlaps)", version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - FileNum: 100, - Size: 1, - Smallest: base.ParseInternalKey("i.SET.101"), - Largest: base.ParseInternalKey("t.SET.102"), - }, + newFileMeta( + 100, + 1, + base.ParseInternalKey("i.SET.101"), + base.ParseInternalKey("t.SET.102"), + ), }, 1: { - { - FileNum: 200, - Size: 1, - Smallest: base.ParseInternalKey("a.SET.201"), - Largest: base.ParseInternalKey("e.SET.202"), - }, - { - FileNum: 210, - Size: 1, - Smallest: base.ParseInternalKey("f.SET.211"), - Largest: base.ParseInternalKey("j.SET.212"), - }, + newFileMeta( + 200, + 1, + base.ParseInternalKey("a.SET.201"), + base.ParseInternalKey("e.SET.202"), + ), + newFileMeta( + 210, + 1, + base.ParseInternalKey("f.SET.211"), + base.ParseInternalKey("j.SET.212"), + ), }, 2: { - { - FileNum: 300, - Size: 1, - Smallest: base.ParseInternalKey("a.SET.301"), - Largest: base.ParseInternalKey("b.SET.302"), - }, - { - FileNum: 310, - Size: 1, - Smallest: base.ParseInternalKey("c.SET.311"), - Largest: base.ParseInternalKey("g.SET.312"), - }, - { - FileNum: 320, - Size: 1, - Smallest: base.ParseInternalKey("h.SET.321"), - Largest: base.ParseInternalKey("m.SET.322"), - }, - { - FileNum: 330, - Size: 1, - Smallest: base.ParseInternalKey("n.SET.331"), - Largest: base.ParseInternalKey("z.SET.332"), - }, + newFileMeta( + 300, + 1, + base.ParseInternalKey("a.SET.301"), + base.ParseInternalKey("b.SET.302"), + ), + newFileMeta( + 310, + 1, + base.ParseInternalKey("c.SET.311"), + base.ParseInternalKey("g.SET.312"), + ), + newFileMeta( + 320, + 1, + base.ParseInternalKey("h.SET.321"), + base.ParseInternalKey("m.SET.322"), + ), + newFileMeta( + 330, + 1, + base.ParseInternalKey("n.SET.331"), + base.ParseInternalKey("z.SET.332"), + ), }, }), picker: compactionPickerForTesting{ @@ -335,44 +344,44 @@ func TestPickCompaction(t *testing.T) { desc: "4 L1 files, 2 L2 files, can grow", version: newVersion(opts, [numLevels][]*fileMetadata{ 1: { - { - FileNum: 200, - Size: 1, - Smallest: base.ParseInternalKey("i1.SET.201"), - Largest: base.ParseInternalKey("i2.SET.202"), - }, - { - FileNum: 210, - Size: 1, - Smallest: base.ParseInternalKey("j1.SET.211"), - Largest: base.ParseInternalKey("j2.SET.212"), - }, - { - FileNum: 220, - Size: 1, - Smallest: base.ParseInternalKey("k1.SET.221"), - Largest: base.ParseInternalKey("k2.SET.222"), - }, - { - FileNum: 230, - Size: 1, - Smallest: base.ParseInternalKey("l1.SET.231"), - Largest: base.ParseInternalKey("l2.SET.232"), - }, + newFileMeta( + 200, + 1, + base.ParseInternalKey("i1.SET.201"), + base.ParseInternalKey("i2.SET.202"), + ), + newFileMeta( + 210, + 1, + base.ParseInternalKey("j1.SET.211"), + base.ParseInternalKey("j2.SET.212"), + ), + newFileMeta( + 220, + 1, + base.ParseInternalKey("k1.SET.221"), + base.ParseInternalKey("k2.SET.222"), + ), + newFileMeta( + 230, + 1, + base.ParseInternalKey("l1.SET.231"), + base.ParseInternalKey("l2.SET.232"), + ), }, 2: { - { - FileNum: 300, - Size: 1, - Smallest: base.ParseInternalKey("a0.SET.301"), - Largest: base.ParseInternalKey("l0.SET.302"), - }, - { - FileNum: 310, - Size: 1, - Smallest: base.ParseInternalKey("l2.SET.311"), - Largest: base.ParseInternalKey("z2.SET.312"), - }, + newFileMeta( + 300, + 1, + base.ParseInternalKey("a0.SET.301"), + base.ParseInternalKey("l0.SET.302"), + ), + newFileMeta( + 310, + 1, + base.ParseInternalKey("l2.SET.311"), + base.ParseInternalKey("z2.SET.312"), + ), }, }), picker: compactionPickerForTesting{ @@ -387,44 +396,44 @@ func TestPickCompaction(t *testing.T) { desc: "4 L1 files, 2 L2 files, can't grow (range)", version: newVersion(opts, [numLevels][]*fileMetadata{ 1: { - { - FileNum: 200, - Size: 1, - Smallest: base.ParseInternalKey("i1.SET.201"), - Largest: base.ParseInternalKey("i2.SET.202"), - }, - { - FileNum: 210, - Size: 1, - Smallest: base.ParseInternalKey("j1.SET.211"), - Largest: base.ParseInternalKey("j2.SET.212"), - }, - { - FileNum: 220, - Size: 1, - Smallest: base.ParseInternalKey("k1.SET.221"), - Largest: base.ParseInternalKey("k2.SET.222"), - }, - { - FileNum: 230, - Size: 1, - Smallest: base.ParseInternalKey("l1.SET.231"), - Largest: base.ParseInternalKey("l2.SET.232"), - }, + newFileMeta( + 200, + 1, + base.ParseInternalKey("i1.SET.201"), + base.ParseInternalKey("i2.SET.202"), + ), + newFileMeta( + 210, + 1, + base.ParseInternalKey("j1.SET.211"), + base.ParseInternalKey("j2.SET.212"), + ), + newFileMeta( + 220, + 1, + base.ParseInternalKey("k1.SET.221"), + base.ParseInternalKey("k2.SET.222"), + ), + newFileMeta( + 230, + 1, + base.ParseInternalKey("l1.SET.231"), + base.ParseInternalKey("l2.SET.232"), + ), }, 2: { - { - FileNum: 300, - Size: 1, - Smallest: base.ParseInternalKey("a0.SET.301"), - Largest: base.ParseInternalKey("j0.SET.302"), - }, - { - FileNum: 310, - Size: 1, - Smallest: base.ParseInternalKey("j2.SET.311"), - Largest: base.ParseInternalKey("z2.SET.312"), - }, + newFileMeta( + 300, + 1, + base.ParseInternalKey("a0.SET.301"), + base.ParseInternalKey("j0.SET.302"), + ), + newFileMeta( + 310, + 1, + base.ParseInternalKey("j2.SET.311"), + base.ParseInternalKey("z2.SET.312"), + ), }, }), picker: compactionPickerForTesting{ @@ -439,44 +448,44 @@ func TestPickCompaction(t *testing.T) { desc: "4 L1 files, 2 L2 files, can't grow (size)", version: newVersion(opts, [numLevels][]*fileMetadata{ 1: { - { - FileNum: 200, - Size: expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("i1.SET.201"), - Largest: base.ParseInternalKey("i2.SET.202"), - }, - { - FileNum: 210, - Size: expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("j1.SET.211"), - Largest: base.ParseInternalKey("j2.SET.212"), - }, - { - FileNum: 220, - Size: expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("k1.SET.221"), - Largest: base.ParseInternalKey("k2.SET.222"), - }, - { - FileNum: 230, - Size: expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("l1.SET.231"), - Largest: base.ParseInternalKey("l2.SET.232"), - }, + newFileMeta( + 200, + expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64)-1, + base.ParseInternalKey("i1.SET.201"), + base.ParseInternalKey("i2.SET.202"), + ), + newFileMeta( + 210, + expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64)-1, + base.ParseInternalKey("j1.SET.211"), + base.ParseInternalKey("j2.SET.212"), + ), + newFileMeta( + 220, + expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64)-1, + base.ParseInternalKey("k1.SET.221"), + base.ParseInternalKey("k2.SET.222"), + ), + newFileMeta( + 230, + expandedCompactionByteSizeLimit(opts, 1, math.MaxUint64)-1, + base.ParseInternalKey("l1.SET.231"), + base.ParseInternalKey("l2.SET.232"), + ), }, 2: { - { - FileNum: 300, - Size: expandedCompactionByteSizeLimit(opts, 2, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("a0.SET.301"), - Largest: base.ParseInternalKey("l0.SET.302"), - }, - { - FileNum: 310, - Size: expandedCompactionByteSizeLimit(opts, 2, math.MaxUint64) - 1, - Smallest: base.ParseInternalKey("l2.SET.311"), - Largest: base.ParseInternalKey("z2.SET.312"), - }, + newFileMeta( + 300, + expandedCompactionByteSizeLimit(opts, 2, math.MaxUint64)-1, + base.ParseInternalKey("a0.SET.301"), + base.ParseInternalKey("l0.SET.302"), + ), + newFileMeta( + 310, + expandedCompactionByteSizeLimit(opts, 2, math.MaxUint64)-1, + base.ParseInternalKey("l2.SET.311"), + base.ParseInternalKey("z2.SET.312"), + ), }, }), picker: compactionPickerForTesting{ @@ -519,6 +528,12 @@ func TestElideTombstone(t *testing.T) { opts := &Options{} opts.EnsureDefaults() + newFileMeta := func(smallest, largest base.InternalKey) *fileMetadata { + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest, largest) + return m + } + type want struct { key string expected bool @@ -543,44 +558,44 @@ func TestElideTombstone(t *testing.T) { level: 1, version: newVersion(opts, [numLevels][]*fileMetadata{ 1: { - { - Smallest: base.ParseInternalKey("c.SET.801"), - Largest: base.ParseInternalKey("g.SET.800"), - }, - { - Smallest: base.ParseInternalKey("x.SET.701"), - Largest: base.ParseInternalKey("y.SET.700"), - }, + newFileMeta( + base.ParseInternalKey("c.SET.801"), + base.ParseInternalKey("g.SET.800"), + ), + newFileMeta( + base.ParseInternalKey("x.SET.701"), + base.ParseInternalKey("y.SET.700"), + ), }, 2: { - { - Smallest: base.ParseInternalKey("d.SET.601"), - Largest: base.ParseInternalKey("h.SET.600"), - }, - { - Smallest: base.ParseInternalKey("r.SET.501"), - Largest: base.ParseInternalKey("t.SET.500"), - }, + newFileMeta( + base.ParseInternalKey("d.SET.601"), + base.ParseInternalKey("h.SET.600"), + ), + newFileMeta( + base.ParseInternalKey("r.SET.501"), + base.ParseInternalKey("t.SET.500"), + ), }, 3: { - { - Smallest: base.ParseInternalKey("f.SET.401"), - Largest: base.ParseInternalKey("g.SET.400"), - }, - { - Smallest: base.ParseInternalKey("w.SET.301"), - Largest: base.ParseInternalKey("x.SET.300"), - }, + newFileMeta( + base.ParseInternalKey("f.SET.401"), + base.ParseInternalKey("g.SET.400"), + ), + newFileMeta( + base.ParseInternalKey("w.SET.301"), + base.ParseInternalKey("x.SET.300"), + ), }, 4: { - { - Smallest: base.ParseInternalKey("f.SET.201"), - Largest: base.ParseInternalKey("m.SET.200"), - }, - { - Smallest: base.ParseInternalKey("t.SET.101"), - Largest: base.ParseInternalKey("t.SET.100"), - }, + newFileMeta( + base.ParseInternalKey("f.SET.201"), + base.ParseInternalKey("m.SET.200"), + ), + newFileMeta( + base.ParseInternalKey("t.SET.101"), + base.ParseInternalKey("t.SET.100"), + ), }, }), wants: []want{ @@ -611,22 +626,22 @@ func TestElideTombstone(t *testing.T) { level: 1, version: newVersion(opts, [numLevels][]*fileMetadata{ 6: { - { - Smallest: base.ParseInternalKey("i.SET.401"), - Largest: base.ParseInternalKey("i.SET.400"), - }, - { - Smallest: base.ParseInternalKey("i.SET.301"), - Largest: base.ParseInternalKey("k.SET.300"), - }, - { - Smallest: base.ParseInternalKey("k.SET.201"), - Largest: base.ParseInternalKey("m.SET.200"), - }, - { - Smallest: base.ParseInternalKey("m.SET.101"), - Largest: base.ParseInternalKey("m.SET.100"), - }, + newFileMeta( + base.ParseInternalKey("i.SET.401"), + base.ParseInternalKey("i.SET.400"), + ), + newFileMeta( + base.ParseInternalKey("i.SET.301"), + base.ParseInternalKey("k.SET.300"), + ), + newFileMeta( + base.ParseInternalKey("k.SET.201"), + base.ParseInternalKey("m.SET.200"), + ), + newFileMeta( + base.ParseInternalKey("m.SET.101"), + base.ParseInternalKey("m.SET.100"), + ), }, }), wants: []want{ @@ -662,6 +677,12 @@ func TestElideTombstone(t *testing.T) { func TestElideRangeTombstone(t *testing.T) { opts := (*Options)(nil).EnsureDefaults() + newFileMeta := func(smallest, largest base.InternalKey) *fileMetadata { + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest, largest) + return m + } + type want struct { key string endKey string @@ -688,44 +709,44 @@ func TestElideRangeTombstone(t *testing.T) { level: 1, version: newVersion(opts, [numLevels][]*fileMetadata{ 1: { - { - Smallest: base.ParseInternalKey("c.SET.801"), - Largest: base.ParseInternalKey("g.SET.800"), - }, - { - Smallest: base.ParseInternalKey("x.SET.701"), - Largest: base.ParseInternalKey("y.SET.700"), - }, + newFileMeta( + base.ParseInternalKey("c.SET.801"), + base.ParseInternalKey("g.SET.800"), + ), + newFileMeta( + base.ParseInternalKey("x.SET.701"), + base.ParseInternalKey("y.SET.700"), + ), }, 2: { - { - Smallest: base.ParseInternalKey("d.SET.601"), - Largest: base.ParseInternalKey("h.SET.600"), - }, - { - Smallest: base.ParseInternalKey("r.SET.501"), - Largest: base.ParseInternalKey("t.SET.500"), - }, + newFileMeta( + base.ParseInternalKey("d.SET.601"), + base.ParseInternalKey("h.SET.600"), + ), + newFileMeta( + base.ParseInternalKey("r.SET.501"), + base.ParseInternalKey("t.SET.500"), + ), }, 3: { - { - Smallest: base.ParseInternalKey("f.SET.401"), - Largest: base.ParseInternalKey("g.SET.400"), - }, - { - Smallest: base.ParseInternalKey("w.SET.301"), - Largest: base.ParseInternalKey("x.SET.300"), - }, + newFileMeta( + base.ParseInternalKey("f.SET.401"), + base.ParseInternalKey("g.SET.400"), + ), + newFileMeta( + base.ParseInternalKey("w.SET.301"), + base.ParseInternalKey("x.SET.300"), + ), }, 4: { - { - Smallest: base.ParseInternalKey("f.SET.201"), - Largest: base.ParseInternalKey("m.SET.200"), - }, - { - Smallest: base.ParseInternalKey("t.SET.101"), - Largest: base.ParseInternalKey("t.SET.100"), - }, + newFileMeta( + base.ParseInternalKey("f.SET.201"), + base.ParseInternalKey("m.SET.200"), + ), + newFileMeta( + base.ParseInternalKey("t.SET.101"), + base.ParseInternalKey("t.SET.100"), + ), }, }), wants: []want{ @@ -755,20 +776,20 @@ func TestElideRangeTombstone(t *testing.T) { level: -1, version: newVersion(opts, [numLevels][]*fileMetadata{ 0: { - { - Smallest: base.ParseInternalKey("h.SET.901"), - Largest: base.ParseInternalKey("j.SET.900"), - }, + newFileMeta( + base.ParseInternalKey("h.SET.901"), + base.ParseInternalKey("j.SET.900"), + ), }, 1: { - { - Smallest: base.ParseInternalKey("c.SET.801"), - Largest: base.ParseInternalKey("g.SET.800"), - }, - { - Smallest: base.ParseInternalKey("x.SET.701"), - Largest: base.ParseInternalKey("y.SET.700"), - }, + newFileMeta( + base.ParseInternalKey("c.SET.801"), + base.ParseInternalKey("g.SET.800"), + ), + newFileMeta( + base.ParseInternalKey("x.SET.701"), + base.ParseInternalKey("y.SET.700"), + ), }, }), wants: []want{ @@ -945,6 +966,13 @@ func TestValidateVersionEdit(t *testing.T) { return nil } + cmp := DefaultComparer.Compare + newFileMeta := func(smallest, largest base.InternalKey) *fileMetadata { + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds(cmp, smallest, largest) + return m + } + testCases := []struct { desc string ve *versionEdit @@ -956,10 +984,10 @@ func TestValidateVersionEdit(t *testing.T) { ve: &versionEdit{ NewFiles: []manifest.NewFileEntry{ { - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte(badKey)}, - Largest: manifest.InternalKey{UserKey: []byte("z")}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte(badKey)}, + manifest.InternalKey{UserKey: []byte("z")}, + ), }, }, }, @@ -971,10 +999,10 @@ func TestValidateVersionEdit(t *testing.T) { ve: &versionEdit{ NewFiles: []manifest.NewFileEntry{ { - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte("a")}, - Largest: manifest.InternalKey{UserKey: []byte(badKey)}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte("a")}, + manifest.InternalKey{UserKey: []byte(badKey)}, + ), }, }, }, @@ -986,16 +1014,16 @@ func TestValidateVersionEdit(t *testing.T) { ve: &versionEdit{ NewFiles: []manifest.NewFileEntry{ { - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte("a")}, - Largest: manifest.InternalKey{UserKey: []byte("c")}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte("a")}, + manifest.InternalKey{UserKey: []byte("c")}, + ), }, { - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte(badKey)}, - Largest: manifest.InternalKey{UserKey: []byte("z")}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte(badKey)}, + manifest.InternalKey{UserKey: []byte("z")}, + ), }, }, }, @@ -1006,10 +1034,10 @@ func TestValidateVersionEdit(t *testing.T) { desc: "single deleted file; start key", ve: &versionEdit{ DeletedFiles: map[manifest.DeletedFileEntry]*manifest.FileMetadata{ - deletedFileEntry{Level: 0, FileNum: 0}: { - Smallest: manifest.InternalKey{UserKey: []byte(badKey)}, - Largest: manifest.InternalKey{UserKey: []byte("z")}, - }, + deletedFileEntry{Level: 0, FileNum: 0}: newFileMeta( + manifest.InternalKey{UserKey: []byte(badKey)}, + manifest.InternalKey{UserKey: []byte("z")}, + ), }, }, vFunc: validateFn, @@ -1019,10 +1047,10 @@ func TestValidateVersionEdit(t *testing.T) { desc: "single deleted file; end key", ve: &versionEdit{ DeletedFiles: map[manifest.DeletedFileEntry]*manifest.FileMetadata{ - deletedFileEntry{Level: 0, FileNum: 0}: { - Smallest: manifest.InternalKey{UserKey: []byte("a")}, - Largest: manifest.InternalKey{UserKey: []byte(badKey)}, - }, + deletedFileEntry{Level: 0, FileNum: 0}: newFileMeta( + manifest.InternalKey{UserKey: []byte("a")}, + manifest.InternalKey{UserKey: []byte(badKey)}, + ), }, }, vFunc: validateFn, @@ -1032,14 +1060,14 @@ func TestValidateVersionEdit(t *testing.T) { desc: "multiple deleted files", ve: &versionEdit{ DeletedFiles: map[manifest.DeletedFileEntry]*manifest.FileMetadata{ - deletedFileEntry{Level: 0, FileNum: 0}: { - Smallest: manifest.InternalKey{UserKey: []byte("a")}, - Largest: manifest.InternalKey{UserKey: []byte("c")}, - }, - deletedFileEntry{Level: 0, FileNum: 1}: { - Smallest: manifest.InternalKey{UserKey: []byte(badKey)}, - Largest: manifest.InternalKey{UserKey: []byte("z")}, - }, + deletedFileEntry{Level: 0, FileNum: 0}: newFileMeta( + manifest.InternalKey{UserKey: []byte("a")}, + manifest.InternalKey{UserKey: []byte("c")}, + ), + deletedFileEntry{Level: 0, FileNum: 1}: newFileMeta( + manifest.InternalKey{UserKey: []byte(badKey)}, + manifest.InternalKey{UserKey: []byte("z")}, + ), }, }, vFunc: validateFn, @@ -1051,28 +1079,28 @@ func TestValidateVersionEdit(t *testing.T) { NewFiles: []manifest.NewFileEntry{ { Level: 0, - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte("b")}, - Largest: manifest.InternalKey{UserKey: []byte("c")}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte("b")}, + manifest.InternalKey{UserKey: []byte("c")}, + ), }, { Level: 0, - Meta: &fileMetadata{ - Smallest: manifest.InternalKey{UserKey: []byte("d")}, - Largest: manifest.InternalKey{UserKey: []byte("g")}, - }, + Meta: newFileMeta( + manifest.InternalKey{UserKey: []byte("d")}, + manifest.InternalKey{UserKey: []byte("g")}, + ), }, }, DeletedFiles: map[manifest.DeletedFileEntry]*manifest.FileMetadata{ - deletedFileEntry{Level: 6, FileNum: 0}: { - Smallest: manifest.InternalKey{UserKey: []byte("a")}, - Largest: manifest.InternalKey{UserKey: []byte("d")}, - }, - deletedFileEntry{Level: 6, FileNum: 1}: { - Smallest: manifest.InternalKey{UserKey: []byte("x")}, - Largest: manifest.InternalKey{UserKey: []byte("z")}, - }, + deletedFileEntry{Level: 6, FileNum: 0}: newFileMeta( + manifest.InternalKey{UserKey: []byte("a")}, + manifest.InternalKey{UserKey: []byte("d")}, + ), + deletedFileEntry{Level: 6, FileNum: 1}: newFileMeta( + manifest.InternalKey{UserKey: []byte("x")}, + manifest.InternalKey{UserKey: []byte("z")}, + ), }, }, vFunc: validateFn, @@ -1357,11 +1385,13 @@ func TestCompactionFindGrandparentLimit(t *testing.T) { t.Fatalf("malformed table spec: %s", s) } fileNum++ - return &fileMetadata{ - FileNum: fileNum, - Smallest: InternalKey{UserKey: []byte(parts[0])}, - Largest: InternalKey{UserKey: []byte(parts[1])}, - } + m := &fileMetadata{FileNum: fileNum} + m.MaybeExtendPointKeyBounds( + cmp, + InternalKey{UserKey: []byte(parts[0])}, + InternalKey{UserKey: []byte(parts[1])}, + ) + return m } datadriven.RunTest(t, "testdata/compaction_find_grandparent_limit", @@ -1440,11 +1470,12 @@ func TestCompactionFindL0Limit(t *testing.T) { if len(parts) != 2 { return nil, errors.Errorf("malformed table spec: %s", s) } - m := &fileMetadata{ - FileNum: base.FileNum(fileNumCounter), - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := &fileMetadata{FileNum: base.FileNum(fileNumCounter)} + m.MaybeExtendPointKeyBounds( + cmp, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) fileNumCounter++ m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() @@ -1586,10 +1617,13 @@ func TestCompactionAtomicUnitBounds(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( + cmp, + base.ParseInternalKey(parts[0]), + base.ParseInternalKey(parts[1]), + ) + return m } datadriven.RunTest(t, "testdata/compaction_atomic_unit_bounds", @@ -2184,15 +2218,18 @@ func TestCompactionReadTriggered(t *testing.T) { } func TestCompactionInuseKeyRanges(t *testing.T) { + cmp := DefaultComparer.Compare parseMeta := func(s string) *fileMetadata { parts := strings.Split(s, "-") if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } - m := &fileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := &fileMetadata{} + m.MaybeExtendRangeKeyBounds( + cmp, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() return m @@ -2299,11 +2336,8 @@ func TestCompactionInuseKeyRangesRandomized(t *testing.T) { } makeFile := func(level, start, end int) *fileMetadata { fileNum++ - m := &fileMetadata{ - FileNum: fileNum, - Smallest: makeIK(level, start), - Largest: makeIK(level, end), - } + m := &fileMetadata{FileNum: fileNum} + m.MaybeExtendPointKeyBounds(opts.Comparer.Compare, makeIK(level, start), makeIK(level, end)) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() return m @@ -2396,11 +2430,12 @@ func TestCompactionAllowZeroSeqNum(t *testing.T) { t.Fatalf("malformed table spec: %s: %s", s, err) } fileNum++ - meta = &fileMetadata{ - FileNum: fileNum, - Smallest: InternalKey{UserKey: []byte(match[2])}, - Largest: InternalKey{UserKey: []byte(match[3])}, - } + meta = &fileMetadata{FileNum: fileNum} + meta.MaybeExtendPointKeyBounds( + d.cmp, + InternalKey{UserKey: []byte(match[2])}, + InternalKey{UserKey: []byte(match[3])}, + ) return level, meta } @@ -2491,15 +2526,18 @@ func TestCompactionAllowZeroSeqNum(t *testing.T) { } func TestCompactionErrorOnUserKeyOverlap(t *testing.T) { + cmp := DefaultComparer.Compare parseMeta := func(s string) *fileMetadata { parts := strings.Split(s, "-") if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } - m := &fileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds( + cmp, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() return m @@ -2615,15 +2653,18 @@ func TestCompactionErrorCleanup(t *testing.T) { } func TestCompactionCheckOrdering(t *testing.T) { + cmp := DefaultComparer.Compare parseMeta := func(s string) *fileMetadata { parts := strings.Split(s, "-") if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } - m := &fileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds( + cmp, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() return m diff --git a/data_test.go b/data_test.go index eeb6f2c4d0..09b088364b 100644 --- a/data_test.go +++ b/data_test.go @@ -632,10 +632,13 @@ func runDBDefineCmd(td *datadriven.TestData, opts *Options) (*DB, error) { if len(parts) != 2 { return nil, errors.Errorf("malformed table spec: %s", s) } - return &fileMetadata{ - Smallest: InternalKey{UserKey: []byte(parts[0])}, - Largest: InternalKey{UserKey: []byte(parts[1])}, - }, nil + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds( + opts.Comparer.Compare, + InternalKey{UserKey: []byte(parts[0])}, + InternalKey{UserKey: []byte(parts[1])}, + ) + return m, nil } // Example, compact: a-c. diff --git a/db.go b/db.go index 271b3ddbad..6a8753942d 100644 --- a/db.go +++ b/db.go @@ -1220,7 +1220,9 @@ func (d *DB) Compact( } iStart := base.MakeInternalKey(start, InternalKeySeqNumMax, InternalKeyKindMax) iEnd := base.MakeInternalKey(end, 0, 0) - meta := []*fileMetadata{{Smallest: iStart, Largest: iEnd}} + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds(d.cmp, iStart, iEnd) + meta := []*fileMetadata{m} d.mu.Lock() maxLevelWithFiles := 1 diff --git a/flush_external.go b/flush_external.go index ccf7a0bbe0..95f68a0129 100644 --- a/flush_external.go +++ b/flush_external.go @@ -30,17 +30,17 @@ func flushExternalTable(untypedDB interface{}, path string, originalMeta *fileMe d.mu.Unlock() m := &fileMetadata{ - FileNum: fileNum, - Size: originalMeta.Size, - CreationTime: time.Now().Unix(), - SmallestPointKey: originalMeta.SmallestPointKey, - LargestPointKey: originalMeta.LargestPointKey, - SmallestRangeKey: originalMeta.SmallestRangeKey, - LargestRangeKey: originalMeta.LargestRangeKey, - Smallest: originalMeta.Smallest, - Largest: originalMeta.Largest, - SmallestSeqNum: originalMeta.SmallestSeqNum, - LargestSeqNum: originalMeta.LargestSeqNum, + FileNum: fileNum, + Size: originalMeta.Size, + CreationTime: time.Now().Unix(), + SmallestSeqNum: originalMeta.SmallestSeqNum, + LargestSeqNum: originalMeta.LargestSeqNum, + } + if originalMeta.HasPointKeys { + m.MaybeExtendPointKeyBounds(d.cmp, originalMeta.SmallestPointKey, originalMeta.LargestPointKey) + } + if originalMeta.HasRangeKeys { + m.MaybeExtendRangeKeyBounds(d.cmp, originalMeta.SmallestRangeKey, originalMeta.LargestRangeKey) } // Hard link the sstable into the DB directory. diff --git a/get_iter_test.go b/get_iter_test.go index 79a28fa83c..9343b2255a 100644 --- a/get_iter_test.go +++ b/get_iter_test.go @@ -496,18 +496,11 @@ func TestGetIter(t *testing.T) { t.Fatalf("desc=%q: memtable Set: %v", desc, err) } + meta.MaybeExtendPointKeyBounds(cmp, ikey, ikey) if i == 0 { - meta.Smallest = ikey meta.SmallestSeqNum = ikey.SeqNum() - meta.Largest = ikey meta.LargestSeqNum = ikey.SeqNum() } else { - if base.InternalCompare(cmp, ikey, meta.Smallest) < 0 { - meta.Smallest = ikey - } - if base.InternalCompare(cmp, ikey, meta.Largest) > 0 { - meta.Largest = ikey - } if meta.SmallestSeqNum > ikey.SeqNum() { meta.SmallestSeqNum = ikey.SeqNum() } diff --git a/ingest.go b/ingest.go index 6f94c75a9c..4e4bdbed04 100644 --- a/ingest.go +++ b/ingest.go @@ -97,18 +97,18 @@ func ingestLoad1( // calculating stats before we can remove the original link. maybeSetStatsFromProperties(meta, &r.Properties) - hasPoints := false { iter, err := r.NewIter(nil /* lower */, nil /* upper */) if err != nil { return nil, err } defer iter.Close() + var smallest InternalKey if key, _ := iter.First(); key != nil { if err := ingestValidateKey(opts, key); err != nil { return nil, err } - meta.SmallestPointKey = key.Clone() + smallest = (*key).Clone() } if err := iter.Error(); err != nil { return nil, err @@ -117,8 +117,7 @@ func ingestLoad1( if err := ingestValidateKey(opts, key); err != nil { return nil, err } - meta.LargestPointKey = key.Clone() - hasPoints = true // Implies smallest point key was also set. + meta.MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest, key.Clone()) } if err := iter.Error(); err != nil { return nil, err @@ -131,14 +130,12 @@ func ingestLoad1( } if iter != nil { defer iter.Close() + var smallest InternalKey if key, _ := iter.First(); key != nil { if err := ingestValidateKey(opts, key); err != nil { return nil, err } - if !hasPoints || - base.InternalCompare(opts.Comparer.Compare, meta.SmallestPointKey, *key) > 0 { - meta.SmallestPointKey = key.Clone() - } + smallest = (*key).Clone() } if err := iter.Error(); err != nil { return nil, err @@ -148,16 +145,11 @@ func ingestLoad1( return nil, err } end := base.MakeRangeDeleteSentinelKey(val) - if !hasPoints || - base.InternalCompare(opts.Comparer.Compare, meta.LargestPointKey, end) < 0 { - meta.LargestPointKey = end.Clone() - hasPoints = true // Implies smallest point key was also set. - } + meta.MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest, end.Clone()) } } // Update the range-key bounds for the table. - var hasRanges bool { iter, err := r.NewRawRangeKeyIter() if err != nil { @@ -165,11 +157,12 @@ func ingestLoad1( } if iter != nil { defer iter.Close() + var smallest InternalKey if key, _ := iter.First(); key != nil { if err := ingestValidateKey(opts, key); err != nil { return nil, err } - meta.SmallestRangeKey = key.Clone() + smallest = (*key).Clone() } if err := iter.Error(); err != nil { return nil, err @@ -184,8 +177,8 @@ func ingestLoad1( if !ok { return nil, errors.Newf("pebble: could not decode range end key") } - meta.LargestRangeKey = base.MakeRangeKeySentinelKey(key.Kind(), end).Clone() - hasRanges = true // Implies smallest range key was also set. + k := base.MakeRangeKeySentinelKey(key.Kind(), end).Clone() + meta.MaybeExtendRangeKeyBounds(opts.Comparer.Compare, smallest, k.Clone()) } if err := iter.Error(); err != nil { return nil, err @@ -193,36 +186,10 @@ func ingestLoad1( } } - if !hasPoints && !hasRanges { + if !meta.HasPointKeys && !meta.HasRangeKeys { return nil, nil } - // Compute the overall smallest / largest fields from the point and key - // ranges. - switch { - case !hasRanges: - // Table has only point keys. Use the point key bounds. - meta.Smallest = meta.SmallestPointKey - meta.Largest = meta.LargestPointKey - case !hasPoints: - // Table has only range key. Use the range key bounds. - meta.Smallest = meta.SmallestRangeKey - meta.Largest = meta.LargestRangeKey - default: - // Table has both points and ranges. Compute the bounds by considering both - // the point and range key bounds. - if base.InternalCompare(opts.Comparer.Compare, meta.SmallestPointKey, meta.SmallestRangeKey) < 0 { - meta.Smallest = meta.SmallestPointKey - } else { - meta.Smallest = meta.SmallestRangeKey - } - if base.InternalCompare(opts.Comparer.Compare, meta.LargestPointKey, meta.LargestRangeKey) > 0 { - meta.Largest = meta.LargestPointKey - } else { - meta.Largest = meta.LargestRangeKey - } - } - // Sanity check that the various bounds on the file were set consistently. if err := meta.Validate(opts.Comparer.Compare, opts.Comparer.FormatKey); err != nil { return nil, err @@ -385,13 +352,12 @@ func ingestUpdateSeqNum( return base.MakeInternalKey(k.UserKey, seqNum, k.Kind()) } for _, m := range meta { - // TODO(travers): We currently lack a means of determining whether one of - // the bounds is "unset" (e.g. a table without point keys). We use a lack of - // user key as a cheap way of checking, for now, until we have a better way. - if m.SmallestPointKey.UserKey != nil { + // NB: we set the fields directly here, rather than via their Extend* + // methods, as we are updating sequence numbers. + if m.HasPointKeys { m.SmallestPointKey = setSeqFn(m.SmallestPointKey) } - if m.SmallestRangeKey.UserKey != nil { + if m.HasRangeKeys { m.SmallestRangeKey = setSeqFn(m.SmallestRangeKey) } m.Smallest = setSeqFn(m.Smallest) @@ -402,7 +368,7 @@ func ingestUpdateSeqNum( // table. // NB: as the largest range key is always an exclusive sentinel, it is never // updated. - if m.LargestPointKey.UserKey != nil && !m.LargestPointKey.IsExclusiveSentinel() { + if m.HasPointKeys && !m.LargestPointKey.IsExclusiveSentinel() { m.LargestPointKey = setSeqFn(m.LargestPointKey) } if !m.Largest.IsExclusiveSentinel() { diff --git a/ingest_test.go b/ingest_test.go index 7239c3e11c..a7fda40bbb 100644 --- a/ingest_test.go +++ b/ingest_test.go @@ -140,10 +140,7 @@ func TestIngestLoadRand(t *testing.T) { return base.InternalCompare(cmp, keys[i], keys[j]) < 0 }) - expected[i].SmallestPointKey = keys[0] - expected[i].LargestPointKey = keys[len(keys)-1] - expected[i].Smallest = expected[i].SmallestPointKey - expected[i].Largest = expected[i].LargestPointKey + expected[i].MaybeExtendPointKeyBounds(cmp, keys[0], keys[len(keys)-1]) w := sstable.NewWriter(f, sstable.WriterOptions{}) var count uint64 @@ -226,10 +223,9 @@ func TestIngestSortAndVerify(t *testing.T) { if cmp(smallest.UserKey, largest.UserKey) > 0 { return fmt.Sprintf("range %v-%v is not valid", smallest, largest) } - meta = append(meta, &fileMetadata{ - Smallest: smallest, - Largest: largest, - }) + m := &fileMetadata{} + m.MaybeExtendPointKeyBounds(cmp, smallest, largest) + meta = append(meta, m) paths = append(paths, strconv.Itoa(i)) } err := ingestSortAndVerify(cmp, meta, paths) @@ -383,19 +379,22 @@ func TestIngestMemtableOverlaps(t *testing.T) { if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } + var smallest, largest base.InternalKey if strings.Contains(parts[0], ".") { if !strings.Contains(parts[1], ".") { t.Fatalf("malformed table spec: %s", s) } - meta.Smallest = base.ParseInternalKey(parts[0]) - meta.Largest = base.ParseInternalKey(parts[1]) + smallest = base.ParseInternalKey(parts[0]) + largest = base.ParseInternalKey(parts[1]) } else { - meta.Smallest = InternalKey{UserKey: []byte(parts[0])} - meta.Largest = InternalKey{UserKey: []byte(parts[1])} + smallest = InternalKey{UserKey: []byte(parts[0])} + largest = InternalKey{UserKey: []byte(parts[1])} } - if mem.cmp(meta.Smallest.UserKey, meta.Largest.UserKey) > 0 { - meta.Smallest, meta.Largest = meta.Largest, meta.Smallest + // If we're using a reverse comparer, flip the file bounds. + if mem.cmp(smallest.UserKey, largest.UserKey) > 0 { + smallest, largest = largest, smallest } + meta.MaybeExtendPointKeyBounds(comparer.Compare, smallest, largest) return meta } @@ -461,10 +460,13 @@ func TestIngestTargetLevel(t *testing.T) { if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } - return fileMetadata{ - Smallest: InternalKey{UserKey: []byte(parts[0])}, - Largest: InternalKey{UserKey: []byte(parts[1])}, - } + m := fileMetadata{} + m.MaybeExtendPointKeyBounds( + d.cmp, + InternalKey{UserKey: []byte(parts[0])}, + InternalKey{UserKey: []byte(parts[1])}, + ) + return m } datadriven.RunTest(t, "testdata/ingest_target_level", func(td *datadriven.TestData) string { @@ -1243,14 +1245,28 @@ func TestIngest_UpdateSequenceNumber(t *testing.T) { // Construct the file metadata from the writer metadata. m := &fileMetadata{ - SmallestPointKey: wm.SmallestPointKey(cmp), - LargestPointKey: maybeUpdateUpperBound(wm.LargestPointKey(cmp)), - SmallestRangeKey: wm.SmallestRangeKey, - LargestRangeKey: maybeUpdateUpperBound(wm.LargestRangeKey), - Smallest: wm.Smallest(cmp), - Largest: maybeUpdateUpperBound(wm.Largest(cmp)), - SmallestSeqNum: 0, // Simulate an ingestion. - LargestSeqNum: 0, + SmallestSeqNum: 0, // Simulate an ingestion. + LargestSeqNum: 0, + } + if wm.HasPointKeys { + m.MaybeExtendPointKeyBounds(cmp, wm.SmallestPoint, wm.LargestPoint) + } + if wm.HasRangeDelKeys { + m.MaybeExtendPointKeyBounds( + cmp, + wm.SmallestRangeDel, + maybeUpdateUpperBound(wm.LargestRangeDel), + ) + } + if wm.HasRangeKeys { + m.MaybeExtendRangeKeyBounds( + cmp, + wm.SmallestRangeKey, + maybeUpdateUpperBound(wm.LargestRangeKey), + ) + } + if err := m.Validate(cmp, base.DefaultFormatter); err != nil { + return err.Error() } // Collect this file. diff --git a/internal/manifest/btree_test.go b/internal/manifest/btree_test.go index 6fdb826596..7cad2813b3 100644 --- a/internal/manifest/btree_test.go +++ b/internal/manifest/btree_test.go @@ -19,10 +19,9 @@ import ( ) func newItem(k InternalKey) *FileMetadata { - return &FileMetadata{ - Smallest: k, - Largest: k, - } + m := &FileMetadata{} + m.MaybeExtendPointKeyBounds(base.DefaultComparer.Compare, k, k) + return m } func cmp(a, b *FileMetadata) int { diff --git a/internal/manifest/l0_sublevels_test.go b/internal/manifest/l0_sublevels_test.go index ce196ce915..6819cf057b 100644 --- a/internal/manifest/l0_sublevels_test.go +++ b/internal/manifest/l0_sublevels_test.go @@ -217,10 +217,12 @@ func TestL0Sublevels(t *testing.T) { } fields := strings.Fields(parts[1]) keyRange := strings.Split(strings.TrimSpace(fields[0]), "-") - m := FileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(keyRange[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(keyRange[1])), - } + m := FileMetadata{} + m.MaybeExtendPointKeyBounds( + base.DefaultComparer.Compare, + base.ParseInternalKey(strings.TrimSpace(keyRange[0])), + base.ParseInternalKey(strings.TrimSpace(keyRange[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() if m.Largest.IsExclusiveSentinel() { @@ -584,11 +586,14 @@ func TestAddL0FilesEquivalence(t *testing.T) { meta := &FileMetadata{ FileNum: base.FileNum(i*10 + j + 1), Size: rng.Uint64n(1 << 20), - Smallest: base.MakeInternalKey(startKey, uint64(2*i+1), base.InternalKeyKindSet), - Largest: base.MakeRangeDeleteSentinelKey(endKey), SmallestSeqNum: uint64(2*i + 1), LargestSeqNum: uint64(2*i + 2), } + meta.MaybeExtendPointKeyBounds( + base.DefaultComparer.Compare, + base.MakeInternalKey(startKey, uint64(2*i+1), base.InternalKeyKindSet), + base.MakeRangeDeleteSentinelKey(endKey), + ) fileMetas = append(fileMetas, meta) filesToAdd = append(filesToAdd, meta) } diff --git a/internal/manifest/level_metadata_test.go b/internal/manifest/level_metadata_test.go index 351044434e..dc63a753bf 100644 --- a/internal/manifest/level_metadata_test.go +++ b/internal/manifest/level_metadata_test.go @@ -38,11 +38,12 @@ func TestLevelIterator(t *testing.T) { if len(parts) != 2 { t.Fatalf("malformed table spec: %q", metaStr) } - m := &FileMetadata{ - FileNum: base.FileNum(len(files) + 1), - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := &FileMetadata{FileNum: base.FileNum(len(files) + 1)} + m.MaybeExtendPointKeyBounds( + base.DefaultComparer.Compare, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() files = append(files, m) diff --git a/internal/manifest/testdata/file_metadata_bounds b/internal/manifest/testdata/file_metadata_bounds new file mode 100644 index 0000000000..cd53785021 --- /dev/null +++ b/internal/manifest/testdata/file_metadata_bounds @@ -0,0 +1,85 @@ +# Points only (single update). + +extend-point-key-bounds +a.SET.0 - z.DEL.42 +---- +combined: [a#0,1-z#42,0] + points: [a#0,1-z#42,0] + +# Rangedels only (single update). + +reset +---- + +extend-point-key-bounds +a.RANGEDEL.0:z +---- +combined: [a#0,15-z#72057594037927935,15] + points: [a#0,15-z#72057594037927935,15] + +# Range keys only (single update). + +reset +---- + +extend-range-key-bounds +a.RANGEKEYSET.0:z +---- +combined: [a#0,21-z#72057594037927935,21] + ranges: [a#0,21-z#72057594037927935,21] + +# Multiple updates with various key kinds. + +reset +---- + +extend-point-key-bounds +m.SET.0 - n.SET.0 +---- +combined: [m#0,1-n#0,1] + points: [m#0,1-n#0,1] + +# Extend the lower point key bound. + +extend-point-key-bounds +j.SET.0 - k.SET.0 +---- +combined: [j#0,1-n#0,1] + points: [j#0,1-n#0,1] + +# Extend the upper point key bound with a rangedel. + +extend-point-key-bounds +k.RANGEDEL.0:o +---- +combined: [j#0,1-o#72057594037927935,15] + points: [j#0,1-o#72057594037927935,15] + +# Extend the lower bounds bound with a range key. + +extend-range-key-bounds +a.RANGEKEYSET.42:m +---- +combined: [a#42,21-o#72057594037927935,15] + points: [j#0,1-o#72057594037927935,15] + ranges: [a#42,21-m#72057594037927935,21] + +# Extend again with a wide range key (equal keys tiebreak on seqnums descending, +# so the overall lower bound is unchanged). + +extend-range-key-bounds +a.RANGEKEYSET.0:z +---- +combined: [a#42,21-z#72057594037927935,21] + points: [j#0,1-o#72057594037927935,15] + ranges: [a#42,21-z#72057594037927935,21] + +# Extend again with a wide rangedel over the same range (equal keys and sequnums +# tiebreak on key kind descending, so the overall upper bound is updated). + +extend-point-key-bounds +a.RANGEDEL.0:z +---- +combined: [a#42,21-z#72057594037927935,15] + points: [a#0,15-z#72057594037927935,15] + ranges: [a#42,21-z#72057594037927935,21] diff --git a/internal/manifest/version.go b/internal/manifest/version.go index cd507de2d3..b57d571d98 100644 --- a/internal/manifest/version.go +++ b/internal/manifest/version.go @@ -99,20 +99,26 @@ type FileMetadata struct { // SmallestPointKey and LargestPointKey are the inclusive bounds for the // internal point keys stored in the table. This includes RANGEDELs, which // alter point keys. - // TODO(travers): A table may not have point or range keys, in which case it - // will have zero-valued smallest and largest keys. Introduce a sentinel key - // to indicate "absence" of the bound. + // NB: these field should be set using MaybeExtendPointKeyBounds. They are + // left exported for reads as an optimization. SmallestPointKey InternalKey LargestPointKey InternalKey + HasPointKeys bool // SmallestRangeKey and LargestRangeKey are the inclusive bounds for the // internal range keys stored in the table. + // NB: these field should be set using MaybeExtendRangeKeyBounds. They are + // left exported for reads as an optimization. SmallestRangeKey InternalKey LargestRangeKey InternalKey + HasRangeKeys bool // Smallest and Largest are the inclusive bounds for the internal keys stored - // in the table, across both point and range keys. These values can be - // reconstructed from the respective point and range key fields. + // in the table, across both point and range keys. + // NB: these fields are derived from their point and range key equivalents, + // and are updated via the MaybeExtend{Point,Range}KeyBounds methods. Smallest InternalKey Largest InternalKey + // smallestSet and largestSet track whether the overall bounds have been set. + boundsSet bool // Smallest and largest sequence numbers in the table, across both point and // range keys. SmallestSeqNum uint64 @@ -140,6 +146,69 @@ type FileMetadata struct { markedForCompaction bool } +// MaybeExtendPointKeyBounds attempts to extend the lower and upper point key +// bounds and overall table bounds with the given smallest and largest keys. The +// smallest and largest bounds may not be extended if the table already has a +// bound that is smaller or larger, respectively. +// NB: calling this method should be preferred to manually setting the bounds by +// manipulating the fields directly, to maintain certain invariants. +func (m *FileMetadata) MaybeExtendPointKeyBounds(cmp Compare, smallest, largest InternalKey) { + // Update the point key bounds. + if !m.HasPointKeys { + m.SmallestPointKey, m.LargestPointKey = smallest, largest + m.HasPointKeys = true + } else { + if base.InternalCompare(cmp, smallest, m.SmallestPointKey) < 0 { + m.SmallestPointKey = smallest + } + if base.InternalCompare(cmp, largest, m.LargestPointKey) > 0 { + m.LargestPointKey = largest + } + } + // Update the overall bounds. + m.maybeExtendOverallBounds(cmp, m.SmallestPointKey, m.LargestPointKey) +} + +// MaybeExtendRangeKeyBounds attempts to extend the lower and upper range key +// bounds and overall table bounds with the given smallest and largest keys. The +// smallest and largest bounds may not be extended if the table already has a +// bound that is smaller or larger, respectively. +// NB: calling this method should be preferred to manually setting the bounds by +// manipulating the fields directly, to maintain certain invariants. +func (m *FileMetadata) MaybeExtendRangeKeyBounds(cmp Compare, smallest, largest InternalKey) { + // Update the range key bounds. + if !m.HasRangeKeys { + m.SmallestRangeKey, m.LargestRangeKey = smallest, largest + m.HasRangeKeys = true + } else { + if base.InternalCompare(cmp, smallest, m.SmallestRangeKey) < 0 { + m.SmallestRangeKey = smallest + } + if base.InternalCompare(cmp, largest, m.LargestRangeKey) > 0 { + m.LargestRangeKey = largest + } + } + // Update the overall bounds. + m.maybeExtendOverallBounds(cmp, m.SmallestRangeKey, m.LargestRangeKey) +} + +// maybeExtendOverallBounds attempts to extend the overall table lower and upper +// bounds. The given bounds may not be used if a lower or upper bound already +// exists that is smaller or larger than the given keys, respectively. +func (m *FileMetadata) maybeExtendOverallBounds(cmp Compare, smallest, largest InternalKey) { + if !m.boundsSet { + m.Smallest, m.Largest = smallest, largest + m.boundsSet = true + } else { + if base.InternalCompare(cmp, smallest, m.Smallest) < 0 { + m.Smallest = smallest + } + if base.InternalCompare(cmp, largest, m.Largest) > 0 { + m.Largest = largest + } + } +} + func (m *FileMetadata) String() string { return fmt.Sprintf("%s:%s-%s", m.FileNum, m.Smallest, m.Largest) } @@ -147,24 +216,12 @@ func (m *FileMetadata) String() string { // Validate validates the metadata for consistency with itself, returning an // error if inconsistent. func (m *FileMetadata) Validate(cmp Compare, formatKey base.FormatKey) error { - // Point key validation. - - if base.InternalCompare(cmp, m.SmallestPointKey, m.LargestPointKey) > 0 { - return base.CorruptionErrorf("file %s has inconsistent point key bounds: %s vs %s", - errors.Safe(m.FileNum), m.SmallestPointKey.Pretty(formatKey), - m.LargestPointKey.Pretty(formatKey)) - } - - // Range key validation. - - if base.InternalCompare(cmp, m.SmallestRangeKey, m.LargestRangeKey) > 0 { - return base.CorruptionErrorf("file %s has inconsistent range key bounds: %s vs %s", - errors.Safe(m.FileNum), m.SmallestRangeKey.Pretty(formatKey), - m.LargestRangeKey.Pretty(formatKey)) - } - // Combined range and point key validation. + if !m.HasPointKeys && !m.HasRangeKeys { + return base.CorruptionErrorf("file %s has neither point nor range keys", + errors.Safe(m.FileNum)) + } if base.InternalCompare(cmp, m.Smallest, m.Largest) > 0 { return base.CorruptionErrorf("file %s has inconsistent bounds: %s vs %s", errors.Safe(m.FileNum), m.Smallest.Pretty(formatKey), @@ -175,11 +232,45 @@ func (m *FileMetadata) Validate(cmp Compare, formatKey base.FormatKey) error { errors.Safe(m.FileNum), m.SmallestSeqNum, m.LargestSeqNum) } - // TODO(travers): add consistency checks to ensure that the point / range key - // smallest / largest are within the bounds of the combined smallest / - // largest. However, first we need a "sentinel key" for indicating whether the - // point or range key bounds are actually set. See the TODO on - // SmallestPointKey. + // Point key validation. + + if m.HasPointKeys { + if base.InternalCompare(cmp, m.SmallestPointKey, m.LargestPointKey) > 0 { + return base.CorruptionErrorf("file %s has inconsistent point key bounds: %s vs %s", + errors.Safe(m.FileNum), m.SmallestPointKey.Pretty(formatKey), + m.LargestPointKey.Pretty(formatKey)) + } + if base.InternalCompare(cmp, m.SmallestPointKey, m.Smallest) < 0 || + base.InternalCompare(cmp, m.LargestPointKey, m.Largest) > 0 { + return base.CorruptionErrorf( + "file %s has inconsistent point key bounds relative to overall bounds: "+ + "overall = [%s-%s], point keys = [%s-%s]", + errors.Safe(m.FileNum), + m.Smallest.Pretty(formatKey), m.Largest.Pretty(formatKey), + m.SmallestPointKey.Pretty(formatKey), m.LargestPointKey.Pretty(formatKey), + ) + } + } + + // Range key validation. + + if m.HasRangeKeys { + if base.InternalCompare(cmp, m.SmallestRangeKey, m.LargestRangeKey) > 0 { + return base.CorruptionErrorf("file %s has inconsistent range key bounds: %s vs %s", + errors.Safe(m.FileNum), m.SmallestRangeKey.Pretty(formatKey), + m.LargestRangeKey.Pretty(formatKey)) + } + if base.InternalCompare(cmp, m.SmallestRangeKey, m.Smallest) < 0 || + base.InternalCompare(cmp, m.LargestRangeKey, m.Largest) > 0 { + return base.CorruptionErrorf( + "file %s has inconsistent range key bounds relative to overall bounds: "+ + "overall = [%s-%s], range keys = [%s-%s]", + errors.Safe(m.FileNum), + m.Smallest.Pretty(formatKey), m.Largest.Pretty(formatKey), + m.SmallestRangeKey.Pretty(formatKey), m.LargestRangeKey.Pretty(formatKey), + ) + } + } return nil } @@ -509,13 +600,11 @@ func ParseVersionDebug( } smallest := base.ParsePrettyInternalKey(fields[1]) largest := base.ParsePrettyInternalKey(fields[2]) - files[level] = append(files[level], &FileMetadata{ - FileNum: base.FileNum(fileNum), - SmallestPointKey: smallest, - LargestPointKey: largest, - Smallest: smallest, - Largest: largest, - }) + m := &FileMetadata{ + FileNum: base.FileNum(fileNum), + } + m.MaybeExtendPointKeyBounds(cmp, smallest, largest) + files[level] = append(files[level], m) } } // Reverse the order of L0 files. This ensures we construct the same diff --git a/internal/manifest/version_edit.go b/internal/manifest/version_edit.go index e759158d04..6829381d68 100644 --- a/internal/manifest/version_edit.go +++ b/internal/manifest/version_edit.go @@ -252,17 +252,29 @@ func (v *VersionEdit) Decode(r io.Reader) error { } } } + smallestPointKey := base.DecodeInternalKey(smallest) + largestPointKey := base.DecodeInternalKey(largest) v.NewFiles = append(v.NewFiles, NewFileEntry{ Level: level, Meta: &FileMetadata{ FileNum: fileNum, Size: size, CreationTime: int64(creationTime), - Smallest: base.DecodeInternalKey(smallest), - Largest: base.DecodeInternalKey(largest), + SmallestPointKey: smallestPointKey, + LargestPointKey: largestPointKey, + HasPointKeys: true, SmallestSeqNum: smallestSeqNum, LargestSeqNum: largestSeqNum, markedForCompaction: markedForCompaction, + // TODO(travers): For now the smallest and largest keys are pinned to + // the smallest and largest point keys, as these are the only types of + // keys supported in the manifest. This will need to change when the + // manifest is updated to support range keys, which will most likely + // leverage a bitset to infer which key types (points or ranges) are + // used for the overall smallest and largest keys. + Smallest: smallestPointKey, + Largest: largestPointKey, + boundsSet: true, }, }) diff --git a/internal/manifest/version_edit_test.go b/internal/manifest/version_edit_test.go index c99f6dedba..59310f5aba 100644 --- a/internal/manifest/version_edit_test.go +++ b/internal/manifest/version_edit_test.go @@ -40,6 +40,32 @@ func checkRoundTrip(e0 VersionEdit) error { } func TestVersionEditRoundTrip(t *testing.T) { + cmp := base.DefaultComparer.Compare + m1 := &FileMetadata{ + FileNum: 805, + Size: 8050, + CreationTime: 805030, + } + m1.MaybeExtendPointKeyBounds( + cmp, + base.DecodeInternalKey([]byte("abc\x00\x01\x02\x03\x04\x05\x06\x07")), + base.DecodeInternalKey([]byte("xyz\x01\xff\xfe\xfd\xfc\xfb\xfa\xf9")), + ) + + m2 := &FileMetadata{ + FileNum: 806, + Size: 8060, + CreationTime: 806040, + SmallestSeqNum: 3, + LargestSeqNum: 5, + markedForCompaction: true, + } + m2.MaybeExtendPointKeyBounds( + cmp, + base.DecodeInternalKey([]byte("A\x00\x01\x02\x03\x04\x05\x06\x07")), + base.DecodeInternalKey([]byte("Z\x01\xff\xfe\xfd\xfc\xfb\xfa\xf9")), + ) + testCases := []VersionEdit{ // An empty version edit. {}, @@ -63,26 +89,11 @@ func TestVersionEditRoundTrip(t *testing.T) { NewFiles: []NewFileEntry{ { Level: 5, - Meta: &FileMetadata{ - FileNum: 805, - Size: 8050, - CreationTime: 805030, - Smallest: base.DecodeInternalKey([]byte("abc\x00\x01\x02\x03\x04\x05\x06\x07")), - Largest: base.DecodeInternalKey([]byte("xyz\x01\xff\xfe\xfd\xfc\xfb\xfa\xf9")), - }, + Meta: m1, }, { Level: 6, - Meta: &FileMetadata{ - FileNum: 806, - Size: 8060, - CreationTime: 806040, - Smallest: base.DecodeInternalKey([]byte("A\x00\x01\x02\x03\x04\x05\x06\x07")), - Largest: base.DecodeInternalKey([]byte("Z\x01\xff\xfe\xfd\xfc\xfb\xfa\xf9")), - SmallestSeqNum: 3, - LargestSeqNum: 5, - markedForCompaction: true, - }, + Meta: m2, }, }, }, @@ -95,6 +106,19 @@ func TestVersionEditRoundTrip(t *testing.T) { } func TestVersionEditDecode(t *testing.T) { + cmp := base.DefaultComparer.Compare + m := &FileMetadata{ + FileNum: 4, + Size: 986, + SmallestSeqNum: 3, + LargestSeqNum: 5, + } + m.MaybeExtendPointKeyBounds( + cmp, + base.MakeInternalKey([]byte("bar"), 5, base.InternalKeyKindDelete), + base.MakeInternalKey([]byte("foo"), 4, base.InternalKeyKindSet), + ) + testCases := []struct { filename string encodedEdits []string @@ -135,14 +159,7 @@ func TestVersionEditDecode(t *testing.T) { NewFiles: []NewFileEntry{ { Level: 0, - Meta: &FileMetadata{ - FileNum: 4, - Size: 986, - Smallest: base.MakeInternalKey([]byte("bar"), 5, base.InternalKeyKindDelete), - Largest: base.MakeInternalKey([]byte("foo"), 4, base.InternalKeyKindSet), - SmallestSeqNum: 3, - LargestSeqNum: 5, - }, + Meta: m, }, }, }, @@ -255,6 +272,7 @@ func TestVersionEditEncodeLastSeqNum(t *testing.T) { } func TestVersionEditApply(t *testing.T) { + cmp := base.DefaultComparer.Compare parseMeta := func(s string) (*FileMetadata, error) { parts := strings.Split(s, ":") if len(parts) != 2 { @@ -265,10 +283,12 @@ func TestVersionEditApply(t *testing.T) { return nil, err } parts = strings.Split(strings.TrimSpace(parts[1]), "-") - m := FileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := FileMetadata{} + m.MaybeExtendPointKeyBounds( + cmp, + 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 { diff --git a/internal/manifest/version_test.go b/internal/manifest/version_test.go index 09a65af712..71ada2eb33 100644 --- a/internal/manifest/version_test.go +++ b/internal/manifest/version_test.go @@ -29,6 +29,7 @@ func ikey(s string) InternalKey { } func TestIkeyRange(t *testing.T) { + cmp := base.DefaultComparer.Compare testCases := []struct { input, want string }{ @@ -73,11 +74,9 @@ func TestIkeyRange(t *testing.T) { var f []*FileMetadata if tc.input != "" { for i, s := range strings.Split(tc.input, " ") { - f = append(f, &FileMetadata{ - FileNum: base.FileNum(i), - Smallest: ikey(s[0:1]), - Largest: ikey(s[2:3]), - }) + m := &FileMetadata{FileNum: base.FileNum(i)} + m.MaybeExtendPointKeyBounds(cmp, ikey(s[0:1]), ikey(s[2:3])) + f = append(f, m) } } levelMetadata := makeLevelMetadata(base.DefaultComparer.Compare, 0, f) @@ -124,85 +123,94 @@ func TestOverlaps(t *testing.T) { } func TestContains(t *testing.T) { - m00 := &FileMetadata{ - FileNum: 700, - Size: 1, - Smallest: base.ParseInternalKey("b.SET.7008"), - Largest: base.ParseInternalKey("e.SET.7009"), - } - m01 := &FileMetadata{ - FileNum: 701, - Size: 1, - Smallest: base.ParseInternalKey("c.SET.7018"), - Largest: base.ParseInternalKey("f.SET.7019"), - } - m02 := &FileMetadata{ - FileNum: 702, - Size: 1, - Smallest: base.ParseInternalKey("f.SET.7028"), - Largest: base.ParseInternalKey("g.SET.7029"), - } - m03 := &FileMetadata{ - FileNum: 703, - Size: 1, - Smallest: base.ParseInternalKey("x.SET.7038"), - Largest: base.ParseInternalKey("y.SET.7039"), - } - m04 := &FileMetadata{ - FileNum: 704, - Size: 1, - Smallest: base.ParseInternalKey("n.SET.7048"), - Largest: base.ParseInternalKey("p.SET.7049"), - } - m05 := &FileMetadata{ - FileNum: 705, - Size: 1, - Smallest: base.ParseInternalKey("p.SET.7058"), - Largest: base.ParseInternalKey("p.SET.7059"), - } - m06 := &FileMetadata{ - FileNum: 706, - Size: 1, - Smallest: base.ParseInternalKey("p.SET.7068"), - Largest: base.ParseInternalKey("u.SET.7069"), - } - m07 := &FileMetadata{ - FileNum: 707, - Size: 1, - Smallest: base.ParseInternalKey("r.SET.7078"), - Largest: base.ParseInternalKey("s.SET.7079"), + cmp := base.DefaultComparer.Compare + newFileMeta := func(fileNum base.FileNum, size uint64, smallest, largest base.InternalKey) *FileMetadata { + m := &FileMetadata{ + FileNum: fileNum, + Size: size, + } + m.MaybeExtendPointKeyBounds(cmp, smallest, largest) + return m } + m00 := newFileMeta( + 700, + 1, + base.ParseInternalKey("b.SET.7008"), + base.ParseInternalKey("e.SET.7009"), + ) + m01 := newFileMeta( + 701, + 1, + base.ParseInternalKey("c.SET.7018"), + base.ParseInternalKey("f.SET.7019"), + ) + m02 := newFileMeta( + 702, + 1, + base.ParseInternalKey("f.SET.7028"), + base.ParseInternalKey("g.SET.7029"), + ) + m03 := newFileMeta( + 703, + 1, + base.ParseInternalKey("x.SET.7038"), + base.ParseInternalKey("y.SET.7039"), + ) + m04 := newFileMeta( + 704, + 1, + base.ParseInternalKey("n.SET.7048"), + base.ParseInternalKey("p.SET.7049"), + ) + m05 := newFileMeta( + 705, + 1, + base.ParseInternalKey("p.SET.7058"), + base.ParseInternalKey("p.SET.7059"), + ) + m06 := newFileMeta( + 706, + 1, + base.ParseInternalKey("p.SET.7068"), + base.ParseInternalKey("u.SET.7069"), + ) + m07 := newFileMeta( + 707, + 1, + base.ParseInternalKey("r.SET.7078"), + base.ParseInternalKey("s.SET.7079"), + ) - m10 := &FileMetadata{ - FileNum: 710, - Size: 1, - Smallest: base.ParseInternalKey("d.SET.7108"), - Largest: base.ParseInternalKey("g.SET.7109"), - } - m11 := &FileMetadata{ - FileNum: 711, - Size: 1, - Smallest: base.ParseInternalKey("g.SET.7118"), - Largest: base.ParseInternalKey("j.SET.7119"), - } - m12 := &FileMetadata{ - FileNum: 712, - Size: 1, - Smallest: base.ParseInternalKey("n.SET.7128"), - Largest: base.ParseInternalKey("p.SET.7129"), - } - m13 := &FileMetadata{ - FileNum: 713, - Size: 1, - Smallest: base.ParseInternalKey("p.SET.7148"), - Largest: base.ParseInternalKey("p.SET.7149"), - } - m14 := &FileMetadata{ - FileNum: 714, - Size: 1, - Smallest: base.ParseInternalKey("p.SET.7138"), - Largest: base.ParseInternalKey("u.SET.7139"), - } + m10 := newFileMeta( + 710, + 1, + base.ParseInternalKey("d.SET.7108"), + base.ParseInternalKey("g.SET.7109"), + ) + m11 := newFileMeta( + 711, + 1, + base.ParseInternalKey("g.SET.7118"), + base.ParseInternalKey("j.SET.7119"), + ) + m12 := newFileMeta( + 712, + 1, + base.ParseInternalKey("n.SET.7128"), + base.ParseInternalKey("p.SET.7129"), + ) + m13 := newFileMeta( + 713, + 1, + base.ParseInternalKey("p.SET.7148"), + base.ParseInternalKey("p.SET.7149"), + ) + m14 := newFileMeta( + 714, + 1, + base.ParseInternalKey("p.SET.7138"), + base.ParseInternalKey("u.SET.7139"), + ) v := Version{ Levels: [NumLevels]LevelMetadata{ @@ -255,7 +263,6 @@ func TestContains(t *testing.T) { {2, m14, false}, } - cmp := base.DefaultComparer.Compare for _, tc := range testCases { got := v.Contains(tc.level, cmp, tc.file) if got != tc.want { @@ -284,10 +291,12 @@ func TestCheckOrdering(t *testing.T) { if len(parts) != 2 { t.Fatalf("malformed table spec: %s", s) } - m := FileMetadata{ - Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])), - Largest: base.ParseInternalKey(strings.TrimSpace(parts[1])), - } + m := FileMetadata{} + m.MaybeExtendPointKeyBounds( + cmp, + base.ParseInternalKey(strings.TrimSpace(parts[0])), + base.ParseInternalKey(strings.TrimSpace(parts[1])), + ) m.SmallestSeqNum = m.Smallest.SeqNum() m.LargestSeqNum = m.Largest.SeqNum() return m @@ -437,3 +446,56 @@ func TestCheckConsistency(t *testing.T) { } }) } + +func TestExtendBounds(t *testing.T) { + cmp := base.DefaultComparer.Compare + parseBounds := func(line string) (lower, upper InternalKey) { + parts := strings.Split(line, "-") + if len(parts) == 1 { + parts = strings.Split(parts[0], ":") + start, end := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + lower = base.ParseInternalKey(start) + switch k := lower.Kind(); k { + case base.InternalKeyKindRangeDelete: + upper = base.MakeRangeDeleteSentinelKey([]byte(end)) + case base.InternalKeyKindRangeKeySet, base.InternalKeyKindRangeKeyUnset, base.InternalKeyKindRangeKeyDelete: + upper = base.MakeRangeKeySentinelKey(k, []byte(end)) + default: + panic(fmt.Sprintf("unknown kind %s with end key", k)) + } + } else { + l, u := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) + lower, upper = base.ParseInternalKey(l), base.ParseInternalKey(u) + } + return + } + format := func(m *FileMetadata) string { + var b bytes.Buffer + fmt.Fprintf(&b, "combined: [%s-%s]\n", m.Smallest, m.Largest) + if m.HasPointKeys { + fmt.Fprintf(&b, " points: [%s-%s]\n", m.SmallestPointKey, m.LargestPointKey) + } + if m.HasRangeKeys { + fmt.Fprintf(&b, " ranges: [%s-%s]\n", m.SmallestRangeKey, m.LargestRangeKey) + } + return b.String() + } + m := &FileMetadata{} + datadriven.RunTest(t, "testdata/file_metadata_bounds", func(d *datadriven.TestData) string { + switch d.Cmd { + case "reset": + m = &FileMetadata{} + return "" + case "extend-point-key-bounds": + u, l := parseBounds(d.Input) + m.MaybeExtendPointKeyBounds(cmp, u, l) + return format(m) + case "extend-range-key-bounds": + u, l := parseBounds(d.Input) + m.MaybeExtendRangeKeyBounds(cmp, u, l) + return format(m) + default: + return fmt.Sprintf("unknown command %s\n", d.Cmd) + } + }) +} diff --git a/level_checker_test.go b/level_checker_test.go index 5557de2e50..e8f27873d5 100644 --- a/level_checker_test.go +++ b/level_checker_test.go @@ -135,11 +135,9 @@ func TestCheckLevelsCornerCases(t *testing.T) { keys := strings.Fields(line) smallestKey := base.ParseInternalKey(keys[0]) largestKey := base.ParseInternalKey(keys[1]) - *li = append(*li, &fileMetadata{ - FileNum: fileNum, - Smallest: smallestKey, - Largest: largestKey, - }) + m := &fileMetadata{FileNum: fileNum} + m.MaybeExtendRangeKeyBounds(cmp, smallestKey, largestKey) + *li = append(*li, m) i++ line = lines[i] diff --git a/level_iter_test.go b/level_iter_test.go index 8280227cf3..6554ed90c9 100644 --- a/level_iter_test.go +++ b/level_iter_test.go @@ -53,11 +53,12 @@ func TestLevelIter(t *testing.T) { } iters = append(iters, f) - meta := &fileMetadata{ - FileNum: FileNum(len(metas)), - } - meta.Smallest = f.keys[0] - meta.Largest = f.keys[len(f.keys)-1] + meta := &fileMetadata{FileNum: FileNum(len(metas))} + meta.MaybeExtendRangeKeyBounds( + DefaultComparer.Compare, + f.keys[0], + f.keys[len(f.keys)-1], + ) metas = append(metas, meta) } files = manifest.NewLevelSliceKeySorted(base.DefaultComparer.Compare, metas) @@ -236,11 +237,15 @@ func (lt *levelIterTest) runBuild(d *datadriven.TestData) string { return err.Error() } lt.readers = append(lt.readers, r) - lt.metas = append(lt.metas, &fileMetadata{ - FileNum: fileNum, - Smallest: meta.SmallestPointKey(lt.cmp.Compare), - Largest: meta.LargestPointKey(lt.cmp.Compare), - }) + m := &fileMetadata{FileNum: fileNum} + if meta.HasPointKeys { + m.MaybeExtendPointKeyBounds(lt.cmp.Compare, meta.SmallestPoint, meta.LargestPoint) + } + if meta.HasRangeDelKeys { + m.MaybeExtendPointKeyBounds(lt.cmp.Compare, meta.SmallestRangeDel, meta.LargestRangeDel) + + } + lt.metas = append(lt.metas, m) var buf bytes.Buffer for _, f := range lt.metas { @@ -455,12 +460,11 @@ func buildLevelIterTables( for i := range readers { iter, err := readers[i].NewIter(nil /* lower */, nil /* upper */) require.NoError(b, err) - key, _ := iter.First() + smallest, _ := iter.First() meta[i] = &fileMetadata{} meta[i].FileNum = FileNum(i) - meta[i].Smallest = *key - key, _ = iter.Last() - meta[i].Largest = *key + largest, _ := iter.Last() + meta[i].MaybeExtendPointKeyBounds(opts.Comparer.Compare, (*smallest).Clone(), (*largest).Clone()) } slice := manifest.NewLevelSliceKeySorted(base.DefaultComparer.Compare, meta) return readers, slice, keys, cleanup diff --git a/merging_iter_test.go b/merging_iter_test.go index 5af8f1d90d..17a79ba070 100644 --- a/merging_iter_test.go +++ b/merging_iter_test.go @@ -178,11 +178,9 @@ func TestMergingIterCornerCases(t *testing.T) { keys := strings.Fields(line) smallestKey := base.ParseInternalKey(keys[0]) largestKey := base.ParseInternalKey(keys[1]) - files[level] = append(files[level], &fileMetadata{ - FileNum: fileNum, - Smallest: smallestKey, - Largest: largestKey, - }) + m := &fileMetadata{FileNum: fileNum} + m.MaybeExtendPointKeyBounds(cmp, smallestKey, largestKey) + files[level] = append(files[level], m) i++ line = lines[i] @@ -547,15 +545,14 @@ func buildLevelsForMergingIterSeqSeek( for j := range readers[i] { iter, err := readers[i][j].NewIter(nil /* lower */, nil /* upper */) require.NoError(b, err) - key, _ := iter.First() + smallest, _ := iter.First() meta[j] = &fileMetadata{} // The same FileNum is being reused across different levels, which // is harmless for the benchmark since each level has its own iterator // creation func. meta[j].FileNum = FileNum(j) - meta[j].Smallest = key.Clone() - key, _ = iter.Last() - meta[j].Largest = key.Clone() + largest, _ := iter.Last() + meta[j].MaybeExtendPointKeyBounds(opts.Comparer.Compare, smallest.Clone(), largest.Clone()) } levelSlices[i] = manifest.NewLevelSliceSpecificOrder(meta) } diff --git a/sstable/suffix_rewriter.go b/sstable/suffix_rewriter.go index 62d0df556b..bdd9cf3476 100644 --- a/sstable/suffix_rewriter.go +++ b/sstable/suffix_rewriter.go @@ -302,8 +302,12 @@ func rewriteDataBlocksToWriter( w.props.NumEntries = r.Properties.NumEntries w.props.RawKeySize = r.Properties.RawKeySize w.props.RawValueSize = r.Properties.RawValueSize - w.meta.SmallestPoint = blocks[0].start - w.meta.LargestPoint = blocks[len(blocks)-1].end + // NB: we set the smallest / largest fields directly here, rather than via the + // Set* methods, as we know we only have point keys. + smallest, largest := blocks[0].start, blocks[len(blocks)-1].end + w.meta.SmallestPoint, w.meta.Smallest = smallest, smallest + w.meta.LargestPoint, w.meta.Largest = largest, largest + w.meta.HasPointKeys = true return nil } diff --git a/sstable/writer.go b/sstable/writer.go index 95fa6e2788..e234ca15e7 100644 --- a/sstable/writer.go +++ b/sstable/writer.go @@ -31,20 +31,103 @@ const encodedBHPEstimatedSize = binary.MaxVarintLen64 * 2 var errWriterClosed = errors.New("pebble: writer is closed") // WriterMetadata holds info about a finished sstable. -// TODO(travers): A table may not have point or range keys, in which case it -// will have zero-valued smallest and largest keys. Introduce a sentinel key to -// indicate "absence" of the bound. type WriterMetadata struct { Size uint64 SmallestPoint InternalKey - SmallestRangeDel InternalKey - SmallestRangeKey InternalKey LargestPoint InternalKey + SmallestRangeDel InternalKey LargestRangeDel InternalKey + SmallestRangeKey InternalKey LargestRangeKey InternalKey - SmallestSeqNum uint64 - LargestSeqNum uint64 - Properties Properties + HasPointKeys bool + HasRangeDelKeys bool + HasRangeKeys bool + // NB: Smallest and Largest are the overall bounds for the writer and are set + // indirectly via calls to SetSmallest* and SetLargest*. These fields should + // be treated as read-only. + Smallest InternalKey + Largest InternalKey + SmallestSeqNum uint64 + LargestSeqNum uint64 + Properties Properties +} + +// SetSmallestPointKey sets the smallest point key to the given key. This may +// also update the overall lower bound in the metadata. +// NB: this method set the "absolute" smallest point key. Any existing key is +// overridden. +func (m *WriterMetadata) SetSmallestPointKey(cmp Compare, k InternalKey) { + m.SmallestPoint = k + m.maybeUpdateSmallest(cmp, k) + m.HasPointKeys = true +} + +// SetSmallestRangeDelKey sets the smallest rangedel key to the given key. This +// may also update the overall lower bound in the metadata. +// NB: this method set the "absolute" smallest rangedel key. Any existing key is +// overridden. +func (m *WriterMetadata) SetSmallestRangeDelKey(cmp Compare, k InternalKey) { + m.SmallestRangeDel = k + m.maybeUpdateSmallest(cmp, k) + m.HasRangeDelKeys = true +} + +// SetSmallestRangeKey sets the smallest range key to the given key. This may +// also update the overall lower bound in the metadata. +// NB: this method set the "absolute" smallest range key. Any existing key is +// overridden. +func (m *WriterMetadata) SetSmallestRangeKey(cmp Compare, k InternalKey) { + m.SmallestRangeKey = k + m.maybeUpdateSmallest(cmp, k) + m.HasRangeKeys = true +} + +// SetLargestPointKey sets the largest point key to the given key. This may also +// update the overall upper bound in the metadata. +// NB: this method set the "absolute" largest point key. Any existing key is +// overridden. +func (m *WriterMetadata) SetLargestPointKey(cmp Compare, k InternalKey) { + m.LargestPoint = k + m.maybeUpdateLargest(cmp, k) + m.HasPointKeys = true +} + +// SetLargestRangeDelKey sets the largest rangedel key to the given key. This +// may also update the overall upper bound in the metadata. +// NB: this method set the "absolute" largest rangedel key. Any existing key is +// overridden. +func (m *WriterMetadata) SetLargestRangeDelKey(cmp Compare, k InternalKey) { + m.LargestRangeDel = k + m.maybeUpdateLargest(cmp, k) + m.HasRangeDelKeys = true +} + +// SetLargestRangeKey sets the largest range key to the given key. This may also +// update the overall lower bound in the metadata. +// NB: this method set the "absolute" largest range key. Any existing key is +// overridden. +func (m *WriterMetadata) SetLargestRangeKey(cmp Compare, k InternalKey) { + m.LargestRangeKey = k + m.maybeUpdateLargest(cmp, k) + m.HasRangeKeys = true +} + +// maybeUpdateSmallest updates the smallest key if no smallest key has been set +// or if the given key is smaller than the existing smallest key. +func (m *WriterMetadata) maybeUpdateSmallest(cmp Compare, k InternalKey) { + if !(m.HasPointKeys || m.HasRangeDelKeys || m.HasRangeKeys) || + base.InternalCompare(cmp, k, m.Smallest) < 0 { + m.Smallest = k + } +} + +// maybeUpdateLargest updates the largest key if no largest key has been set or +// if the given key is smaller than the existing smallest key. +func (m *WriterMetadata) maybeUpdateLargest(cmp Compare, k InternalKey) { + if !(m.HasPointKeys || m.HasRangeDelKeys || m.HasRangeKeys) || + base.InternalCompare(cmp, k, m.Largest) > 0 { + m.Largest = k + } } func (m *WriterMetadata) updateSeqNum(seqNum uint64) { @@ -56,64 +139,6 @@ func (m *WriterMetadata) updateSeqNum(seqNum uint64) { } } -// SmallestPointKey returns the smaller of SmallestPoint and SmallestRangeDel. -func (m *WriterMetadata) SmallestPointKey(cmp Compare) InternalKey { - if m.SmallestPoint.UserKey == nil { - return m.SmallestRangeDel - } - if m.SmallestRangeDel.UserKey == nil { - return m.SmallestPoint - } - if base.InternalCompare(cmp, m.SmallestPoint, m.SmallestRangeDel) < 0 { - return m.SmallestPoint - } - return m.SmallestRangeDel -} - -// LargestPointKey returns the larger of LargestPoint and LargestRangeDel. -func (m *WriterMetadata) LargestPointKey(cmp Compare) InternalKey { - if m.LargestPoint.UserKey == nil { - return m.LargestRangeDel - } - if m.LargestRangeDel.UserKey == nil { - return m.LargestPoint - } - if base.InternalCompare(cmp, m.LargestPoint, m.LargestRangeDel) > 0 { - return m.LargestPoint - } - return m.LargestRangeDel -} - -// Smallest returns the smaller of SmallestPointKey and SmallestRangeKey. -func (m *WriterMetadata) Smallest(cmp Compare) InternalKey { - point := m.SmallestPointKey(cmp) - if point.UserKey == nil { - return m.SmallestRangeKey - } - if m.SmallestRangeKey.UserKey == nil { - return point - } - if base.InternalCompare(cmp, point, m.SmallestRangeKey) < 0 { - return point - } - return m.SmallestRangeKey -} - -// Largest returns the larger of LargestPointKey and LargestRangeKey. -func (m *WriterMetadata) Largest(cmp Compare) InternalKey { - point := m.LargestPointKey(cmp) - if point.UserKey == nil { - return m.LargestRangeKey - } - if m.LargestRangeKey.UserKey == nil { - return point - } - if base.InternalCompare(cmp, point, m.LargestRangeKey) > 0 { - return point - } - return m.LargestRangeKey -} - type flusher interface { Flush() error } @@ -623,9 +648,12 @@ func (w *Writer) addPoint(key InternalKey, value []byte) error { w.dataBlockBuf.dataBlock.add(key, value) w.meta.updateSeqNum(key.SeqNum()) - // block.curKey contains the most recently added key to the block. - w.meta.LargestPoint.UserKey = w.dataBlockBuf.dataBlock.curKey[:len(w.dataBlockBuf.dataBlock.curKey)-8] - w.meta.LargestPoint.Trailer = key.Trailer + k := base.InternalKey{ + // block.curKey contains the most recently added key to the block. + UserKey: w.dataBlockBuf.dataBlock.curKey[:len(w.dataBlockBuf.dataBlock.curKey)-8], + Trailer: key.Trailer, + } + w.meta.SetLargestPointKey(w.compare, k) if w.meta.SmallestPoint.UserKey == nil { // NB: we clone w.meta.LargestPoint rather than "key", even though they are // semantically identical, because we need to ensure that SmallestPoint.UserKey @@ -636,7 +664,7 @@ func (w *Writer) addPoint(key InternalKey, value []byte) error { // then hang on to, and would otherwise continue to alias that whole pool's // slice if we did so. So since we'll need to allocate it its own slice at // some point anyway, we may as well do so here. - w.meta.SmallestPoint = w.meta.LargestPoint.Clone() + w.meta.SetSmallestPointKey(w.compare, w.meta.LargestPoint.Clone()) } w.props.NumEntries++ @@ -707,15 +735,15 @@ func (w *Writer) addTombstone(key InternalKey, value []byte) error { // // Note that writing the v1 format is only supported for tests. if w.props.NumRangeDeletions == 0 { - w.meta.SmallestRangeDel = key.Clone() - w.meta.LargestRangeDel = base.MakeRangeDeleteSentinelKey(value).Clone() + w.meta.SetSmallestRangeDelKey(w.compare, key.Clone()) + w.meta.SetLargestRangeDelKey(w.compare, base.MakeRangeDeleteSentinelKey(value).Clone()) } else { if base.InternalCompare(w.compare, w.meta.SmallestRangeDel, key) > 0 { - w.meta.SmallestRangeDel = key.Clone() + w.meta.SetSmallestRangeDelKey(w.compare, key.Clone()) } end := base.MakeRangeDeleteSentinelKey(value) if base.InternalCompare(w.compare, w.meta.LargestRangeDel, end) < 0 { - w.meta.LargestRangeDel = end.Clone() + w.meta.SetLargestRangeDelKey(w.compare, end.Clone()) } } @@ -725,7 +753,7 @@ func (w *Writer) addTombstone(key InternalKey, value []byte) error { // range tombstone key. The largest range tombstone key will be determined // in Writer.Close() as the end key of the last range tombstone added. if w.props.NumRangeDeletions == 0 { - w.meta.SmallestRangeDel = key.Clone() + w.meta.SetSmallestRangeDelKey(w.compare, key.Clone()) } } @@ -950,7 +978,7 @@ func (w *Writer) addRangeKey(key InternalKey, value []byte) error { // added will be the smallest. The largest range key is determined in // Writer.Close() as the end key of the last range key added to the block. if w.props.NumRangeKeys() == 0 { - w.meta.SmallestRangeKey = key.Clone() + w.meta.SetSmallestRangeKey(w.compare, key.Clone()) } // Update block properties. @@ -1447,7 +1475,8 @@ func (w *Writer) Close() (err error) { // slice passed into Write(). Also, w.meta will often outlive the // blockWriter, and so cloning curValue allows the rangeDelBlock's // internal buffer to get gc'd. - w.meta.LargestRangeDel = base.MakeRangeDeleteSentinelKey(w.rangeDelBlock.curValue).Clone() + k := base.MakeRangeDeleteSentinelKey(w.rangeDelBlock.curValue).Clone() + w.meta.SetLargestRangeDelKey(w.compare, k) } rangeDelBH, err = w.writeBlock(w.rangeDelBlock.finish(), NoCompression, &w.blockBuf) if err != nil { @@ -1469,7 +1498,8 @@ func (w *Writer) Close() (err error) { w.err = errors.Newf("invalid end key: %s", w.rangeKeyBlock.curValue) return w.err } - w.meta.LargestRangeKey = base.MakeRangeKeySentinelKey(kind, endKey).Clone() + k := base.MakeRangeKeySentinelKey(kind, endKey).Clone() + w.meta.SetLargestRangeKey(w.compare, k) // TODO(travers): The lack of compression on the range key block matches the // lack of compression on the range-del block. Revisit whether we want to // enable compression on this block.