Skip to content

Commit

Permalink
scan_internal: Sort keys by kind before calling visitor
Browse files Browse the repository at this point in the history
Previously, we were passing range keys that were sorted by trailer
descending right from ScanInternal. However, the caller isn't
interested in seqnum-based sorting, only kind-based trailer sorting.
This change zeroes seqnums before sorting the keys.

Unblocks cockroachdb#3044.
  • Loading branch information
itsbilal committed Nov 28, 2023
1 parent e5b54da commit 0f3ea04
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 40 deletions.
12 changes: 11 additions & 1 deletion scan_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,17 @@ func scanInternalImpl(
case InternalKeyKindRangeKeyDelete, InternalKeyKindRangeKeyUnset, InternalKeyKindRangeKeySet:
if opts.visitRangeKey != nil {
span := iter.unsafeSpan()
if err := opts.visitRangeKey(span.Start, span.End, span.Keys); err != nil {
// NB: The caller isn't interested in the sequence numbers of these
// range keys. Rather, the caller wants them to be in trailer order
// _after_ zeroing of sequence numbers. Copy span.Keys, sort it, and then
// call visitRangeKey.
keysCopy := make([]keyspan.Key, len(span.Keys))
for i := range span.Keys {
keysCopy[i] = span.Keys[i]
keysCopy[i].Trailer = base.MakeTrailer(0, span.Keys[i].Kind())
}
keyspan.SortKeysByTrailer(&keysCopy)
if err := opts.visitRangeKey(span.Start, span.End, keysCopy); err != nil {
return err
}
}
Expand Down
78 changes: 39 additions & 39 deletions testdata/scan_internal
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ ok

scan-internal file-only-snapshot=efos1
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b#12,1 (d)
c-e:{(#11,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}
e#13,1 (foo)

flush
----

scan-internal
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b#12,1 (d)
c-e:{(#11,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}
e#13,1 (foo)

# Keys deleted by rangedels are elided.
Expand All @@ -50,37 +50,37 @@ committed 1 keys

scan-internal
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b-d#14,RANGEDEL
c-e:{(#11,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}
e#13,1 (foo)

flush
----

scan-internal
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b-d#14,RANGEDEL
c-e:{(#11,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}
e#13,1 (foo)

# Snapshots work with scan internal.

scan-internal snapshot=foo
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
c-e:{(#11,RANGEKEYSET,@5,beep)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
c-e:{(#0,RANGEKEYSET,@5,beep)}

wait-for-file-only-snapshot efos1
----
ok

scan-internal file-only-snapshot=efos1
----
a-c:{(#10,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b#12,1 (d)
c-e:{(#11,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}
e#13,1 (foo)

# Force keys newer than the snapshot into a lower level, then try skip-shared
Expand All @@ -105,15 +105,15 @@ file 000008 contains keys newer than snapshot: pebble: cannot use skip-shared it

scan-internal lower=bb upper=dd
----
bb-c:{(#10,RANGEKEYSET,@5,boop)}
bb-c:{(#0,RANGEKEYSET,@5,boop)}
bb-d#14,RANGEDEL
c-dd:{(#11,RANGEKEYSET,@5,beep)}
c-dd:{(#0,RANGEKEYSET,@5,beep)}

scan-internal lower=b upper=cc
----
b-c:{(#10,RANGEKEYSET,@5,boop)}
b-c:{(#0,RANGEKEYSET,@5,boop)}
b-cc#14,RANGEDEL
c-cc:{(#11,RANGEKEYSET,@5,beep)}
c-cc:{(#0,RANGEKEYSET,@5,beep)}

reset
----
Expand Down Expand Up @@ -150,10 +150,10 @@ committed 1 keys

scan-internal
----
a-b:{(#13,RANGEKEYDEL)}
b-c:{(#12,RANGEKEYUNSET,@6) (#10,RANGEKEYSET,@8)}
c-d:{(#12,RANGEKEYUNSET,@6)}
d-e:{(#11,RANGEKEYSET,@6)}
a-b:{(#0,RANGEKEYDEL)}
b-c:{(#0,RANGEKEYSET,@8) (#0,RANGEKEYUNSET,@6)}
c-d:{(#0,RANGEKEYUNSET,@6)}
d-e:{(#0,RANGEKEYSET,@6)}

flush
----
Expand All @@ -168,10 +168,10 @@ lsm

scan-internal
----
a-b:{(#13,RANGEKEYDEL)}
b-c:{(#12,RANGEKEYUNSET,@6) (#10,RANGEKEYSET,@8)}
c-d:{(#12,RANGEKEYUNSET,@6)}
d-e:{(#11,RANGEKEYSET,@6)}
a-b:{(#0,RANGEKEYDEL)}
b-c:{(#0,RANGEKEYSET,@8) (#0,RANGEKEYUNSET,@6)}
c-d:{(#0,RANGEKEYUNSET,@6)}
d-e:{(#0,RANGEKEYSET,@6)}

batch ingest
range-key-set e f @3
Expand All @@ -181,12 +181,12 @@ wrote 2 keys to batch ""

scan-internal
----
a-b:{(#13,RANGEKEYDEL)}
b-c:{(#12,RANGEKEYUNSET,@6) (#10,RANGEKEYSET,@8)}
c-d:{(#12,RANGEKEYUNSET,@6)}
d-e:{(#11,RANGEKEYSET,@6)}
e-f:{(#14,RANGEKEYSET,@3)}
f-g:{(#14,RANGEKEYUNSET,@3)}
a-b:{(#0,RANGEKEYDEL)}
b-c:{(#0,RANGEKEYSET,@8) (#0,RANGEKEYUNSET,@6)}
c-d:{(#0,RANGEKEYUNSET,@6)}
d-e:{(#0,RANGEKEYSET,@6)}
e-f:{(#0,RANGEKEYSET,@3)}
f-g:{(#0,RANGEKEYUNSET,@3)}

batch ingest
range-key-unset e f @3
Expand All @@ -196,12 +196,12 @@ wrote 2 keys to batch ""

scan-internal
----
a-b:{(#13,RANGEKEYDEL)}
b-c:{(#12,RANGEKEYUNSET,@6) (#10,RANGEKEYSET,@8)}
c-d:{(#12,RANGEKEYUNSET,@6)}
d-e:{(#11,RANGEKEYSET,@6)}
e-f:{(#15,RANGEKEYUNSET,@3)}
f-g:{(#15,RANGEKEYSET,@3)}
a-b:{(#0,RANGEKEYDEL)}
b-c:{(#0,RANGEKEYSET,@8) (#0,RANGEKEYUNSET,@6)}
c-d:{(#0,RANGEKEYUNSET,@6)}
d-e:{(#0,RANGEKEYSET,@6)}
e-f:{(#0,RANGEKEYUNSET,@3)}
f-g:{(#0,RANGEKEYSET,@3)}

# Range key masking is not exercised, with range keys that could mask point
# keys being returned alongside point keys.
Expand All @@ -222,9 +222,9 @@ committed 2 keys

scan-internal
----
a-c:{(#11,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b@3#10,1 (bar)
c-e:{(#12,RANGEKEYSET,@5,beep)}
c-e:{(#0,RANGEKEYSET,@5,beep)}

# Point keys are collapsed in a way similar to a compaction.

Expand Down Expand Up @@ -313,7 +313,7 @@ lsm

scan-internal
----
a-c:{(#11,RANGEKEYSET,@5,boop)}
a-c:{(#0,RANGEKEYSET,@5,boop)}
b@3#10,1 (bar)
c-e#12,RANGEDEL
f@8#13,1 (baz)
Expand Down

0 comments on commit 0f3ea04

Please sign in to comment.