Skip to content

Commit

Permalink
storage: adjust prefix point-synthesizing behavior
Browse files Browse the repository at this point in the history
Update the point-synthesizing iterator to save range key state even when in
prefix iteration mode. Previously the point-synthesizing iterator avoided
copying the range key state when used within prefix iteration mode. This led to
a bug where an iterator could be moved into an exhausted position, at which the
previous range key buffers are no longer valid.

Release note: None
  • Loading branch information
jbowens committed Feb 23, 2023
1 parent 05f7c76 commit 66b4030
Showing 1 changed file with 11 additions and 24 deletions.
35 changes: 11 additions & 24 deletions pkg/storage/point_synthesizing_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,17 +288,11 @@ func (i *PointSynthesizingIter) updateRangeKeys() {
// last call to updateRangeKeys().
if i.rangeKeyChanged {
i.rangeKeyChanged = false
if i.prefix {
// A prefix iterator will always be at the start bound of the range key,
// and never move onto a different range key, so we can omit the cloning.
i.rangeKeys = i.iter.RangeKeys().Versions
} else {
rangeKeys := i.iter.RangeKeys()
i.rangeKeysStart = append(i.rangeKeysStart[:0], rangeKeys.Bounds.Key...)
rangeKeys.Versions.CloneInto(&i.rangeKeysBuf)
i.rangeKeys = i.rangeKeysBuf
i.rangeKeysStartPassed = false // we'll compare below
}
rangeKeys := i.iter.RangeKeys()
i.rangeKeysStart = append(i.rangeKeysStart[:0], rangeKeys.Bounds.Key...)
rangeKeys.Versions.CloneInto(&i.rangeKeysBuf)
i.rangeKeys = i.rangeKeysBuf
i.rangeKeysStartPassed = false // we'll compare below
}

// Update the current iterator position.
Expand Down Expand Up @@ -899,19 +893,12 @@ func (i *PointSynthesizingIter) assertInvariants() error {
}

// rangeKeysStart must be set, and rangeKeysPos must be at or after it.
// prefix iterators do not set rangeKeysStart.
if !i.prefix {
if len(i.rangeKeysStart) == 0 {
return errors.AssertionFailedf("no rangeKeysStart at %s", i.iter.UnsafeKey())
}
if i.rangeKeysPos.Compare(i.rangeKeysStart) < 0 {
return errors.AssertionFailedf("rangeKeysPos %s not after rangeKeysStart %s",
i.rangeKeysPos, i.rangeKeysStart)
}
} else {
if len(i.rangeKeysStart) != 0 {
return errors.AssertionFailedf("rangeKeysStart set to %s for prefix iterator", i.rangeKeysStart)
}
if len(i.rangeKeysStart) == 0 {
return errors.AssertionFailedf("no rangeKeysStart at %s", i.iter.UnsafeKey())
}
if i.rangeKeysPos.Compare(i.rangeKeysStart) < 0 {
return errors.AssertionFailedf("rangeKeysPos %s not after rangeKeysStart %s",
i.rangeKeysPos, i.rangeKeysStart)
}

// rangeKeysStartPassed must match rangeKeysPos and rangeKeysStart. Note that
Expand Down

0 comments on commit 66b4030

Please sign in to comment.