From 6911f8c33fa52eb1093d1e7beb50f7888440ac61 Mon Sep 17 00:00:00 2001
From: Owen Diehl <ow.diehl@gmail.com>
Date: Wed, 31 Jan 2024 09:05:40 -0800
Subject: [PATCH] pr feedback Signed-off-by: Owen Diehl <ow.diehl@gmail.com>

---
 pkg/bloomcompactor/v2controller.go      | 15 +++++++++++----
 pkg/bloomcompactor/v2controller_test.go |  7 ++++++-
 pkg/storage/bloom/v1/dedupe.go          |  6 +++---
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/pkg/bloomcompactor/v2controller.go b/pkg/bloomcompactor/v2controller.go
index 07ceae02d94e7..31f73740c1ff7 100644
--- a/pkg/bloomcompactor/v2controller.go
+++ b/pkg/bloomcompactor/v2controller.go
@@ -88,7 +88,11 @@ func (s *SimpleBloomController) do(ctx context.Context) error {
 		return nil
 	}
 
-	work := blockPlansForGaps(tsdbsWithGaps, metas)
+	work, err := blockPlansForGaps(tsdbsWithGaps, metas)
+	if err != nil {
+		level.Error(s.logger).Log("msg", "failed to create plan", "err", err)
+		return errors.Wrap(err, "failed to create plan")
+	}
 
 	// 5. Generate Blooms
 	// Now that we have the gaps, we will generate a bloom block for each gap.
@@ -194,7 +198,7 @@ type blockPlan struct {
 // blockPlansForGaps groups tsdb gaps we wish to fill with overlapping but out of date blocks.
 // This allows us to expedite bloom generation by using existing blocks to fill in the gaps
 // since many will contain the same chunks.
-func blockPlansForGaps(tsdbs []tsdbGaps, metas []Meta) []blockPlan {
+func blockPlansForGaps(tsdbs []tsdbGaps, metas []Meta) ([]blockPlan, error) {
 	plans := make([]blockPlan, 0, len(tsdbs))
 
 	for _, idx := range tsdbs {
@@ -248,7 +252,10 @@ func blockPlansForGaps(tsdbs []tsdbGaps, metas []Meta) []blockPlan {
 				peekingBlocks,
 			)
 
-			deduped := v1.Collect[BlockRef](itr)
+			deduped, err := v1.Collect[BlockRef](itr)
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to dedupe blocks")
+			}
 			planGap.blocks = deduped
 
 			plan.gaps = append(plan.gaps, planGap)
@@ -257,7 +264,7 @@ func blockPlansForGaps(tsdbs []tsdbGaps, metas []Meta) []blockPlan {
 		plans = append(plans, plan)
 	}
 
-	return plans
+	return plans, nil
 }
 
 // Used to signal the gaps that need to be populated for a tsdb
diff --git a/pkg/bloomcompactor/v2controller_test.go b/pkg/bloomcompactor/v2controller_test.go
index 240413bc90d94..9f3f56153af32 100644
--- a/pkg/bloomcompactor/v2controller_test.go
+++ b/pkg/bloomcompactor/v2controller_test.go
@@ -237,6 +237,7 @@ func Test_blockPlansForGaps(t *testing.T) {
 		ownershipRange v1.FingerprintBounds
 		tsdbs          []tsdb.Identifier
 		metas          []Meta
+		err            bool
 		exp            []blockPlan
 	}{
 		{
@@ -403,7 +404,11 @@ func Test_blockPlansForGaps(t *testing.T) {
 			gaps, err := gapsBetweenTSDBsAndMetas(tc.ownershipRange, tc.tsdbs, tc.metas)
 			require.NoError(t, err)
 
-			plans := blockPlansForGaps(gaps, tc.metas)
+			plans, err := blockPlansForGaps(gaps, tc.metas)
+			if tc.err {
+				require.Error(t, err)
+				return
+			}
 			require.Equal(t, tc.exp, plans)
 
 		})
diff --git a/pkg/storage/bloom/v1/dedupe.go b/pkg/storage/bloom/v1/dedupe.go
index 008fc4bba07c3..2e1a7cca42f36 100644
--- a/pkg/storage/bloom/v1/dedupe.go
+++ b/pkg/storage/bloom/v1/dedupe.go
@@ -55,17 +55,17 @@ func (it *DedupeIter[A, B]) At() B {
 
 // Collect collects an interator into a slice. It uses
 // CollectInto with a new slice
-func Collect[T any](itr Iterator[T]) []T {
+func Collect[T any](itr Iterator[T]) ([]T, error) {
 	return CollectInto(itr, nil)
 }
 
 // CollectInto collects the elements of an iterator into a provided slice
 // which is returned
-func CollectInto[T any](itr Iterator[T], into []T) []T {
+func CollectInto[T any](itr Iterator[T], into []T) ([]T, error) {
 	into = into[:0]
 
 	for itr.Next() {
 		into = append(into, itr.At())
 	}
-	return into
+	return into, itr.Err()
 }