From c99634978cb189744946e6dc388f0cc4183e98f2 Mon Sep 17 00:00:00 2001 From: Salva Corts Date: Fri, 7 Jun 2024 11:41:19 +0200 Subject: [PATCH] fix: Fix bloom deleter PR after merge (#13167) --- pkg/bloombuild/planner/planner.go | 7 +--- pkg/bloombuild/planner/versioned_range.go | 38 +++++++++++++------ .../planner/versioned_range_test.go | 3 +- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/pkg/bloombuild/planner/planner.go b/pkg/bloombuild/planner/planner.go index 99cc25a49888..ea2ea5db531b 100644 --- a/pkg/bloombuild/planner/planner.go +++ b/pkg/bloombuild/planner/planner.go @@ -365,10 +365,7 @@ func (p *Planner) processTenantTaskResults( } combined := append(originalMetas, newMetas...) - outdated, err := outdatedMetas(combined) - if err != nil { - return fmt.Errorf("failed to find outdated metas: %w", err) - } + outdated := outdatedMetas(combined) level.Debug(logger).Log("msg", "found outdated metas", "outdated", len(outdated)) if err := p.deleteOutdatedMetasAndBlocks(ctx, table, tenant, outdated); err != nil { @@ -476,7 +473,7 @@ func (p *Planner) loadTenantWork( // If this is the first this we see this table, initialize the map if tenantTableWork[table] == nil { - tenantTableWork[table] = make(map[string][]v1.FingerprintBounds, tenants.Len()) + tenantTableWork[table] = make(map[string][]v1.FingerprintBounds, tenants.Remaining()) } for tenants.Next() && tenants.Err() == nil && ctx.Err() == nil { diff --git a/pkg/bloombuild/planner/versioned_range.go b/pkg/bloombuild/planner/versioned_range.go index 3a436353954e..578b5d7ef83a 100644 --- a/pkg/bloombuild/planner/versioned_range.go +++ b/pkg/bloombuild/planner/versioned_range.go @@ -210,17 +210,30 @@ func (t tsdbTokenRange) reassemble(from int) tsdbTokenRange { return t[:len(t)-(reassembleTo-from)] } -func outdatedMetas(metas []bloomshipper.Meta) (outdated []bloomshipper.Meta, err error) { +func outdatedMetas(metas []bloomshipper.Meta) []bloomshipper.Meta { + var outdated []bloomshipper.Meta + // Sort metas descending by most recent source when checking // for outdated metas (older metas are discarded if they don't change the range). sort.Slice(metas, func(i, j int) bool { - a, err := metas[i].MostRecentSource() - if err != nil { - panic(err.Error()) + a, aExists := metas[i].MostRecentSource() + b, bExists := metas[j].MostRecentSource() + + if !aExists && !bExists { + // stable sort two sourceless metas by their bounds (easier testing) + return metas[i].Bounds.Less(metas[j].Bounds) } - b, err := metas[j].MostRecentSource() - if err != nil { - panic(err.Error()) + + if !aExists { + // If a meta has no sources, it's out of date by definition. + // By convention we sort it to the beginning of the list and will mark it for removal later + return true + } + + if !bExists { + // if a exists but b does not, mark b as lesser, sorting b to the + // front + return false } return !a.TS.Before(b.TS) }) @@ -231,9 +244,11 @@ func outdatedMetas(metas []bloomshipper.Meta) (outdated []bloomshipper.Meta, err ) for _, meta := range metas { - mostRecent, err := meta.MostRecentSource() - if err != nil { - return nil, err + mostRecent, exists := meta.MostRecentSource() + if !exists { + // if the meta exists but does not reference a TSDB, it's out of date + // TODO(owen-d): this shouldn't happen, figure out why + outdated = append(outdated, meta) } version := int(model.TimeFromUnixNano(mostRecent.TS.UnixNano())) tokenRange, added = tokenRange.Add(version, meta.Bounds) @@ -242,6 +257,5 @@ func outdatedMetas(metas []bloomshipper.Meta) (outdated []bloomshipper.Meta, err } } - return outdated, nil - + return outdated } diff --git a/pkg/bloombuild/planner/versioned_range_test.go b/pkg/bloombuild/planner/versioned_range_test.go index 9827e9cd932c..e58f143842f1 100644 --- a/pkg/bloombuild/planner/versioned_range_test.go +++ b/pkg/bloombuild/planner/versioned_range_test.go @@ -315,8 +315,7 @@ func Test_OutdatedMetas(t *testing.T) { }, } { t.Run(tc.desc, func(t *testing.T) { - outdated, err := outdatedMetas(tc.metas) - require.NoError(t, err) + outdated := outdatedMetas(tc.metas) require.Equal(t, tc.exp, outdated) }) }