From b77606f72b19c06e402e7f22de6d4b7737eeb39a Mon Sep 17 00:00:00 2001 From: Bilal Akhtar Date: Wed, 21 Jun 2023 12:57:02 -0400 Subject: [PATCH] db: Zero out bounds for key type not present in ScanInternal 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. --- scan_internal.go | 22 ++++++++++++++++ scan_internal_test.go | 2 +- testdata/scan_internal | 59 ++++++++++++++++++++++++++++++++---------- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/scan_internal.go b/scan_internal.go index b9fd57b987..b027f1e659 100644 --- a/scan_internal.go +++ b/scan_internal.go @@ -770,6 +770,7 @@ 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) } @@ -777,14 +778,24 @@ func (d *DB) truncateSharedFile( 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{} } } } @@ -794,6 +805,7 @@ 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) } @@ -801,14 +813,24 @@ func (d *DB) truncateSharedFile( 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{} } } } diff --git a/scan_internal_test.go b/scan_internal_test.go index 74a403e777..367d9c7044 100644 --- a/scan_internal_test.go +++ b/scan_internal_test.go @@ -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 } } diff --git a/testdata/scan_internal b/testdata/scan_internal index d98f744279..b4a6e04b61 100644 --- a/testdata/scan_internal +++ b/testdata/scan_internal @@ -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 ---- @@ -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 @@ -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]