Skip to content

Commit

Permalink
db: Zero out bounds for key type not present in ScanInternal
Browse files Browse the repository at this point in the history
Previously, when we were exporting shared files through
truncateSharedFile, and we found no point keys in the relevant
span but found range keys (or vice versa), we would leave some of
the bounds of the not-found key type set. This could trip up
logic elsewhere that relied on null key bounds for keys not
present.
  • Loading branch information
itsbilal committed Jun 22, 2023
1 parent 73b9c4d commit b77606f
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 14 deletions.
22 changes: 22 additions & 0 deletions scan_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,21 +770,32 @@ func (d *DB) truncateSharedFile(
sst.SmallestPointKey.UserKey = sst.SmallestPointKey.UserKey[:0]
sst.SmallestPointKey.Trailer = 0
key, _ := iter.SeekGE(lower, base.SeekGEFlagsNone)
foundPointKey := key != nil
if key != nil {
sst.SmallestPointKey.CopyFrom(*key)
}
if rangeDelIter != nil {
span := rangeDelIter.SeekGE(lower)
if span != nil && (len(sst.SmallestPointKey.UserKey) == 0 || base.InternalCompare(cmp, span.SmallestKey(), sst.SmallestPointKey) < 0) {
sst.SmallestPointKey.CopyFrom(span.SmallestKey())
foundPointKey = true
}
}
if !foundPointKey {
// There are no point keys in the span we're interested in.
sst.SmallestPointKey = InternalKey{}
sst.LargestPointKey = InternalKey{}
}
sst.SmallestRangeKey.UserKey = sst.SmallestRangeKey.UserKey[:0]
sst.SmallestRangeKey.Trailer = 0
if rangeKeyIter != nil {
span := rangeKeyIter.SeekGE(lower)
if span != nil {
sst.SmallestRangeKey.CopyFrom(span.SmallestKey())
} else {
// There are no range keys in the span we're interested in.
sst.SmallestRangeKey = InternalKey{}
sst.LargestRangeKey = InternalKey{}
}
}
}
Expand All @@ -794,21 +805,32 @@ func (d *DB) truncateSharedFile(
sst.LargestPointKey.UserKey = sst.LargestPointKey.UserKey[:0]
sst.LargestPointKey.Trailer = 0
key, _ := iter.SeekLT(upper, base.SeekLTFlagsNone)
foundPointKey := key != nil
if key != nil {
sst.LargestPointKey.CopyFrom(*key)
}
if rangeDelIter != nil {
span := rangeDelIter.SeekLT(upper)
if span != nil && (len(sst.LargestPointKey.UserKey) == 0 || base.InternalCompare(cmp, span.LargestKey(), sst.LargestPointKey) > 0) {
sst.LargestPointKey.CopyFrom(span.LargestKey())
foundPointKey = true
}
}
if !foundPointKey {
// There are no point keys in the span we're interested in.
sst.SmallestPointKey = InternalKey{}
sst.LargestPointKey = InternalKey{}
}
sst.LargestRangeKey.UserKey = sst.LargestRangeKey.UserKey[:0]
sst.LargestRangeKey.Trailer = 0
if rangeKeyIter != nil {
span := rangeKeyIter.SeekLT(upper)
if span != nil {
sst.LargestRangeKey.CopyFrom(span.LargestKey())
} else {
// There are no range keys in the span we're interested in.
sst.SmallestRangeKey = InternalKey{}
sst.LargestRangeKey = InternalKey{}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion scan_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func TestScanInternal(t *testing.T) {
reader = snap
case "skip-shared":
fileVisitor = func(sst *SharedSSTMeta) error {
fmt.Fprintf(&b, "shared file: %s [%s-%s]\n", sst.fileNum, sst.Smallest.String(), sst.Largest.String())
fmt.Fprintf(&b, "shared file: %s [%s-%s] [point=%s-%s] [range=%s-%s]\n", sst.fileNum, sst.Smallest.String(), sst.Largest.String(), sst.SmallestPointKey.String(), sst.LargestPointKey.String(), sst.SmallestRangeKey.String(), sst.LargestRangeKey.String())
return nil
}
}
Expand Down
59 changes: 46 additions & 13 deletions testdata/scan_internal
Original file line number Diff line number Diff line change
Expand Up @@ -247,33 +247,33 @@ f@8#13,1 (baz)

scan-internal skip-shared lower=a upper=z
----
shared file: 000005 [a#11,21-e#72057594037927935,15]
shared file: 000005 [a#11,21-e#72057594037927935,15] [point=b@3#10,1-e#72057594037927935,15] [range=a#11,21-c#72057594037927935,21]
f@8#13,1 (baz)

scan-internal skip-shared lower=a upper=e
----
shared file: 000005 [a#11,21-e#72057594037927935,15]
shared file: 000005 [a#11,21-e#72057594037927935,15] [point=b@3#10,1-e#72057594037927935,15] [range=a#11,21-c#72057594037927935,21]

scan-internal skip-shared lower=a upper=d
----
shared file: 000005 [a#11,21-d#72057594037927935,15]
shared file: 000005 [a#11,21-d#72057594037927935,15] [point=b@3#10,1-d#72057594037927935,15] [range=a#11,21-c#72057594037927935,21]

scan-internal skip-shared lower=a upper=c
----
shared file: 000005 [a#11,21-c#72057594037927935,21]
shared file: 000005 [a#11,21-c#72057594037927935,21] [point=b@3#10,1-b@3#10,1] [range=a#11,21-c#72057594037927935,21]

scan-internal skip-shared lower=a upper=b
----
shared file: 000005 [a#11,21-b#72057594037927935,21]
shared file: 000005 [a#11,21-b#72057594037927935,21] [point=#0,0-#0,0] [range=a#11,21-b#72057594037927935,21]

scan-internal skip-shared lower=b upper=z
----
shared file: 000005 [b#11,21-e#72057594037927935,15]
shared file: 000005 [b#11,21-e#72057594037927935,15] [point=b@3#10,1-e#72057594037927935,15] [range=b#11,21-c#72057594037927935,21]
f@8#13,1 (baz)

scan-internal skip-shared lower=b upper=bb
----
shared file: 000005 [b#11,21-bb#72057594037927935,21]
shared file: 000005 [b#11,21-bb#72057594037927935,21] [point=b@3#10,1-b@3#10,1] [range=b#11,21-bb#72057594037927935,21]

scan-internal skip-shared lower=e upper=ff
----
Expand Down Expand Up @@ -308,24 +308,28 @@ lsm
scan-internal skip-shared lower=c upper=d
----

scan-internal skip-shared lower=a upper=d
----
shared file: 000005 [a#10,1-c#72057594037927935,15] [point=a#10,1-c#72057594037927935,15] [range=#0,0-#0,0]

scan-internal skip-shared lower=bb upper=d
----
shared file: 000005 [bb#12,15-c#72057594037927935,15]
shared file: 000005 [bb#12,15-c#72057594037927935,15] [point=bb#12,15-c#72057594037927935,15] [range=#0,0-#0,0]

scan-internal skip-shared lower=c upper=ea
----
shared file: 000005 [e#13,21-ea#72057594037927935,21]
shared file: 000005 [e#13,21-ea#72057594037927935,21] [point=#0,0-#0,0] [range=e#13,21-ea#72057594037927935,21]

scan-internal skip-shared lower=c upper=z
----
shared file: 000005 [e#13,21-f#11,1]
shared file: 000005 [e#13,21-f#11,1] [point=f#11,1-f#11,1] [range=e#13,21-ee#72057594037927935,21]

# An upper bound equalling a file's Largest user key should be reason enough to
# truncate that file's bounds.

scan-internal skip-shared lower=c upper=f
----
shared file: 000005 [e#13,21-ee#72057594037927935,21]
shared file: 000005 [e#13,21-ee#72057594037927935,21] [point=#0,0-#0,0] [range=e#13,21-ee#72057594037927935,21]

# Construct a file with an exclusive sentinel as the largest key. Verify that
# scan-internal with skip-shared and an upper bound at that exclusive sentinel
Expand All @@ -351,8 +355,37 @@ compact a-z

scan-internal skip-shared lower=a upper=ee
----
shared file: 000005 [a#10,1-ee#72057594037927935,21]
shared file: 000005 [a#10,1-ee#72057594037927935,21] [point=a#10,1-c#72057594037927935,15] [range=e#12,21-ee#72057594037927935,21]

scan-internal skip-shared lower=b upper=ee
----
shared file: 000005 [b#11,15-ee#72057594037927935,21]
shared file: 000005 [b#11,15-ee#72057594037927935,21] [point=b#11,15-c#72057594037927935,15] [range=e#12,21-ee#72057594037927935,21]

# Ensure we don't leave any range key bounds unintentionally set.

reset
----

batch commit
range-key-set a aa @5 boop
set b foo
set c bar
set d baz
----
committed 4 keys

flush
----

compact a-z
----
6:
000005:[a#10,RANGEKEYSET-d#13,SET]

scan-internal skip-shared lower=b upper=e
----
shared file: 000005 [b#11,1-d#13,1] [point=b#11,1-d#13,1] [range=#0,0-#0,0]

scan-internal skip-shared lower=a upper=aaa
----
shared file: 000005 [a#10,21-aa#72057594037927935,21] [point=#0,0-#0,0] [range=a#10,21-aa#72057594037927935,21]

0 comments on commit b77606f

Please sign in to comment.