Skip to content

Commit

Permalink
storage: use RangeKeyChanged() in MVCCClearTimeRange
Browse files Browse the repository at this point in the history
Some minor optimizations that might yield minor performance
improvements. We don't have any benchmarks for this function, and it's
used fairly rarely.

Release justification: bug fixes and low-risk updates to new functionality

Release note: None
  • Loading branch information
erikgrinaker committed Aug 21, 2022
1 parent 1b064b4 commit cb09df3
Showing 1 changed file with 26 additions and 26 deletions.
52 changes: 26 additions & 26 deletions pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2554,7 +2554,7 @@ func MVCCClearTimeRange(
}
}

clearRangeKeys = MVCCRangeKeyStack{}
clearRangeKeys.Clear()
return nil
}

Expand Down Expand Up @@ -2595,10 +2595,6 @@ func MVCCClearTimeRange(
// restoredMeta becomes the latest version of this MVCC key.
var restoredMeta enginepb.MVCCMetadata

// rangeKeyStart contains the start bound of the current range key, if any.
// This allows skipping range keys that we've already seen and processed.
var rangeKeyStart roachpb.Key

iter.SeekGE(MVCCKey{Key: key})
for {
if ok, err := iter.Valid(); err != nil {
Expand All @@ -2611,28 +2607,28 @@ func MVCCClearTimeRange(

// If we encounter a new range key stack, flush the previous range keys (if
// any) and buffer these for clearing.
if bounds := iter.RangeBounds(); !bounds.Key.Equal(rangeKeyStart) {
rangeKeyStart = append(rangeKeyStart[:0], bounds.Key...)

if err := flushRangeKeys(nil); err != nil {
if iter.RangeKeyChanged() {
if err := flushRangeKeys(nil); err != nil { // empties clearRangeKeys
return nil, err
}
if batchSize >= maxBatchSize || batchByteSize >= maxBatchByteSize {
resumeKey = k.Key.Clone()
break
}

// TODO(erikgrinaker): Consider a Clone() variant that can reuse a buffer.
clearRangeKeys = iter.RangeKeys().Clone()
}

if hasPoint, _ := iter.HasPointAndRange(); !hasPoint {
// If we landed on a bare range tombstone, we need to check if it revealed
// anything below the time bounds as well.
iter.NextIgnoringTime()
continue
hasPoint, hasRange := iter.HasPointAndRange()
if hasRange {
iter.RangeKeys().CloneInto(&clearRangeKeys)
}
if !hasPoint {
// If we landed on a bare range tombstone, we need to check if it revealed
// anything below the time bounds as well.
iter.NextIgnoringTime()
continue
}
}

// Process point keys.
vRaw := iter.UnsafeValue()
v, err := DecodeMVCCValue(vRaw)
if err != nil {
Expand All @@ -2652,14 +2648,17 @@ func MVCCClearTimeRange(

// If there was an MVCC range tombstone between this version and the
// cleared key, then we didn't restore it after all, but we must still
// adjust the stats for the range tombstone.
// adjust the stats for the range tombstone. RangeKeysIgnoringTime()
// is cheap, so we don't need any caching here.
if !restoredMeta.Deleted {
if v, ok := iter.RangeKeysIgnoringTime().FirstAbove(k.Timestamp); ok {
if v.Timestamp.LessEq(clearedMeta.Timestamp.ToTimestamp()) {
restoredMeta.Deleted = true
restoredMeta.KeyBytes = 0
restoredMeta.ValBytes = 0
restoredMeta.Timestamp = v.Timestamp.ToLegacyTimestamp()
if rangeKeys := iter.RangeKeysIgnoringTime(); !rangeKeys.IsEmpty() {
if v, ok := rangeKeys.FirstAbove(k.Timestamp); ok {
if v.Timestamp.LessEq(clearedMeta.Timestamp.ToTimestamp()) {
restoredMeta.Deleted = true
restoredMeta.KeyBytes = 0
restoredMeta.ValBytes = 0
restoredMeta.Timestamp = v.Timestamp.ToLegacyTimestamp()
}
}
}
}
Expand Down Expand Up @@ -2694,7 +2693,8 @@ func MVCCClearTimeRange(
if v, ok := clearRangeKeys.FirstAbove(k.Timestamp); ok {
if !clearedMetaKey.Key.Equal(k.Key) ||
!clearedMeta.Timestamp.ToTimestamp().LessEq(v.Timestamp) {
if !iter.RangeKeysIgnoringTime().HasBetween(k.Timestamp, v.Timestamp.Prev()) {
rangeKeys := iter.RangeKeysIgnoringTime()
if rangeKeys.IsEmpty() || !rangeKeys.HasBetween(k.Timestamp, v.Timestamp.Prev()) {
ms.Add(enginepb.MVCCStats{
LastUpdateNanos: v.Timestamp.WallTime,
LiveCount: 1,
Expand Down

0 comments on commit cb09df3

Please sign in to comment.