Skip to content

Commit

Permalink
db: fix SINGLEDEL, MERGE + DEL interaction
Browse files Browse the repository at this point in the history
Previously, when a MERGE consumed a deletion tombstone during a compaction, the
MERGE key was transformed into a SET. This SET lost knowledge of the previous
deletion. Previously, a subsequent SingleDelete (intended to remove the MERGE
key) could elide the SET, resurrecting internal keys that were deleted by the
consumed deletion tombstone.

This commit adjusts the compaction iterator to produce a SETWITHDEL (see #1255)
that preserves knowledge of the consumed deletion tombstone. This ensures that
a SINGLEDEL that meets the merged key yields a DEL.
  • Loading branch information
jbowens authored and sumeerbhola committed Dec 13, 2023
1 parent 50519c5 commit f065d7a
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 25 deletions.
7 changes: 4 additions & 3 deletions compaction_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ func (i *compactionIter) Next() (*InternalKey, []byte) {
// into a SET.
var includesBase bool
switch i.key.Kind() {
case InternalKeyKindSet:
case InternalKeyKindSet, InternalKeyKindSetWithDelete:
includesBase = true
case InternalKeyKindMerge:
default:
Expand Down Expand Up @@ -832,7 +832,8 @@ func (i *compactionIter) mergeNext(valueMerger ValueMerger) stripeChangeType {
// We've hit a deletion tombstone. Return everything up to this point and
// then skip entries until the next snapshot stripe. We change the kind
// of the result key to a Set so that it shadows keys in lower
// levels. That is, MERGE+DEL -> SET.
// levels. That is, MERGE+DEL -> SETWITHDEL.
//
// We do the same for SingleDelete since SingleDelete is only
// permitted (with deterministic behavior) for keys that have been
// set once since the last SingleDelete/Delete, so everything
Expand All @@ -845,7 +846,7 @@ func (i *compactionIter) mergeNext(valueMerger ValueMerger) stripeChangeType {
// single Set, and then merge in any following Sets, but that is
// complicated wrt code and unnecessary given the narrow permitted
// use of SingleDelete.
i.key.SetKind(InternalKeyKindSet)
i.key.SetKind(InternalKeyKindSetWithDelete)
i.skip = true
return sameStripeSkippable

Expand Down
14 changes: 7 additions & 7 deletions testdata/compaction_iter
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

# Non-deterministic use of SINGLEDEL where there are two older SETs that have
Expand All @@ -803,7 +803,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

define
Expand Down Expand Up @@ -1067,29 +1067,29 @@ iter
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter allow-zero-seqnum=true
first
next
----
a#0,1:5[base]
a#0,18:5[base]
.

iter elide-tombstones=true
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter snapshots=2
first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand All @@ -1098,7 +1098,7 @@ first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand Down
32 changes: 24 additions & 8 deletions testdata/compaction_iter_delete_sized
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

# Non-deterministic use of SINGLEDEL where there are two older SETs that have
Expand All @@ -802,7 +802,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

define
Expand Down Expand Up @@ -1066,29 +1066,29 @@ iter
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter allow-zero-seqnum=true
first
next
----
a#0,1:5[base]
a#0,18:5[base]
.

iter elide-tombstones=true
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter snapshots=2
first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand All @@ -1097,7 +1097,7 @@ first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand Down Expand Up @@ -1863,7 +1863,7 @@ missized-dels=0

# Regression test for #3087.
#
# Whne a DELSIZED and a SINGLEDEL meet in a compaction, a DEL key should be
# When a DELSIZED and a SINGLEDEL meet in a compaction, a DEL key should be
# emitted.

define
Expand All @@ -1879,3 +1879,19 @@ next
----
a#5,0:
.

# When a MERGE and a DEL[SIZED] meet in a compaction, a SETWITHDEL key (NOT a
# SET) should be emitted. Otherwise, a sequence such as SINGLEDDEL, MERGE, DEL,
# SET could result in the SET re-appearing.

define
a.MERGE.5:foo
a.DEL.3:
----

iter
first
next
----
a#5,18:foo[base]
.
14 changes: 7 additions & 7 deletions testdata/compaction_iter_set_with_del
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

# Non-deterministic use of SINGLEDEL where there are two older SETs that have
Expand All @@ -802,7 +802,7 @@ iter
first
next
----
a#6,1:b[base]
a#6,18:b[base]
.

define
Expand Down Expand Up @@ -1066,29 +1066,29 @@ iter
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter allow-zero-seqnum=true
first
next
----
a#0,1:5[base]
a#0,18:5[base]
.

iter elide-tombstones=true
first
next
----
a#5,1:5[base]
a#5,18:5[base]
.

iter snapshots=2
first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand All @@ -1097,7 +1097,7 @@ first
next
next
----
a#5,1:5[base]
a#5,18:5[base]
a#1,2:1
.

Expand Down

0 comments on commit f065d7a

Please sign in to comment.