Skip to content

Commit

Permalink
db: fix skipped key during Prev at synthetic range key marker
Browse files Browse the repository at this point in the history
During forward seek calls (SeekGE, SeekGEWithLimit, SeekPrefixGE) on an
iterator that includes range keys, the Pebble iterator will stop at the search
key if there exists a range key straddling the search key. If there's no
coincident point key at the same position, this introduces an 'ephemeral'
iterator position that only exists after the seek.

If the user Prev'd while positioned over an 'ephemeral' iterator position, the
underlying internal iterator would mistakenly be Prev'd twice, skipping a key.

This change alters the logic around when to Prev twice, avoiding the second
Prev if the exposed iterator position is ephemeral.
  • Loading branch information
jbowens committed Feb 22, 2022
1 parent 13f8f7c commit 998400e
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
28 changes: 27 additions & 1 deletion iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ type iteratorRangeKeyState struct {
buf []byte
}

// isEphemeralPosition returns true iff the current iterator position is
// ephemeral, and won't be visited during subsequent relative positioning
// operations.
//
// The iterator position resulting from a SeekGE or SeekPrefixGE that lands on a
// straddling range key without a coincident point key is such a position.
func (i *Iterator) isEphemeralPosition() bool {
return i.rangeKey != nil && i.rangeKey.rangeKeyOnly && !i.equal(i.rangeKey.start, i.key)
}

type lastPositioningOpKind int8

const (
Expand Down Expand Up @@ -1269,8 +1279,24 @@ func (i *Iterator) PrevWithLimit(limit []byte) IterValidityState {
// Already at the right place.
}
if i.pos == iterPosCurForward || i.pos == iterPosNext || i.pos == iterPosCurForwardPaused {
stepAgain := i.pos == iterPosNext
// Switching direction.
stepAgain := i.pos == iterPosNext

// Synthetic range key markers are a special case. Consider SeekGE(b)
// which finds a range key [a, c). To ensure the user observes the range
// key, the Iterator pauses at Key() = b. The iterator must advance the
// internal iterator to see if there's also a coincident point key at
// 'b', leaving the iterator at iterPosNext if there's not.
//
// This is a problem: Synthetic range key markers are only interleaved
// during the original seek. A subsequent Prev() of i.iter will not move
// back onto the synthetic range key marker. In this case where the
// previous iterator position was a synthetic range key start boundary,
// we must not step a second time.
if i.isEphemeralPosition() {
stepAgain = false
}

// We set i.iterValidityState to IterExhausted here to force the calls
// to prevUserKey to save the current key i.iter is pointing at in
// order to determine when the prev user-key is reached.
Expand Down
25 changes: 25 additions & 0 deletions testdata/rangekeys
Original file line number Diff line number Diff line change
Expand Up @@ -571,3 +571,28 @@ c: (c, .)
cat: (., [cat-e) @3=bax)
d: (d, [cat-e) @3=bax)
.

# Test Prev-ing back over a synthetic range key marker. Synthetic range-key
# markers (the keys interleaved at 'c' during a SeekGE(c) when there's a
# straddling range key) are ephemeral, and Prev-ing back must move back the
# appropriate number of times.

reset
----

batch
set a a
range-key-set b e @1 foo
----
wrote 2 keys

combined-iter
seek-ge b
prev
seek-ge c
prev
----
b: (., [b-e) @1=foo)
a: (a, .)
c: (., [b-e) @1=foo)
b: (., [b-e) @1=foo)

0 comments on commit 998400e

Please sign in to comment.