Skip to content

Commit

Permalink
metamorphic: no synthetic suffix with overlapping RangeKeySets
Browse files Browse the repository at this point in the history
Applying a synthetic suffix to overlapping `RangeKeySet`s leads to a
logical ambiguity - they both have the same suffix but they can have
different values. In practical terms it causes metamorphic test
failures because their relative sorting order is indeterminate.

Add code in the metamorphic tests to keep track of `RangeKeySet`s and
prevent the use of synthetic suffix when the external object has
overlapping `RangeKeySet`s.

Fixes #3833
Fixes #3824
  • Loading branch information
RaduBerinde committed Aug 16, 2024
1 parent ac08db2 commit 0e8a11a
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
7 changes: 7 additions & 0 deletions metamorphic/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,13 @@ func (g *generator) writerIngestExternalFiles() {
continue
}

// We can only use a synthetic suffix if we don't have overlapping range
// key sets (because they will become logically conflicting when we
// replace their suffixes with the synthetic one).
if g.keyManager.ExternalObjectHasOverlappingRangeKeySets(objs[i].externalObjID) {
continue
}

// Generate a suffix that sorts before any previously generated suffix.
objs[i].syntheticSuffix = g.keyGenerator.IncMaxSuffix()
}
Expand Down
35 changes: 32 additions & 3 deletions metamorphic/key_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ type objKeyMeta struct {
hasRangeDels bool
hasRangeKeys bool
hasRangeKeyUnset bool
// List of RangeKeySets for this object. Used to check for overlapping
// RangeKeySets in external files.
rangeKeySets []pebble.KeyRange
}

// MergeKey adds the given key at the given meta timestamp, merging the histories as needed.
Expand Down Expand Up @@ -252,6 +255,7 @@ func (okm *objKeyMeta) MergeFrom(from *objKeyMeta, metaTimestamp int, cmp base.C
okm.hasRangeDels = okm.hasRangeDels || from.hasRangeDels
okm.hasRangeKeys = okm.hasRangeKeys || from.hasRangeKeys
okm.hasRangeKeyUnset = okm.hasRangeKeyUnset || from.hasRangeKeyUnset
okm.rangeKeySets = append(okm.rangeKeySets, from.rangeKeySets...)
}

// objKeyMeta looks up the objKeyMeta for a given object, creating it if necessary.
Expand Down Expand Up @@ -325,6 +329,25 @@ func (k *keyManager) KeysForExternalIngest(obj externalObjWithBounds) []keyMeta
return res
}

func (k *keyManager) ExternalObjectHasOverlappingRangeKeySets(externalObjID objID) bool {
meta := k.objKeyMeta(externalObjID)
if len(meta.rangeKeySets) == 0 {
return false
}
ranges := meta.rangeKeySets
// Sort by start key.
slices.SortFunc(ranges, func(a, b pebble.KeyRange) int {
return k.comparer.Compare(a.Start, b.Start)
})
// Check overlap between adjacent ranges.
for i := 0; i < len(ranges)-1; i++ {
if ranges[i].OverlapsKeyRange(k.comparer.Compare, ranges[i+1]) {
return true
}
}
return false
}

func (k *keyManager) nextMetaTimestamp() int {
ret := k.metaTimestamp
k.metaTimestamp++
Expand Down Expand Up @@ -614,11 +637,17 @@ func (k *keyManager) update(o op) {
k.objKeyMeta(s.writerID).hasRangeKeys = true
case *rangeKeySetOp:
k.expandBounds(s.writerID, k.makeEndExclusiveBounds(s.start, s.end))
k.objKeyMeta(s.writerID).hasRangeKeys = true
meta := k.objKeyMeta(s.writerID)
meta.hasRangeKeys = true
meta.rangeKeySets = append(meta.rangeKeySets, pebble.KeyRange{
Start: s.start,
End: s.end,
})
case *rangeKeyUnsetOp:
k.expandBounds(s.writerID, k.makeEndExclusiveBounds(s.start, s.end))
k.objKeyMeta(s.writerID).hasRangeKeys = true
k.objKeyMeta(s.writerID).hasRangeKeyUnset = true
meta := k.objKeyMeta(s.writerID)
meta.hasRangeKeys = true
meta.hasRangeKeyUnset = true
case *ingestOp:
// Some ingestion operations may attempt to ingest overlapping sstables
// which is prohibited. We know at generation time whether these
Expand Down

0 comments on commit 0e8a11a

Please sign in to comment.