From 1c3c8a8775db90c1d20bfcf0bc3966fd7a712820 Mon Sep 17 00:00:00 2001 From: Salva Corts Date: Wed, 10 Apr 2024 17:00:33 +0200 Subject: [PATCH] fix(blooms): Fix findGaps when ownership goes to MaxUInt64 and that is covered by existing meta --- pkg/bloomcompactor/controller.go | 14 +++++++++++--- pkg/bloomcompactor/controller_test.go | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/bloomcompactor/controller.go b/pkg/bloomcompactor/controller.go index 37a7c6bc69b69..e5416c7866cdd 100644 --- a/pkg/bloomcompactor/controller.go +++ b/pkg/bloomcompactor/controller.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "math" "sort" "sync" @@ -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) @@ -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)) } diff --git a/pkg/bloomcompactor/controller_test.go b/pkg/bloomcompactor/controller_test.go index 2367ee3cc9566..5c6a506473476 100644 --- a/pkg/bloomcompactor/controller_test.go +++ b/pkg/bloomcompactor/controller_test.go @@ -2,6 +2,7 @@ package bloomcompactor import ( "fmt" + "math" "testing" "time" @@ -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)