Skip to content

Commit

Permalink
db: fix overlap check for flushable ingest excises
Browse files Browse the repository at this point in the history
Previously, we would incorrectly return false (denoting no overlap)
between passed-in UserKeyBounds and a flushable ingest, if we
see a file that starts after the passed-in bounds, even if there
was an excise span overlapping with those bounds. This change
fixes that, and adds a unit test to catch this edge case.

Fixes #3963.
  • Loading branch information
itsbilal committed Sep 26, 2024
1 parent 2569414 commit be56747
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 24 deletions.
2 changes: 1 addition & 1 deletion flushable.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ func (s *ingestedFlushable) anyFileOverlaps(bounds base.UserKeyBounds) bool {
if !bounds.End.IsUpperBoundFor(s.comparer.Compare, fileBounds.Start) {
// The file starts after the bounds end. There is no overlap, and
// further files will not overlap either (the files are sorted).
return false
break
}
// There is overlap. Note that UserKeyBounds.Overlaps() performs exactly the
// checks above.
Expand Down
14 changes: 13 additions & 1 deletion ingest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ func TestExcise(t *testing.T) {
EventListener: &EventListener{FlushEnd: func(info FlushInfo) {
flushed = true
}},
FormatMajorVersion: FormatVirtualSSTables,
FormatMajorVersion: FormatFlushableIngestExcises,
Logger: testLogger{t},
}
if blockSize != 0 {
Expand Down Expand Up @@ -709,6 +709,18 @@ func TestExcise(t *testing.T) {
}
return ""

case "block-flush":
d.mu.Lock()
d.mu.compact.flushing = true
d.mu.Unlock()
return ""

case "allow-flush":
d.mu.Lock()
d.mu.compact.flushing = false
d.mu.Unlock()
return ""

case "ingest":
flushed = false
noWait := false
Expand Down
94 changes: 72 additions & 22 deletions testdata/excise
Original file line number Diff line number Diff line change
Expand Up @@ -515,16 +515,16 @@ set c something

ingest-and-excise ext5 excise=c-cc
----
memtable flushed
flushable ingest

lsm
----
L0.0:
000012:[x#14,SET-x#14,SET]
000013:[x#14,SET-x#14,SET]
L6:
000013(000009):[a#0,SET-b#0,SET]
000014(000009):[a#0,SET-b#0,SET]
000010:[c#17,SET-c#17,SET]
000014(000009):[d@6#15,DEL-d@6#0,SET]
000015(000009):[d@6#15,DEL-d@6#0,SET]

iter lower=c upper=e
last
Expand Down Expand Up @@ -626,16 +626,16 @@ del b-e

ingest-and-excise ext5 excise=b-e
----
memtable flushed
flushable ingest

lsm
----
L0.0:
000010:[b#18,SET-f#18,SET]
000012:[x#15,SET-x#15,SET]
000013:[x#15,SET-x#15,SET]
L6:
000013(000009):[a#0,SET-a#0,SET]
000014(000009):[g#0,SET-g#0,SET]
000014(000009):[a#0,SET-a#0,SET]
000015(000009):[g#0,SET-g#0,SET]

iter lower=c upper=e
last
Expand Down Expand Up @@ -666,7 +666,7 @@ compact a z
lsm
----
L6:
000017:[a#0,SET-x#0,SET]
000018:[a#0,SET-x#0,SET]

batch commit
----
Expand All @@ -688,11 +688,11 @@ ingest-and-excise ext5 ext6 excise=b-e
lsm
----
L0.0:
000018:[a#22,SET-a#22,SET]
000019:[b#23,SET-f#23,SET]
000019:[a#22,SET-a#22,SET]
000020:[b#23,SET-f#23,SET]
L6:
000020(000017):[a#0,SET-aa#0,SET]
000021(000017):[f#0,SET-x#0,SET]
000021(000018):[a#0,SET-aa#0,SET]
000022(000018):[f#0,SET-x#0,SET]

iter lower=a upper=f
first
Expand All @@ -709,7 +709,7 @@ c: (somethingElse, .)
.
.

# Two ovelrapping ingestions wait on one another even if
# Two overlapping ingestions wait on one another even if
# the overlap is only on the excise span.

reset
Expand Down Expand Up @@ -756,14 +756,6 @@ e: (fee, .)
flush
----

lsm
----
L0.0:
000007(000006):[a#10,SET-a#10,SET]
L6:
000004:[b#15,SET-c#15,SET]
000008:[d#16,SET-e#16,SET]


iter
first
Expand All @@ -779,3 +771,61 @@ c: (bar, .)
d: (gee, .)
e: (fee, .)
.

# Regression test for #3963. Ensure that flushable ingests with excises
# correctly compute boundary overlaps with eventually file-only snapshots.

reset
----

batch
set a foo
set b bar
set c baz
set e something
----

flush
----

block-flush
----

batch
set b foobar
set d barbaz
----

build ext8
set d foobar
set e somethingelse
----

ingest-and-excise ext8 excise=b-g no-wait
----

file-only-snapshot s1
b c
----
ok

batch
set b invisible
----

allow-flush
----

flush
----

# This iterator should not return any keys.

iter snapshot=s1 lower=b upper=c
first
next
next
----
.
.
.

0 comments on commit be56747

Please sign in to comment.