Skip to content

Commit

Permalink
sstable: fix compaction iterator bug with prefix replacement
Browse files Browse the repository at this point in the history
The sstable compaction iterators stop when the key exceeds
`vState.upper`; however this is incorrect when we replace prefixes
(the key has the content prefix and `vState.upper` has the synthetic
prefix).

Switch to using `i.upper` which is in the "content prefix space" and
was already calculated at initialization.
  • Loading branch information
RaduBerinde committed Feb 22, 2024
1 parent 6053d8e commit 6ad6346
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 6 deletions.
5 changes: 5 additions & 0 deletions data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,11 @@ func runBuildRemoteCmd(td *datadriven.TestData, d *DB, storage remote.Storage) e
}

writeOpts := d.opts.MakeWriterOptions(0 /* level */, tableFormat)
if rand.Intn(4) == 0 {
// Force two-level indexes.
writeOpts.BlockSize = 5
writeOpts.IndexBlockSize = 5
}

f, err := storage.CreateObject(path)
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions sstable/reader_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,10 @@ func (i *compactionIterator) skipForward(
*i.bytesIterated += uint64(curOffset - i.prevOffset)
i.prevOffset = curOffset

if i.vState != nil && key != nil {
cmp := i.cmp(key.UserKey, i.vState.upper.UserKey)
if cmp > 0 || (i.vState.upper.IsExclusiveSentinel() && cmp == 0) {
// We have an upper bound when the table is virtual.
if i.upper != nil && key != nil {
cmp := i.cmp(key.UserKey, i.upper)
if cmp > 0 || (!i.endKeyInclusive && cmp == 0) {
return nil, base.LazyValue{}
}
}
Expand Down
7 changes: 4 additions & 3 deletions sstable/reader_iter_two_lvl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1113,9 +1113,10 @@ func (i *twoLevelCompactionIterator) skipForward(
*i.bytesIterated += uint64(curOffset - i.prevOffset)
i.prevOffset = curOffset

if i.vState != nil && key != nil {
cmp := i.cmp(key.UserKey, i.vState.upper.UserKey)
if cmp > 0 || (i.vState.upper.IsExclusiveSentinel() && cmp == 0) {
// We have an upper bound when the table is virtual.
if i.upper != nil && key != nil {
cmp := i.cmp(key.UserKey, i.upper)
if cmp > 0 || (!i.endKeyInclusive && cmp == 0) {
return nil, base.LazyValue{}
}
}
Expand Down
62 changes: 62 additions & 0 deletions testdata/ingest_external
Original file line number Diff line number Diff line change
Expand Up @@ -418,3 +418,65 @@ iter
seek-lt de
----
a1: (foo, .)


# Test compactions with synthetic prefix.
reset
----

build-remote ext
set ax ax
set da da
set db db
set dc dc
set ux ux
----

# Replace prefix with one greater than the original one.
ingest-external prefix-replace=(d,e)
ext,10,ea,ed
----

# Replace prefix with one smaller than the original one.
ingest-external prefix-replace=(d,b)
ext,10,ba,bd
----

# Write some keys so we actually perform a compaction.
batch
set a a
set c c
set f f
----

compact a z
----

lsm
----
6:
000008:[a#0,SET-f#0,SET]

# Make sure we see both ba..bc and ea..ec.
iter
first
next
next
next
next
next
next
next
next
next
----
a: (a, .)
ba: (da, .)
bb: (db, .)
bc: (dc, .)
c: (c, .)
ea: (da, .)
eb: (db, .)
ec: (dc, .)
f: (f, .)
.

0 comments on commit 6ad6346

Please sign in to comment.