From 0679437825d1fa802c6c78a4f8e3f550e23477b9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 27 Oct 2022 18:29:01 +0100 Subject: [PATCH 1/6] promql/tests: couple more labels.FromStrings Signed-off-by: Bryan Boreham --- promql/engine_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/promql/engine_test.go b/promql/engine_test.go index 660ffda5ad..f389951eb3 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3114,17 +3114,11 @@ func TestRangeQuery(t *testing.T) { Result: Matrix{ Series{ Points: []Point{{V: 1, T: 0}, {V: 3, T: 60000}, {V: 5, T: 120000}}, - Metric: labels.Labels{ - labels.Label{Name: "__name__", Value: "bar"}, - labels.Label{Name: "job", Value: "2"}, - }, + Metric: labels.FromStrings("__name__", "bar", "job", "2"), }, Series{ Points: []Point{{V: 3, T: 60000}, {V: 5, T: 120000}}, - Metric: labels.Labels{ - labels.Label{Name: "__name__", Value: "foo"}, - labels.Label{Name: "job", Value: "1"}, - }, + Metric: labels.FromStrings("__name__", "foo", "job", "1"), }, }, Start: time.Unix(0, 0), From aa9385ea88b58ce124e19e38c43aeb302dabb7b6 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 27 Oct 2022 18:30:17 +0100 Subject: [PATCH 2/6] tsdb: one more labels.FromStrings Signed-off-by: Bryan Boreham --- tsdb/head_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 7853a9d869..36f498b9ef 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -2448,10 +2448,7 @@ func TestHeadShardedPostings(t *testing.T) { // Append some series. app := head.Appender(context.Background()) for i := 0; i < 100; i++ { - _, err := app.Append(0, labels.Labels{ - {Name: "unique", Value: fmt.Sprintf("value%d", i)}, - {Name: "const", Value: "1"}, - }, 100, 0) + _, err := app.Append(0, labels.FromStrings("unique", fmt.Sprintf("value%d", i), "const", "1"), 100, 0) require.NoError(t, err) } require.NoError(t, app.Commit()) From 9bb9faabb1cb5333a3e98af5d57c87279ef2f953 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 27 Oct 2022 18:32:54 +0100 Subject: [PATCH 3/6] rules tests: use EmptyLabels instead of nil Signed-off-by: Bryan Boreham --- rules/manager_test.go | 12 ++++++------ rules/origin_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rules/manager_test.go b/rules/manager_test.go index 98ce3c35b1..f993c28712 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -831,7 +831,7 @@ func TestUpdate_AlwaysRestore(t *testing.T) { ruleManager.start() defer ruleManager.Stop() - err := ruleManager.Update(10*time.Second, []string{"fixtures/rules_alerts.yaml"}, nil, "", nil) + err := ruleManager.Update(10*time.Second, []string{"fixtures/rules_alerts.yaml"}, labels.EmptyLabels(), "", nil) require.NoError(t, err) for _, g := range ruleManager.groups { @@ -840,7 +840,7 @@ func TestUpdate_AlwaysRestore(t *testing.T) { } // Use different file, so groups haven't changed, therefore, we expect state restoration - err = ruleManager.Update(10*time.Second, []string{"fixtures/rules_alerts2.yaml"}, nil, "", nil) + err = ruleManager.Update(10*time.Second, []string{"fixtures/rules_alerts2.yaml"}, labels.EmptyLabels(), "", nil) for _, g := range ruleManager.groups { require.True(t, g.shouldRestore) } @@ -863,7 +863,7 @@ func TestUpdate_AlwaysRestoreDoesntAffectUnchangedGroups(t *testing.T) { ruleManager.start() defer ruleManager.Stop() - err := ruleManager.Update(10*time.Second, files, nil, "", nil) + err := ruleManager.Update(10*time.Second, files, labels.EmptyLabels(), "", nil) require.NoError(t, err) for _, g := range ruleManager.groups { @@ -872,7 +872,7 @@ func TestUpdate_AlwaysRestoreDoesntAffectUnchangedGroups(t *testing.T) { } // Use the same file, so groups haven't changed, therefore, we don't expect state restoration - err = ruleManager.Update(10*time.Second, files, nil, "", nil) + err = ruleManager.Update(10*time.Second, files, labels.EmptyLabels(), "", nil) for _, g := range ruleManager.groups { require.False(t, g.shouldRestore) } @@ -1090,7 +1090,7 @@ func reloadRules(rgs *rulefmt.RuleGroups, t *testing.T, tmpFile *os.File, ruleMa _, _ = tmpFile.Seek(0, 0) _, err = tmpFile.Write(bs) require.NoError(t, err) - err = ruleManager.Update(interval, []string{tmpFile.Name()}, nil, "", nil) + err = ruleManager.Update(interval, []string{tmpFile.Name()}, labels.EmptyLabels(), "", nil) require.NoError(t, err) } @@ -1686,7 +1686,7 @@ groups: }, }) m.start() - err = m.Update(time.Second, []string{fname}, nil, "", nil) + err = m.Update(time.Second, []string{fname}, labels.EmptyLabels(), "", nil) require.NoError(t, err) rgs := m.RuleGroups() diff --git a/rules/origin_test.go b/rules/origin_test.go index 70463f12b0..f25e8c2a59 100644 --- a/rules/origin_test.go +++ b/rules/origin_test.go @@ -29,7 +29,7 @@ import ( type unknownRule struct{} func (u unknownRule) Name() string { return "" } -func (u unknownRule) Labels() labels.Labels { return nil } +func (u unknownRule) Labels() labels.Labels { return labels.EmptyLabels() } func (u unknownRule) Eval(ctx context.Context, evalDelay time.Duration, time time.Time, queryFunc QueryFunc, url *url.URL, i int) (promql.Vector, error) { return nil, nil } From 2e22653db482e7840ebe383ecde42503820fda2d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 27 Oct 2022 18:34:26 +0100 Subject: [PATCH 4/6] tsdb: use abstractions over Labels Signed-off-by: Bryan Boreham --- tsdb/compact.go | 6 +++++- tsdb/compact_test.go | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tsdb/compact.go b/tsdb/compact.go index 6063e4a5f1..7c721de3ee 100644 --- a/tsdb/compact.go +++ b/tsdb/compact.go @@ -1176,13 +1176,17 @@ func (c *LeveledCompactor) populateSymbols(sets []storage.ChunkSeriesSet, outBlo obIx = labels.StableHash(s.Labels()) % uint64(len(outBlocks)) } - for _, l := range s.Labels() { + err := s.Labels().Validate(func(l labels.Label) error { if err := batchers[obIx].addSymbol(l.Name); err != nil { return errors.Wrap(err, "addSymbol to batcher") } if err := batchers[obIx].addSymbol(l.Value); err != nil { return errors.Wrap(err, "addSymbol to batcher") } + return nil + }) + if err != nil { + return err } } diff --git a/tsdb/compact_test.go b/tsdb/compact_test.go index f331adb675..4db5f3b178 100644 --- a/tsdb/compact_test.go +++ b/tsdb/compact_test.go @@ -596,10 +596,10 @@ func TestCompaction_CompactWithSplitting(t *testing.T) { require.Equal(t, uint64(shardIndex), lbls.Labels().Hash()%shardCount) // Collect all symbols used by series. - for _, l := range lbls.Labels() { + lbls.Labels().Range(func(l labels.Label) { seriesSymbols[l.Name] = struct{}{} seriesSymbols[l.Value] = struct{}{} - } + }) } require.NoError(t, p.Err()) From 6009066423f0ba1ddfe8b885c3945b7d3a5c43dc Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 27 Mar 2023 17:43:55 +0000 Subject: [PATCH 5/6] tsdb/index tests: use labels.FromStrings --- tsdb/index/index_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tsdb/index/index_test.go b/tsdb/index/index_test.go index 2732a0d42b..930c2ba048 100644 --- a/tsdb/index/index_test.go +++ b/tsdb/index/index_test.go @@ -630,10 +630,8 @@ func BenchmarkReader_ShardedPostings(b *testing.B) { require.NoError(b, iw.AddSymbol("unique")) for i := 1; i <= numSeries; i++ { - require.NoError(b, iw.AddSeries(storage.SeriesRef(i), labels.Labels{ - {Name: "const", Value: fmt.Sprintf("%10d", 1)}, - {Name: "unique", Value: fmt.Sprintf("%10d", i)}, - })) + require.NoError(b, iw.AddSeries(storage.SeriesRef(i), + labels.FromStrings("const", fmt.Sprintf("%10d", 1), "unique", fmt.Sprintf("%10d", i)))) } require.NoError(b, iw.Close()) From cc8eb55a225e6951859759075bd99dee73d7cdf0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 27 Mar 2023 18:03:03 +0000 Subject: [PATCH 6/6] tsdb: call StableHash as appropriate labels.Labels.Hash() is not guaranteed to be stable over time. --- tsdb/compact_test.go | 2 +- tsdb/head_test.go | 2 +- tsdb/index/index_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tsdb/compact_test.go b/tsdb/compact_test.go index 4db5f3b178..ef62c6966b 100644 --- a/tsdb/compact_test.go +++ b/tsdb/compact_test.go @@ -593,7 +593,7 @@ func TestCompaction_CompactWithSplitting(t *testing.T) { ref := p.At() require.NoError(t, idxr.Series(ref, &lbls, nil)) - require.Equal(t, uint64(shardIndex), lbls.Labels().Hash()%shardCount) + require.Equal(t, uint64(shardIndex), labels.StableHash(lbls.Labels())%shardCount) // Collect all symbols used by series. lbls.Labels().Range(func(l labels.Label) { diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 36f498b9ef..4571e4db96 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -2495,7 +2495,7 @@ func TestHeadShardedPostings(t *testing.T) { var lbls labels.ScratchBuilder require.NoError(t, ir.Series(id, &lbls, nil)) - require.Equal(t, shardIndex, lbls.Labels().Hash()%shardCount) + require.Equal(t, shardIndex, labels.StableHash(lbls.Labels())%shardCount) } } } diff --git a/tsdb/index/index_test.go b/tsdb/index/index_test.go index 930c2ba048..64f8674f77 100644 --- a/tsdb/index/index_test.go +++ b/tsdb/index/index_test.go @@ -292,7 +292,7 @@ func TestIndexRW_Postings(t *testing.T) { var lbls labels.ScratchBuilder require.NoError(t, ir.Series(id, &lbls, nil)) - require.Equal(t, shardIndex, lbls.Labels().Hash()%shardCount) + require.Equal(t, shardIndex, labels.StableHash(lbls.Labels())%shardCount) } } })