Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(blooms): Fix findGaps when ownership goes to MaxUInt64 and that is covered by existing meta #12558

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading