Skip to content

Commit

Permalink
fix(blooms): Fix findGaps when ownership goes to MaxUInt64 and that i…
Browse files Browse the repository at this point in the history
…s covered by existing meta (#12558)
  • Loading branch information
salvacorts committed Apr 17, 2024
1 parent 2f24b67 commit 0ee2a61
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
14 changes: 11 additions & 3 deletions pkg/bloomcompactor/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"math"
"sort"
"sync"

Expand Down Expand Up @@ -735,7 +736,11 @@ func findGaps(ownershipRange v1.FingerprintBounds, metas []v1.FingerprintBounds)

searchRange := ownershipRange.Slice(leftBound, clippedMeta.Max)
// update the left bound for the next iteration
leftBound = min(clippedMeta.Max+1, ownershipRange.Max+1)
// We do the max to prevent the max bound to overflow from MaxUInt64 to 0
leftBound = min(
max(clippedMeta.Max+1, clippedMeta.Max),
max(ownershipRange.Max+1, ownershipRange.Max),
)

// since we've already ensured that the meta is within the ownership range,
// we know the xor will be of length zero (when the meta is equal to the ownership range)
Expand All @@ -750,8 +755,11 @@ func findGaps(ownershipRange v1.FingerprintBounds, metas []v1.FingerprintBounds)
gaps = append(gaps, xors[0])
}

if leftBound <= ownershipRange.Max {
// There is a gap between the last meta and the end of the ownership range.
// If the leftBound is less than the ownership range max, and it's smaller than MaxUInt64,
// There is a gap between the last meta and the end of the ownership range.
// Note: we check `leftBound < math.MaxUint64` since in the loop above we clamp the
// leftBound to MaxUint64 to prevent an overflow to 0: `max(clippedMeta.Max+1, clippedMeta.Max)`
if leftBound < math.MaxUint64 && leftBound <= ownershipRange.Max {
gaps = append(gaps, v1.NewBounds(leftBound, ownershipRange.Max))
}

Expand Down
22 changes: 22 additions & 0 deletions pkg/bloomcompactor/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bloomcompactor

import (
"fmt"
"math"
"testing"
"time"

Expand Down Expand Up @@ -103,6 +104,27 @@ func Test_findGaps(t *testing.T) {
v1.NewBounds(6, 7),
},
},
{
desc: "full ownership range with single meta",
err: false,
exp: nil,
ownershipRange: v1.NewBounds(0, math.MaxUint64),
metas: []v1.FingerprintBounds{
v1.NewBounds(0, math.MaxUint64),
},
},
{
desc: "full ownership range with multiple metas",
err: false,
exp: nil,
ownershipRange: v1.NewBounds(0, math.MaxUint64),
// Three metas covering the whole 0 - MaxUint64
metas: []v1.FingerprintBounds{
v1.NewBounds(0, math.MaxUint64/3),
v1.NewBounds(math.MaxUint64/3+1, math.MaxUint64/2),
v1.NewBounds(math.MaxUint64/2+1, math.MaxUint64),
},
},
} {
t.Run(tc.desc, func(t *testing.T) {
gaps, err := findGaps(tc.ownershipRange, tc.metas)
Expand Down

0 comments on commit 0ee2a61

Please sign in to comment.