From 89bf6e1df9e474488d4042bfdd3b4dcdfcb08889 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 29 Jun 2022 17:10:14 +0100 Subject: [PATCH 01/35] tsdb: Tidy up some test code Use simpler utility function to create Labels objects, making fewer assumptions about the data structure. Signed-off-by: Bryan Boreham --- tsdb/ooo_head_read_test.go | 16 ++++------------ tsdb/querier_test.go | 12 ++++++------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/tsdb/ooo_head_read_test.go b/tsdb/ooo_head_read_test.go index 8dca1ea59..b489c6e57 100644 --- a/tsdb/ooo_head_read_test.go +++ b/tsdb/ooo_head_read_test.go @@ -379,23 +379,15 @@ func TestOOOHeadChunkReader_LabelValues(t *testing.T) { app := head.Appender(context.Background()) // Add in-order samples - _, err := app.Append(0, labels.Labels{ - {Name: "foo", Value: "bar1"}, - }, 100, 1) + _, err := app.Append(0, labels.FromStrings("foo", "bar1"), 100, 1) require.NoError(t, err) - _, err = app.Append(0, labels.Labels{ - {Name: "foo", Value: "bar2"}, - }, 100, 2) + _, err = app.Append(0, labels.FromStrings("foo", "bar2"), 100, 2) require.NoError(t, err) // Add ooo samples for those series - _, err = app.Append(0, labels.Labels{ - {Name: "foo", Value: "bar1"}, - }, 90, 1) + _, err = app.Append(0, labels.FromStrings("foo", "bar1"), 90, 1) require.NoError(t, err) - _, err = app.Append(0, labels.Labels{ - {Name: "foo", Value: "bar2"}, - }, 90, 2) + _, err = app.Append(0, labels.FromStrings("foo", "bar2"), 90, 2) require.NoError(t, err) require.NoError(t, app.Commit()) diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index 3b44cef51..e6a75da2f 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -2160,7 +2160,7 @@ func TestBlockBaseSeriesSet(t *testing.T) { { series: []refdSeries{ { - lset: labels.New([]labels.Label{{Name: "a", Value: "a"}}...), + lset: labels.FromStrings("a", "a"), chunks: []chunks.Meta{ {Ref: 29}, {Ref: 45}, @@ -2173,19 +2173,19 @@ func TestBlockBaseSeriesSet(t *testing.T) { ref: 12, }, { - lset: labels.New([]labels.Label{{Name: "a", Value: "a"}, {Name: "b", Value: "b"}}...), + lset: labels.FromStrings("a", "a", "b", "b"), chunks: []chunks.Meta{ {Ref: 82}, {Ref: 23}, {Ref: 234}, {Ref: 65}, {Ref: 26}, }, ref: 10, }, { - lset: labels.New([]labels.Label{{Name: "b", Value: "c"}}...), + lset: labels.FromStrings("b", "c"), chunks: []chunks.Meta{{Ref: 8282}}, ref: 1, }, { - lset: labels.New([]labels.Label{{Name: "b", Value: "b"}}...), + lset: labels.FromStrings("b", "b"), chunks: []chunks.Meta{ {Ref: 829}, {Ref: 239}, {Ref: 2349}, {Ref: 659}, {Ref: 269}, }, @@ -2198,14 +2198,14 @@ func TestBlockBaseSeriesSet(t *testing.T) { { series: []refdSeries{ { - lset: labels.New([]labels.Label{{Name: "a", Value: "a"}, {Name: "b", Value: "b"}}...), + lset: labels.FromStrings("a", "a", "b", "b"), chunks: []chunks.Meta{ {Ref: 82}, {Ref: 23}, {Ref: 234}, {Ref: 65}, {Ref: 26}, }, ref: 10, }, { - lset: labels.New([]labels.Label{{Name: "b", Value: "c"}}...), + lset: labels.FromStrings("b", "c"), chunks: []chunks.Meta{{Ref: 8282}}, ref: 3, }, From 1695a7ee2f4a76e50eab6fabfa27f30050dfeb8e Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 10 Jul 2022 15:29:04 +0100 Subject: [PATCH 02/35] promql: refactor BenchmarkRangeQuery so we can re-use test cases Signed-off-by: Bryan Boreham --- promql/bench_test.go | 56 +++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/promql/bench_test.go b/promql/bench_test.go index c3de6ca47..6fb20d1ab 100644 --- a/promql/bench_test.go +++ b/promql/bench_test.go @@ -27,17 +27,7 @@ import ( "github.com/prometheus/prometheus/util/teststorage" ) -func BenchmarkRangeQuery(b *testing.B) { - stor := teststorage.New(b) - defer stor.Close() - opts := EngineOpts{ - Logger: nil, - Reg: nil, - MaxSamples: 50000000, - Timeout: 100 * time.Second, - } - engine := NewEngine(opts) - +func setupRangeQueryTestData(stor *teststorage.TestStorage, engine *Engine, interval, numIntervals int) error { metrics := []labels.Labels{} metrics = append(metrics, labels.FromStrings("__name__", "a_one")) metrics = append(metrics, labels.FromStrings("__name__", "b_one")) @@ -65,25 +55,26 @@ func BenchmarkRangeQuery(b *testing.B) { } refs := make([]storage.SeriesRef, len(metrics)) - // A day of data plus 10k steps. - numIntervals := 8640 + 10000 - for s := 0; s < numIntervals; s++ { a := stor.Appender(context.Background()) - ts := int64(s * 10000) // 10s interval. + ts := int64(s * interval) for i, metric := range metrics { ref, _ := a.Append(refs[i], metric, ts, float64(s)+float64(i)/float64(len(metrics))) refs[i] = ref } if err := a.Commit(); err != nil { - b.Fatal(err) + return err } } + return nil +} - type benchCase struct { - expr string - steps int - } +type benchCase struct { + expr string + steps int +} + +func rangeQueryCases() []benchCase { cases := []benchCase{ // Plain retrieval. { @@ -210,7 +201,30 @@ func BenchmarkRangeQuery(b *testing.B) { tmp = append(tmp, benchCase{expr: c.expr, steps: 1000}) } } - cases = tmp + return tmp +} + +func BenchmarkRangeQuery(b *testing.B) { + stor := teststorage.New(b) + defer stor.Close() + opts := EngineOpts{ + Logger: nil, + Reg: nil, + MaxSamples: 50000000, + Timeout: 100 * time.Second, + } + engine := NewEngine(opts) + + const interval = 10000 // 10s interval. + // A day of data plus 10k steps. + numIntervals := 8640 + 10000 + + err := setupRangeQueryTestData(stor, engine, interval, numIntervals) + if err != nil { + b.Fatal(err) + } + cases := rangeQueryCases() + for _, c := range cases { name := fmt.Sprintf("expr=%s,steps=%d", c.expr, c.steps) b.Run(name, func(b *testing.B) { From a19b369f9e518e6b89b59a72c82ba3bb687b2a7a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 13 Dec 2022 18:14:58 +0000 Subject: [PATCH 03/35] labels: avoid lint warning on New() This code is a bit cleaner. Signed-off-by: Bryan Boreham --- model/labels/labels.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/model/labels/labels.go b/model/labels/labels.go index aafba218a..103e8e5c3 100644 --- a/model/labels/labels.go +++ b/model/labels/labels.go @@ -357,9 +357,7 @@ func EmptyLabels() Labels { // The caller has to guarantee that all label names are unique. func New(ls ...Label) Labels { set := make(Labels, 0, len(ls)) - for _, l := range ls { - set = append(set, l) - } + set = append(set, ls...) sort.Sort(set) return set From ea7345a09c3f566ebd4ee30cafc8b77ff98f8f5b Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 13 Dec 2022 19:02:25 +0000 Subject: [PATCH 04/35] labels: improve comment on Builder.Set Signed-off-by: Bryan Boreham --- model/labels/labels.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/labels/labels.go b/model/labels/labels.go index 103e8e5c3..08c353c8a 100644 --- a/model/labels/labels.go +++ b/model/labels/labels.go @@ -468,7 +468,7 @@ Outer: return b } -// Set the name/value pair as a label. +// Set the name/value pair as a label. A value of "" means delete that label. func (b *Builder) Set(n, v string) *Builder { if v == "" { // Empty labels are the same as missing labels. From 2b8b8d9ac774bf5cacd650125aad29e6ecb69939 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 26 May 2022 14:59:13 +0000 Subject: [PATCH 05/35] labels: new methods to work without access to internals Without changing the definition of `labels.Labels`, add methods which enable code using it to work without knowledge of the internals. Signed-off-by: Bryan Boreham --- model/labels/labels.go | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/model/labels/labels.go b/model/labels/labels.go index 08c353c8a..453c3f60d 100644 --- a/model/labels/labels.go +++ b/model/labels/labels.go @@ -412,6 +412,49 @@ func Compare(a, b Labels) int { return len(a) - len(b) } +// Copy labels from b on top of whatever was in ls previously, reusing memory or expanding if needed. +func (ls *Labels) CopyFrom(b Labels) { + (*ls) = append((*ls)[:0], b...) +} + +// IsEmpty returns true if ls represents an empty set of labels. +func (ls Labels) IsEmpty() bool { + return len(ls) == 0 +} + +// Range calls f on each label. +func (ls Labels) Range(f func(l Label)) { + for _, l := range ls { + f(l) + } +} + +// Validate calls f on each label. If f returns a non-nil error, then it returns that error cancelling the iteration. +func (ls Labels) Validate(f func(l Label) error) error { + for _, l := range ls { + if err := f(l); err != nil { + return err + } + } + return nil +} + +// InternStrings calls intern on every string value inside ls, replacing them with what it returns. +func (ls *Labels) InternStrings(intern func(string) string) { + for i, l := range *ls { + (*ls)[i].Name = intern(l.Name) + (*ls)[i].Value = intern(l.Value) + } +} + +// ReleaseStrings calls release on every string value inside ls. +func (ls Labels) ReleaseStrings(release func(string)) { + for _, l := range ls { + release(l.Name) + release(l.Value) + } +} + // Builder allows modifying Labels. type Builder struct { base Labels @@ -523,3 +566,41 @@ Outer: } return res } + +// ScratchBuilder allows efficient construction of a Labels from scratch. +type ScratchBuilder struct { + add Labels +} + +// NewScratchBuilder creates a ScratchBuilder initialized for Labels with n entries. +func NewScratchBuilder(n int) ScratchBuilder { + return ScratchBuilder{add: make([]Label, 0, n)} +} + +func (b *ScratchBuilder) Reset() { + b.add = b.add[:0] +} + +// Add a name/value pair. +// Note if you Add the same name twice you will get a duplicate label, which is invalid. +func (b *ScratchBuilder) Add(name, value string) { + b.add = append(b.add, Label{Name: name, Value: value}) +} + +// Sort the labels added so far by name. +func (b *ScratchBuilder) Sort() { + sort.Sort(b.add) +} + +// Return the name/value pairs added so far as a Labels object. +// Note: if you want them sorted, call Sort() first. +func (b *ScratchBuilder) Labels() Labels { + // Copy the slice, so the next use of ScratchBuilder doesn't overwrite. + return append([]Label{}, b.add...) +} + +// Write the newly-built Labels out to ls, reusing its buffer if long enough. +// Callers must ensure that there are no other references to ls. +func (b *ScratchBuilder) Overwrite(ls *Labels) { + (*ls) = append((*ls)[:0], b.add...) +} From 617bee60f19bfae850ffc6386e4be7637a0cb631 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 26 Jun 2022 20:11:53 +0100 Subject: [PATCH 06/35] labels: use ScratchBuilder in ReadLabels Instead of relying on being able to append to it like a slice. Signed-off-by: Bryan Boreham --- model/labels/test_utils.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/model/labels/test_utils.go b/model/labels/test_utils.go index a683588d1..05b816882 100644 --- a/model/labels/test_utils.go +++ b/model/labels/test_utils.go @@ -17,7 +17,6 @@ import ( "bufio" "fmt" "os" - "sort" "strings" ) @@ -51,13 +50,14 @@ func ReadLabels(fn string, n int) ([]Labels, error) { defer f.Close() scanner := bufio.NewScanner(f) + b := ScratchBuilder{} var mets []Labels hashes := map[uint64]struct{}{} i := 0 for scanner.Scan() && i < n { - m := make(Labels, 0, 10) + b.Reset() r := strings.NewReplacer("\"", "", "{", "", "}", "") s := r.Replace(scanner.Text()) @@ -65,10 +65,11 @@ func ReadLabels(fn string, n int) ([]Labels, error) { labelChunks := strings.Split(s, ",") for _, labelChunk := range labelChunks { split := strings.Split(labelChunk, ":") - m = append(m, Label{Name: split[0], Value: split[1]}) + b.Add(split[0], split[1]) } // Order of the k/v labels matters, don't assume we'll always receive them already sorted. - sort.Sort(m) + b.Sort() + m := b.Labels() h := m.Hash() if _, ok := hashes[h]; ok { From cbf432d2ac788b3bc83fd66630e3ea4cf0406644 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 26 May 2022 14:58:06 +0000 Subject: [PATCH 07/35] Update package labels tests for new labels.Labels type Re-did the FromStrings test to avoid assumptions about how it works. Signed-off-by: Bryan Boreham --- model/labels/labels_test.go | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/model/labels/labels_test.go b/model/labels/labels_test.go index 0fd0edacc..96b6ff9bc 100644 --- a/model/labels/labels_test.go +++ b/model/labels/labels_test.go @@ -36,10 +36,6 @@ func TestLabels_String(t *testing.T) { lables: Labels{}, expected: "{}", }, - { - lables: nil, - expected: "{}", - }, } for _, c := range cases { str := c.lables.String() @@ -316,18 +312,18 @@ func TestLabels_Equal(t *testing.T) { func TestLabels_FromStrings(t *testing.T) { labels := FromStrings("aaa", "111", "bbb", "222") - expected := Labels{ - { - Name: "aaa", - Value: "111", - }, - { - Name: "bbb", - Value: "222", - }, - } - - require.Equal(t, expected, labels, "unexpected labelset") + x := 0 + labels.Range(func(l Label) { + switch x { + case 0: + require.Equal(t, Label{Name: "aaa", Value: "111"}, l, "unexpected value") + case 1: + require.Equal(t, Label{Name: "bbb", Value: "222"}, l, "unexpected value") + default: + t.Fatalf("unexpected labelset value %d: %v", x, l) + } + x++ + }) require.Panics(t, func() { FromStrings("aaa", "111", "bbb") }) //nolint:staticcheck // Ignore SA5012, error is intentional test. } @@ -539,7 +535,6 @@ func TestBuilder(t *testing.T) { want: FromStrings("aaa", "111", "ccc", "333"), }, { - base: nil, set: []Label{{"aaa", "111"}, {"bbb", "222"}, {"ccc", "333"}}, del: []string{"bbb"}, want: FromStrings("aaa", "111", "ccc", "333"), @@ -604,8 +599,7 @@ func TestBuilder(t *testing.T) { func TestLabels_Hash(t *testing.T) { lbls := FromStrings("foo", "bar", "baz", "qux") require.Equal(t, lbls.Hash(), lbls.Hash()) - require.NotEqual(t, lbls.Hash(), Labels{lbls[1], lbls[0]}.Hash(), "unordered labels match.") - require.NotEqual(t, lbls.Hash(), Labels{lbls[0]}.Hash(), "different labels match.") + require.NotEqual(t, lbls.Hash(), FromStrings("foo", "bar").Hash(), "different labels match.") } var benchmarkLabelsResult uint64 @@ -623,7 +617,7 @@ func BenchmarkLabels_Hash(b *testing.B) { // Label ~20B name, 50B value. b.Set(fmt.Sprintf("abcdefghijabcdefghijabcdefghij%d", i), fmt.Sprintf("abcdefghijabcdefghijabcdefghijabcdefghijabcdefghij%d", i)) } - return b.Labels(nil) + return b.Labels(EmptyLabels()) }(), }, { @@ -634,7 +628,7 @@ func BenchmarkLabels_Hash(b *testing.B) { // Label ~50B name, 50B value. b.Set(fmt.Sprintf("abcdefghijabcdefghijabcdefghijabcdefghijabcdefghij%d", i), fmt.Sprintf("abcdefghijabcdefghijabcdefghijabcdefghijabcdefghij%d", i)) } - return b.Labels(nil) + return b.Labels(EmptyLabels()) }(), }, { From b10fd9aea3b169d60f9815d5baba6da7393c44ca Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 10 Jul 2022 15:22:49 +0100 Subject: [PATCH 08/35] model/labels: add a basic test for ScratchBuilder Signed-off-by: Bryan Boreham --- model/labels/labels_test.go | 40 +++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/model/labels/labels_test.go b/model/labels/labels_test.go index 96b6ff9bc..3a1732d22 100644 --- a/model/labels/labels_test.go +++ b/model/labels/labels_test.go @@ -596,6 +596,46 @@ func TestBuilder(t *testing.T) { } } +func TestScratchBuilder(t *testing.T) { + for i, tcase := range []struct { + add []Label + want Labels + }{ + { + add: []Label{}, + want: EmptyLabels(), + }, + { + add: []Label{{"aaa", "111"}}, + want: FromStrings("aaa", "111"), + }, + { + add: []Label{{"aaa", "111"}, {"bbb", "222"}, {"ccc", "333"}}, + want: FromStrings("aaa", "111", "bbb", "222", "ccc", "333"), + }, + { + add: []Label{{"bbb", "222"}, {"aaa", "111"}, {"ccc", "333"}}, + want: FromStrings("aaa", "111", "bbb", "222", "ccc", "333"), + }, + { + add: []Label{{"ddd", "444"}}, + want: FromStrings("ddd", "444"), + }, + } { + overwriteTarget := EmptyLabels() + t.Run(fmt.Sprint(i), func(t *testing.T) { + b := ScratchBuilder{} + for _, lbl := range tcase.add { + b.Add(lbl.Name, lbl.Value) + } + b.Sort() + require.Equal(t, tcase.want, b.Labels()) + b.Overwrite(&overwriteTarget) + require.Equal(t, tcase.want, overwriteTarget) + }) + } +} + func TestLabels_Hash(t *testing.T) { lbls := FromStrings("foo", "bar", "baz", "qux") require.Equal(t, lbls.Hash(), lbls.Hash()) From 8ad7b64c0f1ba15f6a4b2f3275707548b26e5e5f Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:13:03 +0000 Subject: [PATCH 09/35] Update package model/relabel for new labels.Labels type Signed-off-by: Bryan Boreham --- model/relabel/relabel.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/model/relabel/relabel.go b/model/relabel/relabel.go index c731f6e0d..0cc6eeeb7 100644 --- a/model/relabel/relabel.go +++ b/model/relabel/relabel.go @@ -203,20 +203,20 @@ func (re Regexp) String() string { // Process returns a relabeled copy of the given label set. The relabel configurations // are applied in order of input. -// If a label set is dropped, nil is returned. +// If a label set is dropped, EmptyLabels and false is returned. // May return the input labelSet modified. -func Process(lbls labels.Labels, cfgs ...*Config) labels.Labels { - lb := labels.NewBuilder(nil) +func Process(lbls labels.Labels, cfgs ...*Config) (ret labels.Labels, keep bool) { + lb := labels.NewBuilder(labels.EmptyLabels()) for _, cfg := range cfgs { - lbls = relabel(lbls, cfg, lb) - if lbls == nil { - return nil + lbls, keep = relabel(lbls, cfg, lb) + if !keep { + return labels.EmptyLabels(), false } } - return lbls + return lbls, true } -func relabel(lset labels.Labels, cfg *Config, lb *labels.Builder) labels.Labels { +func relabel(lset labels.Labels, cfg *Config, lb *labels.Builder) (ret labels.Labels, keep bool) { var va [16]string values := va[:0] if len(cfg.SourceLabels) > cap(values) { @@ -232,19 +232,19 @@ func relabel(lset labels.Labels, cfg *Config, lb *labels.Builder) labels.Labels switch cfg.Action { case Drop: if cfg.Regex.MatchString(val) { - return nil + return labels.EmptyLabels(), false } case Keep: if !cfg.Regex.MatchString(val) { - return nil + return labels.EmptyLabels(), false } case DropEqual: if lset.Get(cfg.TargetLabel) == val { - return nil + return labels.EmptyLabels(), false } case KeepEqual: if lset.Get(cfg.TargetLabel) != val { - return nil + return labels.EmptyLabels(), false } case Replace: indexes := cfg.Regex.FindStringSubmatchIndex(val) @@ -271,29 +271,29 @@ func relabel(lset labels.Labels, cfg *Config, lb *labels.Builder) labels.Labels mod := sum64(md5.Sum([]byte(val))) % cfg.Modulus lb.Set(cfg.TargetLabel, fmt.Sprintf("%d", mod)) case LabelMap: - for _, l := range lset { + lset.Range(func(l labels.Label) { if cfg.Regex.MatchString(l.Name) { res := cfg.Regex.ReplaceAllString(l.Name, cfg.Replacement) lb.Set(res, l.Value) } - } + }) case LabelDrop: - for _, l := range lset { + lset.Range(func(l labels.Label) { if cfg.Regex.MatchString(l.Name) { lb.Del(l.Name) } - } + }) case LabelKeep: - for _, l := range lset { + lset.Range(func(l labels.Label) { if !cfg.Regex.MatchString(l.Name) { lb.Del(l.Name) } - } + }) default: panic(fmt.Errorf("relabel: unknown relabel action type %q", cfg.Action)) } - return lb.Labels(lset) + return lb.Labels(lset), true } // sum64 sums the md5 hash to an uint64. From fe9fe0e1e5d7bc4efd796fadd594d3d6380d6a66 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:13:18 +0000 Subject: [PATCH 10/35] Update package model/relabel tests for new labels.Labels type Signed-off-by: Bryan Boreham --- model/relabel/relabel_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/model/relabel/relabel_test.go b/model/relabel/relabel_test.go index 0b0dfd511..d277d778d 100644 --- a/model/relabel/relabel_test.go +++ b/model/relabel/relabel_test.go @@ -28,6 +28,7 @@ func TestRelabel(t *testing.T) { input labels.Labels relabel []*Config output labels.Labels + drop bool }{ { input: labels.FromMap(map[string]string{ @@ -101,7 +102,7 @@ func TestRelabel(t *testing.T) { Action: Replace, }, }, - output: nil, + drop: true, }, { input: labels.FromMap(map[string]string{ @@ -115,7 +116,7 @@ func TestRelabel(t *testing.T) { Action: Drop, }, }, - output: nil, + drop: true, }, { input: labels.FromMap(map[string]string{ @@ -177,7 +178,7 @@ func TestRelabel(t *testing.T) { Action: Keep, }, }, - output: nil, + drop: true, }, { input: labels.FromMap(map[string]string{ @@ -483,7 +484,7 @@ func TestRelabel(t *testing.T) { TargetLabel: "__port1", }, }, - output: nil, + drop: true, }, { input: labels.FromMap(map[string]string{ @@ -517,7 +518,7 @@ func TestRelabel(t *testing.T) { TargetLabel: "__port2", }, }, - output: nil, + drop: true, }, } @@ -538,8 +539,11 @@ func TestRelabel(t *testing.T) { } } - res := Process(test.input, test.relabel...) - require.Equal(t, test.output, res) + res, keep := Process(test.input, test.relabel...) + require.Equal(t, !test.drop, keep) + if keep { + require.Equal(t, test.output, res) + } } } @@ -721,7 +725,7 @@ func BenchmarkRelabel(b *testing.B) { for _, tt := range tests { b.Run(tt.name, func(b *testing.B) { for i := 0; i < b.N; i++ { - _ = Process(tt.lbls, tt.cfgs...) + _, _ = Process(tt.lbls, tt.cfgs...) } }) } From 1f04899ae33720830abcce683e72f9446627f253 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:13:50 +0000 Subject: [PATCH 11/35] Update package model/textparse for new labels.Labels type Parse metrics using labels.ScratchBuilder, so we reduce assumptions about internals of Labels. Signed-off-by: Bryan Boreham --- model/textparse/openmetricsparse.go | 28 +++++++++++++--------------- model/textparse/promparse.go | 21 +++++++++------------ model/textparse/protobufparse.go | 27 ++++++++++++--------------- 3 files changed, 34 insertions(+), 42 deletions(-) diff --git a/model/textparse/openmetricsparse.go b/model/textparse/openmetricsparse.go index 932a3d96d..3fc80b5d6 100644 --- a/model/textparse/openmetricsparse.go +++ b/model/textparse/openmetricsparse.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "math" - "sort" "strings" "unicode/utf8" @@ -82,6 +81,7 @@ func (l *openMetricsLexer) Error(es string) { // This is based on the working draft https://docs.google.com/document/u/1/d/1KwV0mAXwwbvvifBvDKH_LU1YjyXE_wxCkHNoCGq1GX0/edit type OpenMetricsParser struct { l *openMetricsLexer + builder labels.ScratchBuilder series []byte text []byte mtype MetricType @@ -158,14 +158,11 @@ func (p *OpenMetricsParser) Comment() []byte { // Metric writes the labels of the current sample into the passed labels. // It returns the string from which the metric was parsed. func (p *OpenMetricsParser) Metric(l *labels.Labels) string { - // Allocate the full immutable string immediately, so we just - // have to create references on it below. + // Copy the buffer to a string: this is only necessary for the return value. s := string(p.series) - *l = append(*l, labels.Label{ - Name: labels.MetricName, - Value: s[:p.offsets[0]-p.start], - }) + p.builder.Reset() + p.builder.Add(labels.MetricName, s[:p.offsets[0]-p.start]) for i := 1; i < len(p.offsets); i += 4 { a := p.offsets[i] - p.start @@ -173,16 +170,16 @@ func (p *OpenMetricsParser) Metric(l *labels.Labels) string { c := p.offsets[i+2] - p.start d := p.offsets[i+3] - p.start + value := s[c:d] // Replacer causes allocations. Replace only when necessary. if strings.IndexByte(s[c:d], byte('\\')) >= 0 { - *l = append(*l, labels.Label{Name: s[a:b], Value: lvalReplacer.Replace(s[c:d])}) - continue + value = lvalReplacer.Replace(value) } - *l = append(*l, labels.Label{Name: s[a:b], Value: s[c:d]}) + p.builder.Add(s[a:b], value) } - // Sort labels. - sort.Sort(*l) + p.builder.Sort() + *l = p.builder.Labels() return s } @@ -204,17 +201,18 @@ func (p *OpenMetricsParser) Exemplar(e *exemplar.Exemplar) bool { e.Ts = p.exemplarTs } + p.builder.Reset() for i := 0; i < len(p.eOffsets); i += 4 { a := p.eOffsets[i] - p.start b := p.eOffsets[i+1] - p.start c := p.eOffsets[i+2] - p.start d := p.eOffsets[i+3] - p.start - e.Labels = append(e.Labels, labels.Label{Name: s[a:b], Value: s[c:d]}) + p.builder.Add(s[a:b], s[c:d]) } - // Sort the labels. - sort.Sort(e.Labels) + p.builder.Sort() + e.Labels = p.builder.Labels() return true } diff --git a/model/textparse/promparse.go b/model/textparse/promparse.go index a3bb8bb9b..d503ff9a7 100644 --- a/model/textparse/promparse.go +++ b/model/textparse/promparse.go @@ -21,7 +21,6 @@ import ( "fmt" "io" "math" - "sort" "strconv" "strings" "unicode/utf8" @@ -144,6 +143,7 @@ func (l *promlexer) Error(es string) { // Prometheus text exposition format. type PromParser struct { l *promlexer + builder labels.ScratchBuilder series []byte text []byte mtype MetricType @@ -212,14 +212,11 @@ func (p *PromParser) Comment() []byte { // Metric writes the labels of the current sample into the passed labels. // It returns the string from which the metric was parsed. func (p *PromParser) Metric(l *labels.Labels) string { - // Allocate the full immutable string immediately, so we just - // have to create references on it below. + // Copy the buffer to a string: this is only necessary for the return value. s := string(p.series) - *l = append(*l, labels.Label{ - Name: labels.MetricName, - Value: s[:p.offsets[0]-p.start], - }) + p.builder.Reset() + p.builder.Add(labels.MetricName, s[:p.offsets[0]-p.start]) for i := 1; i < len(p.offsets); i += 4 { a := p.offsets[i] - p.start @@ -227,16 +224,16 @@ func (p *PromParser) Metric(l *labels.Labels) string { c := p.offsets[i+2] - p.start d := p.offsets[i+3] - p.start + value := s[c:d] // Replacer causes allocations. Replace only when necessary. if strings.IndexByte(s[c:d], byte('\\')) >= 0 { - *l = append(*l, labels.Label{Name: s[a:b], Value: lvalReplacer.Replace(s[c:d])}) - continue + value = lvalReplacer.Replace(value) } - *l = append(*l, labels.Label{Name: s[a:b], Value: s[c:d]}) + p.builder.Add(s[a:b], value) } - // Sort labels to maintain the sorted labels invariant. - sort.Sort(*l) + p.builder.Sort() + *l = p.builder.Labels() return s } diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index a9c940879..37c6f0ebb 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -19,7 +19,6 @@ import ( "fmt" "io" "math" - "sort" "strings" "unicode/utf8" @@ -59,6 +58,8 @@ type ProtobufParser struct { // that we have to decode the next MetricFamily. state Entry + builder labels.ScratchBuilder // held here to reduce allocations when building Labels + mf *dto.MetricFamily // The following are just shenanigans to satisfy the Parser interface. @@ -245,23 +246,19 @@ func (p *ProtobufParser) Comment() []byte { // Metric writes the labels of the current sample into the passed labels. // It returns the string from which the metric was parsed. func (p *ProtobufParser) Metric(l *labels.Labels) string { - *l = append(*l, labels.Label{ - Name: labels.MetricName, - Value: p.getMagicName(), - }) + p.builder.Reset() + p.builder.Add(labels.MetricName, p.getMagicName()) for _, lp := range p.mf.GetMetric()[p.metricPos].GetLabel() { - *l = append(*l, labels.Label{ - Name: lp.GetName(), - Value: lp.GetValue(), - }) + p.builder.Add(lp.GetName(), lp.GetValue()) } if needed, name, value := p.getMagicLabel(); needed { - *l = append(*l, labels.Label{Name: name, Value: value}) + p.builder.Add(name, value) } // Sort labels to maintain the sorted labels invariant. - sort.Sort(*l) + p.builder.Sort() + *l = p.builder.Labels() return p.metricBytes.String() } @@ -305,12 +302,12 @@ func (p *ProtobufParser) Exemplar(ex *exemplar.Exemplar) bool { ex.HasTs = true ex.Ts = ts.GetSeconds()*1000 + int64(ts.GetNanos()/1_000_000) } + p.builder.Reset() for _, lp := range exProto.GetLabel() { - ex.Labels = append(ex.Labels, labels.Label{ - Name: lp.GetName(), - Value: lp.GetValue(), - }) + p.builder.Add(lp.GetName(), lp.GetValue()) } + p.builder.Sort() + ex.Labels = p.builder.Labels() return true } From 8d350d9e0c84d90d3e639521da47e89eec57a4ab Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:14:19 +0000 Subject: [PATCH 12/35] Update package model/textparse tests for new labels.Labels type We don't want to touch the result labels now we create them differently. Signed-off-by: Bryan Boreham --- model/textparse/openmetricsparse_test.go | 1 - model/textparse/promparse_test.go | 6 ++---- model/textparse/protobufparse_test.go | 2 -- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/model/textparse/openmetricsparse_test.go b/model/textparse/openmetricsparse_test.go index 9453db5d5..68b7fea8a 100644 --- a/model/textparse/openmetricsparse_test.go +++ b/model/textparse/openmetricsparse_test.go @@ -246,7 +246,6 @@ foo_total 17.0 1520879607.789 # {xx="yy"} 5` require.Equal(t, true, found) require.Equal(t, *exp[i].e, e) } - res = res[:0] case EntryType: m, typ := p.Type() diff --git a/model/textparse/promparse_test.go b/model/textparse/promparse_test.go index 6a1216da1..dadad3449 100644 --- a/model/textparse/promparse_test.go +++ b/model/textparse/promparse_test.go @@ -192,7 +192,6 @@ testmetric{label="\"bar\""} 1` require.Equal(t, exp[i].t, ts) require.Equal(t, exp[i].v, v) require.Equal(t, exp[i].lset, res) - res = res[:0] case EntryType: m, typ := p.Type() @@ -414,7 +413,7 @@ func BenchmarkParse(b *testing.B) { case EntrySeries: m, _, _ := p.Series() - res := make(labels.Labels, 0, 5) + var res labels.Labels p.Metric(&res) total += len(m) @@ -426,7 +425,7 @@ func BenchmarkParse(b *testing.B) { }) b.Run(parserName+"/decode-metric-reuse/"+fn, func(b *testing.B) { total := 0 - res := make(labels.Labels, 0, 5) + var res labels.Labels b.SetBytes(int64(len(buf) / promtestdataSampleCount)) b.ReportAllocs() @@ -451,7 +450,6 @@ func BenchmarkParse(b *testing.B) { total += len(m) i++ - res = res[:0] } } } diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index 33826ad8e..b8b868172 100644 --- a/model/textparse/protobufparse_test.go +++ b/model/textparse/protobufparse_test.go @@ -630,7 +630,6 @@ metric: < require.Equal(t, true, found) require.Equal(t, exp[i].e[0], e) } - res = res[:0] case EntryHistogram: m, ts, shs, fhs := p.Histogram() @@ -642,7 +641,6 @@ metric: < require.Equal(t, exp[i].t, int64(0)) } require.Equal(t, exp[i].lset, res) - res = res[:0] require.Equal(t, exp[i].m, string(m)) if shs != nil { require.Equal(t, exp[i].shs, shs) From 623d306f91103ea8e12960f2c3ba5c799b7c1a39 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:26:05 +0000 Subject: [PATCH 13/35] Update package config for new labels.Labels type Signed-off-by: Bryan Boreham --- config/config.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 8e8460d4c..8eb88eb51 100644 --- a/config/config.go +++ b/config/config.go @@ -80,7 +80,8 @@ func Load(s string, expandExternalLabels bool, logger log.Logger) (*Config, erro return cfg, nil } - for i, v := range cfg.GlobalConfig.ExternalLabels { + b := labels.ScratchBuilder{} + cfg.GlobalConfig.ExternalLabels.Range(func(v labels.Label) { newV := os.Expand(v.Value, func(s string) string { if s == "$" { return "$" @@ -93,10 +94,10 @@ func Load(s string, expandExternalLabels bool, logger log.Logger) (*Config, erro }) if newV != v.Value { level.Debug(logger).Log("msg", "External label replaced", "label", v.Name, "input", v.Value, "output", newV) - v.Value = newV - cfg.GlobalConfig.ExternalLabels[i] = v } - } + b.Add(v.Name, newV) + }) + cfg.GlobalConfig.ExternalLabels = b.Labels() return cfg, nil } @@ -361,13 +362,16 @@ func (c *GlobalConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } - for _, l := range gc.ExternalLabels { + if err := gc.ExternalLabels.Validate(func(l labels.Label) error { if !model.LabelName(l.Name).IsValid() { return fmt.Errorf("%q is not a valid label name", l.Name) } if !model.LabelValue(l.Value).IsValid() { return fmt.Errorf("%q is not a valid label value", l.Value) } + return nil + }); err != nil { + return err } // First set the correct scrape interval, then check that the timeout @@ -394,7 +398,7 @@ func (c *GlobalConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { // isZero returns true iff the global config is the zero value. func (c *GlobalConfig) isZero() bool { - return c.ExternalLabels == nil && + return c.ExternalLabels.IsEmpty() && c.ScrapeInterval == 0 && c.ScrapeTimeout == 0 && c.EvaluationInterval == 0 && From 927a14b0e9f575b46163114c202d1470962d262c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:18:47 +0000 Subject: [PATCH 14/35] Update package tsdb/index for new labels.Labels type Incomplete - needs further changes to `Decoder.Series()`. Signed-off-by: Bryan Boreham --- tsdb/index/index.go | 11 +++++++---- tsdb/index/postings.go | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tsdb/index/index.go b/tsdb/index/index.go index 9d7897860..ba78b5712 100644 --- a/tsdb/index/index.go +++ b/tsdb/index/index.go @@ -423,7 +423,7 @@ func (w *Writer) AddSeries(ref storage.SeriesRef, lset labels.Labels, chunks ... return errors.Errorf("out-of-order series added with label set %q", lset) } - if ref < w.lastRef && len(w.lastSeries) != 0 { + if ref < w.lastRef && !w.lastSeries.IsEmpty() { return errors.Errorf("series with reference greater than %d already added", ref) } // We add padding to 16 bytes to increase the addressable space we get through 4 byte @@ -437,9 +437,9 @@ func (w *Writer) AddSeries(ref storage.SeriesRef, lset labels.Labels, chunks ... } w.buf2.Reset() - w.buf2.PutUvarint(len(lset)) + w.buf2.PutUvarint(lset.Len()) - for _, l := range lset { + if err := lset.Validate(func(l labels.Label) error { var err error cacheEntry, ok := w.symbolCache[l.Name] nameIndex := cacheEntry.index @@ -465,6 +465,9 @@ func (w *Writer) AddSeries(ref storage.SeriesRef, lset labels.Labels, chunks ... } } w.buf2.PutUvarint32(valueIndex) + return nil + }); err != nil { + return err } w.buf2.PutUvarint(len(chunks)) @@ -496,7 +499,7 @@ func (w *Writer) AddSeries(ref storage.SeriesRef, lset labels.Labels, chunks ... return errors.Wrap(err, "write series data") } - w.lastSeries = append(w.lastSeries[:0], lset...) + w.lastSeries.CopyFrom(lset) w.lastRef = ref return nil diff --git a/tsdb/index/postings.go b/tsdb/index/postings.go index 22e85ab36..64574f85c 100644 --- a/tsdb/index/postings.go +++ b/tsdb/index/postings.go @@ -353,9 +353,9 @@ func (p *MemPostings) Iter(f func(labels.Label, Postings) error) error { func (p *MemPostings) Add(id storage.SeriesRef, lset labels.Labels) { p.mtx.Lock() - for _, l := range lset { + lset.Range(func(l labels.Label) { p.addFor(id, l) - } + }) p.addFor(id, allPostingsKey) p.mtx.Unlock() From d3d96ec887b312157504489022991a7ff673b478 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 28 Jun 2022 16:03:26 +0100 Subject: [PATCH 15/35] tsdb/index: use ScratchBuilder to create Labels This necessitates a change to the `tsdb.IndexReader` interface: `index.Reader` is used from multiple goroutines concurrently, so we can't have state in it. We do retain a `ScratchBuilder` in `blockBaseSeriesSet` which is iterator-like. Signed-off-by: Bryan Boreham --- cmd/promtool/tsdb.go | 6 ++++-- tsdb/block.go | 9 +++++---- tsdb/db_test.go | 4 +++- tsdb/head_read.go | 2 +- tsdb/head_test.go | 14 ++++++++------ tsdb/index/index.go | 12 +++++++----- tsdb/index/index_test.go | 13 ++++++++----- tsdb/querier.go | 3 ++- tsdb/querier_test.go | 7 ++++--- tsdb/repair_test.go | 5 +++-- 10 files changed, 45 insertions(+), 30 deletions(-) diff --git a/cmd/promtool/tsdb.go b/cmd/promtool/tsdb.go index 91b97f5c5..1c6ae6f6b 100644 --- a/cmd/promtool/tsdb.go +++ b/cmd/promtool/tsdb.go @@ -472,8 +472,9 @@ func analyzeBlock(path, blockID string, limit int, runExtended bool) error { } lbls := labels.Labels{} chks := []chunks.Meta{} + builder := labels.ScratchBuilder{} for p.Next() { - if err = ir.Series(p.At(), &lbls, &chks); err != nil { + if err = ir.Series(p.At(), &builder, &lbls, &chks); err != nil { return err } // Amount of the block time range not covered by this series. @@ -589,10 +590,11 @@ func analyzeCompaction(block tsdb.BlockReader, indexr tsdb.IndexReader) (err err nBuckets := 10 histogram := make([]int, nBuckets) totalChunks := 0 + var builder labels.ScratchBuilder for postingsr.Next() { lbsl := labels.Labels{} var chks []chunks.Meta - if err := indexr.Series(postingsr.At(), &lbsl, &chks); err != nil { + if err := indexr.Series(postingsr.At(), &builder, &lbsl, &chks); err != nil { return err } diff --git a/tsdb/block.go b/tsdb/block.go index b06f0940a..33413bb21 100644 --- a/tsdb/block.go +++ b/tsdb/block.go @@ -82,7 +82,7 @@ type IndexReader interface { // Series populates the given labels and chunk metas for the series identified // by the reference. // Returns storage.ErrNotFound if the ref does not resolve to a known series. - Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error + Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error // LabelNames returns all the unique label names present in the index in sorted order. LabelNames(matchers ...*labels.Matcher) ([]string, error) @@ -499,8 +499,8 @@ func (r blockIndexReader) SortedPostings(p index.Postings) index.Postings { return r.ir.SortedPostings(p) } -func (r blockIndexReader) Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error { - if err := r.ir.Series(ref, lset, chks); err != nil { +func (r blockIndexReader) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error { + if err := r.ir.Series(ref, builder, lset, chks); err != nil { return errors.Wrapf(err, "block: %s", r.b.Meta().ULID) } return nil @@ -563,10 +563,11 @@ func (pb *Block) Delete(mint, maxt int64, ms ...*labels.Matcher) error { var lset labels.Labels var chks []chunks.Meta + var builder labels.ScratchBuilder Outer: for p.Next() { - err := ir.Series(p.At(), &lset, &chks) + err := ir.Series(p.At(), &builder, &lset, &chks) if err != nil { return err } diff --git a/tsdb/db_test.go b/tsdb/db_test.go index cea4b6e36..7eb3e272e 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -1830,6 +1830,8 @@ func TestChunkAtBlockBoundary(t *testing.T) { err = db.Compact() require.NoError(t, err) + var builder labels.ScratchBuilder + for _, block := range db.Blocks() { r, err := block.Index() require.NoError(t, err) @@ -1849,7 +1851,7 @@ func TestChunkAtBlockBoundary(t *testing.T) { chunkCount := 0 for p.Next() { - err = r.Series(p.At(), &lset, &chks) + err = r.Series(p.At(), &builder, &lset, &chks) require.NoError(t, err) for _, c := range chks { require.True(t, meta.MinTime <= c.MinTime && c.MaxTime <= meta.MaxTime, diff --git a/tsdb/head_read.go b/tsdb/head_read.go index 985a15792..c6017e7b8 100644 --- a/tsdb/head_read.go +++ b/tsdb/head_read.go @@ -148,7 +148,7 @@ func (h *headIndexReader) SortedPostings(p index.Postings) index.Postings { } // Series returns the series for the given reference. -func (h *headIndexReader) Series(ref storage.SeriesRef, lbls *labels.Labels, chks *[]chunks.Meta) error { +func (h *headIndexReader) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lbls *labels.Labels, chks *[]chunks.Meta) error { s := h.head.series.getByID(chunks.HeadSeriesRef(ref)) if s == nil { diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 59824ae08..f10dc9f64 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -1446,10 +1446,11 @@ func TestGCChunkAccess(t *testing.T) { idx := h.indexRange(0, 1500) var ( - lset labels.Labels - chunks []chunks.Meta + lset labels.Labels + chunks []chunks.Meta + builder labels.ScratchBuilder ) - require.NoError(t, idx.Series(1, &lset, &chunks)) + require.NoError(t, idx.Series(1, &builder, &lset, &chunks)) require.Equal(t, labels.FromStrings("a", "1"), lset) require.Equal(t, 2, len(chunks)) @@ -1499,10 +1500,11 @@ func TestGCSeriesAccess(t *testing.T) { idx := h.indexRange(0, 2000) var ( - lset labels.Labels - chunks []chunks.Meta + lset labels.Labels + chunks []chunks.Meta + builder labels.ScratchBuilder ) - require.NoError(t, idx.Series(1, &lset, &chunks)) + require.NoError(t, idx.Series(1, &builder, &lset, &chunks)) require.Equal(t, labels.FromStrings("a", "1"), lset) require.Equal(t, 2, len(chunks)) diff --git a/tsdb/index/index.go b/tsdb/index/index.go index ba78b5712..9cf2425ec 100644 --- a/tsdb/index/index.go +++ b/tsdb/index/index.go @@ -1597,7 +1597,7 @@ func (r *Reader) LabelValueFor(id storage.SeriesRef, label string) (string, erro } // Series reads the series with the given ID and writes its labels and chunks into lbls and chks. -func (r *Reader) Series(id storage.SeriesRef, lbls *labels.Labels, chks *[]chunks.Meta) error { +func (r *Reader) Series(id storage.SeriesRef, builder *labels.ScratchBuilder, lbls *labels.Labels, chks *[]chunks.Meta) error { offset := id // In version 2 series IDs are no longer exact references but series are 16-byte padded // and the ID is the multiple of 16 of the actual position. @@ -1608,7 +1608,7 @@ func (r *Reader) Series(id storage.SeriesRef, lbls *labels.Labels, chks *[]chunk if d.Err() != nil { return d.Err() } - return errors.Wrap(r.dec.Series(d.Get(), lbls, chks), "read series") + return errors.Wrap(r.dec.Series(d.Get(), builder, lbls, chks), "read series") } func (r *Reader) Postings(name string, values ...string) (Postings, error) { @@ -1836,8 +1836,9 @@ func (dec *Decoder) LabelValueFor(b []byte, label string) (string, error) { } // Series decodes a series entry from the given byte slice into lset and chks. -func (dec *Decoder) Series(b []byte, lbls *labels.Labels, chks *[]chunks.Meta) error { - *lbls = (*lbls)[:0] +// Previous contents of lbls can be overwritten - make sure you copy before retaining. +func (dec *Decoder) Series(b []byte, builder *labels.ScratchBuilder, lbls *labels.Labels, chks *[]chunks.Meta) error { + builder.Reset() *chks = (*chks)[:0] d := encoding.Decbuf{B: b} @@ -1861,8 +1862,9 @@ func (dec *Decoder) Series(b []byte, lbls *labels.Labels, chks *[]chunks.Meta) e return errors.Wrap(err, "lookup label value") } - *lbls = append(*lbls, labels.Label{Name: ln, Value: lv}) + builder.Add(ln, lv) } + builder.Overwrite(lbls) // Read the chunks meta data. k = d.Uvarint() diff --git a/tsdb/index/index_test.go b/tsdb/index/index_test.go index 6346f2ee8..ff22fb1c0 100644 --- a/tsdb/index/index_test.go +++ b/tsdb/index/index_test.go @@ -124,7 +124,7 @@ func (m mockIndex) SortedPostings(p Postings) Postings { return NewListPostings(ep) } -func (m mockIndex) Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error { +func (m mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error { s, ok := m.series[ref] if !ok { return errors.New("not found") @@ -199,9 +199,10 @@ func TestIndexRW_Postings(t *testing.T) { var l labels.Labels var c []chunks.Meta + var builder labels.ScratchBuilder for i := 0; p.Next(); i++ { - err := ir.Series(p.At(), &l, &c) + err := ir.Series(p.At(), &builder, &l, &c) require.NoError(t, err) require.Equal(t, 0, len(c)) @@ -311,6 +312,7 @@ func TestPostingsMany(t *testing.T) { {in: []string{"126a", "126b", "127", "127a", "127b", "128", "128a", "128b", "129", "129a", "129b"}}, } + var builder labels.ScratchBuilder for _, c := range cases { it, err := ir.Postings("i", c.in...) require.NoError(t, err) @@ -319,7 +321,7 @@ func TestPostingsMany(t *testing.T) { var lbls labels.Labels var metas []chunks.Meta for it.Next() { - require.NoError(t, ir.Series(it.At(), &lbls, &metas)) + require.NoError(t, ir.Series(it.At(), &builder, &lbls, &metas)) got = append(got, lbls.Get("i")) } require.NoError(t, it.Err()) @@ -421,16 +423,17 @@ func TestPersistence_index_e2e(t *testing.T) { var lset, explset labels.Labels var chks, expchks []chunks.Meta + var builder labels.ScratchBuilder for gotp.Next() { require.True(t, expp.Next()) ref := gotp.At() - err := ir.Series(ref, &lset, &chks) + err := ir.Series(ref, &builder, &lset, &chks) require.NoError(t, err) - err = mi.Series(expp.At(), &explset, &expchks) + err = mi.Series(expp.At(), &builder, &explset, &expchks) require.NoError(t, err) require.Equal(t, explset, lset) require.Equal(t, expchks, chks) diff --git a/tsdb/querier.go b/tsdb/querier.go index 89e3d5719..4082279ae 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -452,12 +452,13 @@ type blockBaseSeriesSet struct { bufChks []chunks.Meta bufLbls labels.Labels + builder labels.ScratchBuilder err error } func (b *blockBaseSeriesSet) Next() bool { for b.p.Next() { - if err := b.index.Series(b.p.At(), &b.bufLbls, &b.bufChks); err != nil { + if err := b.index.Series(b.p.At(), &b.builder, &b.bufLbls, &b.bufChks); err != nil { // Postings may be stale. Skip if no underlying series exists. if errors.Cause(err) == storage.ErrNotFound { continue diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index e6a75da2f..9933b7158 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -1270,7 +1270,7 @@ func (m mockIndex) SortedPostings(p index.Postings) index.Postings { return index.NewListPostings(ep) } -func (m mockIndex) Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error { +func (m mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error { s, ok := m.series[ref] if !ok { return storage.ErrNotFound @@ -1884,9 +1884,10 @@ func TestPostingsForMatchers(t *testing.T) { p, err := PostingsForMatchers(ir, c.matchers...) require.NoError(t, err) + var builder labels.ScratchBuilder for p.Next() { lbls := labels.Labels{} - require.NoError(t, ir.Series(p.At(), &lbls, &[]chunks.Meta{})) + require.NoError(t, ir.Series(p.At(), &builder, &lbls, &[]chunks.Meta{})) if _, ok := exp[lbls.String()]; !ok { t.Errorf("Evaluating %v, unexpected result %s", c.matchers, lbls.String()) } else { @@ -2097,7 +2098,7 @@ func (m mockMatcherIndex) SortedPostings(p index.Postings) index.Postings { return index.EmptyPostings() } -func (m mockMatcherIndex) Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error { +func (m mockMatcherIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error { return nil } diff --git a/tsdb/repair_test.go b/tsdb/repair_test.go index 6d95fc0a6..d39411560 100644 --- a/tsdb/repair_test.go +++ b/tsdb/repair_test.go @@ -80,11 +80,12 @@ func TestRepairBadIndexVersion(t *testing.T) { require.NoError(t, err) p, err := r.Postings("b", "1") require.NoError(t, err) + var builder labels.ScratchBuilder for p.Next() { t.Logf("next ID %d", p.At()) var lset labels.Labels - require.Error(t, r.Series(p.At(), &lset, nil)) + require.Error(t, r.Series(p.At(), &builder, &lset, nil)) } require.NoError(t, p.Err()) require.NoError(t, r.Close()) @@ -106,7 +107,7 @@ func TestRepairBadIndexVersion(t *testing.T) { var lset labels.Labels var chks []chunks.Meta - require.NoError(t, r.Series(p.At(), &lset, &chks)) + require.NoError(t, r.Series(p.At(), &builder, &lset, &chks)) res = append(res, lset) } From a5bdff414b8d7b1925c5634c688f415262880de3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:18:37 +0000 Subject: [PATCH 16/35] Update package tsdb/index tests for new labels.Labels type Note in one cases we needed an extra copy of labels in case they change. Signed-off-by: Bryan Boreham --- tsdb/index/index_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tsdb/index/index_test.go b/tsdb/index/index_test.go index ff22fb1c0..e6b2e890c 100644 --- a/tsdb/index/index_test.go +++ b/tsdb/index/index_test.go @@ -68,14 +68,14 @@ func (m mockIndex) AddSeries(ref storage.SeriesRef, l labels.Labels, chunks ...c if _, ok := m.series[ref]; ok { return errors.Errorf("series with reference %d already added", ref) } - for _, lbl := range l { + l.Range(func(lbl labels.Label) { m.symbols[lbl.Name] = struct{}{} m.symbols[lbl.Value] = struct{}{} if _, ok := m.postings[lbl]; !ok { m.postings[lbl] = []storage.SeriesRef{} } m.postings[lbl] = append(m.postings[lbl], ref) - } + }) m.postings[allPostingsKey] = append(m.postings[allPostingsKey], ref) s := series{l: l} @@ -129,7 +129,7 @@ func (m mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, if !ok { return errors.New("not found") } - *lset = append((*lset)[:0], s.l...) + lset.CopyFrom(s.l) *chks = append((*chks)[:0], s.chunks...) return nil @@ -322,7 +322,7 @@ func TestPostingsMany(t *testing.T) { var metas []chunks.Meta for it.Next() { require.NoError(t, ir.Series(it.At(), &builder, &lbls, &metas)) - got = append(got, lbls.Get("i")) + got = append(got, lbls.Copy().Get("i")) } require.NoError(t, it.Err()) exp := []string{} @@ -346,10 +346,10 @@ func TestPersistence_index_e2e(t *testing.T) { symbols := map[string]struct{}{} for _, lset := range lbls { - for _, l := range lset { + lset.Range(func(l labels.Label) { symbols[l.Name] = struct{}{} symbols[l.Value] = struct{}{} - } + }) } var input indexWriterSeriesSlice @@ -397,14 +397,14 @@ func TestPersistence_index_e2e(t *testing.T) { require.NoError(t, err) require.NoError(t, mi.AddSeries(storage.SeriesRef(i), s.labels, s.chunks...)) - for _, l := range s.labels { + s.labels.Range(func(l labels.Label) { valset, ok := values[l.Name] if !ok { valset = map[string]struct{}{} values[l.Name] = valset } valset[l.Value] = struct{}{} - } + }) postings.Add(storage.SeriesRef(i), s.labels) } From 14ad2e780be7e2f3ff49737a424ddc359f4adb1b Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 20 Feb 2022 20:42:11 +0000 Subject: [PATCH 17/35] Update package tsdb/agent for new labels.Labels type Signed-off-by: Bryan Boreham --- tsdb/agent/db.go | 10 ++++++--- tsdb/agent/db_test.go | 52 +++++++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/tsdb/agent/db.go b/tsdb/agent/db.go index 0675d5a28..7e959573b 100644 --- a/tsdb/agent/db.go +++ b/tsdb/agent/db.go @@ -713,7 +713,7 @@ func (a *appender) Append(ref storage.SeriesRef, l labels.Labels, t int64, v flo // Ensure no empty or duplicate labels have gotten through. This mirrors the // equivalent validation code in the TSDB's headAppender. l = l.WithoutEmpty() - if len(l) == 0 { + if l.IsEmpty() { return 0, errors.Wrap(tsdb.ErrInvalidSample, "empty labelset") } @@ -786,13 +786,17 @@ func (a *appender) AppendExemplar(ref storage.SeriesRef, l labels.Labels, e exem // Exemplar label length does not include chars involved in text rendering such as quotes // equals sign, or commas. See definition of const ExemplarMaxLabelLength. labelSetLen := 0 - for _, l := range e.Labels { + err := e.Labels.Validate(func(l labels.Label) error { labelSetLen += utf8.RuneCountInString(l.Name) labelSetLen += utf8.RuneCountInString(l.Value) if labelSetLen > exemplar.ExemplarMaxLabelSetLength { - return 0, storage.ErrExemplarLabelLength + return storage.ErrExemplarLabelLength } + return nil + }) + if err != nil { + return 0, err } // Check for duplicate vs last stored exemplar for this series, and discard those. diff --git a/tsdb/agent/db_test.go b/tsdb/agent/db_test.go index c32e901e6..5933944de 100644 --- a/tsdb/agent/db_test.go +++ b/tsdb/agent/db_test.go @@ -49,28 +49,28 @@ func TestDB_InvalidSeries(t *testing.T) { _, err := app.Append(0, labels.Labels{}, 0, 0) require.ErrorIs(t, err, tsdb.ErrInvalidSample, "should reject empty labels") - _, err = app.Append(0, labels.Labels{{Name: "a", Value: "1"}, {Name: "a", Value: "2"}}, 0, 0) + _, err = app.Append(0, labels.FromStrings("a", "1", "a", "2"), 0, 0) require.ErrorIs(t, err, tsdb.ErrInvalidSample, "should reject duplicate labels") }) t.Run("Exemplars", func(t *testing.T) { - sRef, err := app.Append(0, labels.Labels{{Name: "a", Value: "1"}}, 0, 0) + sRef, err := app.Append(0, labels.FromStrings("a", "1"), 0, 0) require.NoError(t, err, "should not reject valid series") - _, err = app.AppendExemplar(0, nil, exemplar.Exemplar{}) + _, err = app.AppendExemplar(0, labels.EmptyLabels(), exemplar.Exemplar{}) require.EqualError(t, err, "unknown series ref when trying to add exemplar: 0") - e := exemplar.Exemplar{Labels: labels.Labels{{Name: "a", Value: "1"}, {Name: "a", Value: "2"}}} - _, err = app.AppendExemplar(sRef, nil, e) + e := exemplar.Exemplar{Labels: labels.FromStrings("a", "1", "a", "2")} + _, err = app.AppendExemplar(sRef, labels.EmptyLabels(), e) require.ErrorIs(t, err, tsdb.ErrInvalidExemplar, "should reject duplicate labels") - e = exemplar.Exemplar{Labels: labels.Labels{{Name: "a_somewhat_long_trace_id", Value: "nYJSNtFrFTY37VR7mHzEE/LIDt7cdAQcuOzFajgmLDAdBSRHYPDzrxhMA4zz7el8naI/AoXFv9/e/G0vcETcIoNUi3OieeLfaIRQci2oa"}}} - _, err = app.AppendExemplar(sRef, nil, e) + e = exemplar.Exemplar{Labels: labels.FromStrings("a_somewhat_long_trace_id", "nYJSNtFrFTY37VR7mHzEE/LIDt7cdAQcuOzFajgmLDAdBSRHYPDzrxhMA4zz7el8naI/AoXFv9/e/G0vcETcIoNUi3OieeLfaIRQci2oa")} + _, err = app.AppendExemplar(sRef, labels.EmptyLabels(), e) require.ErrorIs(t, err, storage.ErrExemplarLabelLength, "should reject too long label length") // Inverse check - e = exemplar.Exemplar{Labels: labels.Labels{{Name: "a", Value: "1"}}, Value: 20, Ts: 10, HasTs: true} - _, err = app.AppendExemplar(sRef, nil, e) + e = exemplar.Exemplar{Labels: labels.FromStrings("a", "1"), Value: 20, Ts: 10, HasTs: true} + _, err = app.AppendExemplar(sRef, labels.EmptyLabels(), e) require.NoError(t, err, "should not reject valid exemplars") }) } @@ -426,9 +426,7 @@ func Test_ExistingWAL_NextRef(t *testing.T) { // Append series app := db.Appender(context.Background()) for i := 0; i < seriesCount; i++ { - lset := labels.Labels{ - {Name: model.MetricNameLabel, Value: fmt.Sprintf("series_%d", i)}, - } + lset := labels.FromStrings(model.MetricNameLabel, fmt.Sprintf("series_%d", i)) _, err := app.Append(0, lset, 0, 100) require.NoError(t, err) } @@ -470,11 +468,11 @@ func startTime() (int64, error) { } // Create series for tests. -func labelsForTest(lName string, seriesCount int) []labels.Labels { - var series []labels.Labels +func labelsForTest(lName string, seriesCount int) [][]labels.Label { + var series [][]labels.Label for i := 0; i < seriesCount; i++ { - lset := labels.Labels{ + lset := []labels.Label{ {Name: "a", Value: lName}, {Name: "instance", Value: "localhost" + strconv.Itoa(i)}, {Name: "job", Value: "prometheus"}, @@ -507,28 +505,28 @@ func TestStorage_DuplicateExemplarsIgnored(t *testing.T) { app := s.Appender(context.Background()) defer s.Close() - sRef, err := app.Append(0, labels.Labels{{Name: "a", Value: "1"}}, 0, 0) + sRef, err := app.Append(0, labels.FromStrings("a", "1"), 0, 0) require.NoError(t, err, "should not reject valid series") // Write a few exemplars to our appender and call Commit(). // If the Labels, Value or Timestamp are different than the last exemplar, // then a new one should be appended; Otherwise, it should be skipped. - e := exemplar.Exemplar{Labels: labels.Labels{{Name: "a", Value: "1"}}, Value: 20, Ts: 10, HasTs: true} - _, _ = app.AppendExemplar(sRef, nil, e) - _, _ = app.AppendExemplar(sRef, nil, e) + e := exemplar.Exemplar{Labels: labels.FromStrings("a", "1"), Value: 20, Ts: 10, HasTs: true} + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) - e.Labels = labels.Labels{{Name: "b", Value: "2"}} - _, _ = app.AppendExemplar(sRef, nil, e) - _, _ = app.AppendExemplar(sRef, nil, e) - _, _ = app.AppendExemplar(sRef, nil, e) + e.Labels = labels.FromStrings("b", "2") + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) e.Value = 42 - _, _ = app.AppendExemplar(sRef, nil, e) - _, _ = app.AppendExemplar(sRef, nil, e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) e.Ts = 25 - _, _ = app.AppendExemplar(sRef, nil, e) - _, _ = app.AppendExemplar(sRef, nil, e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) + _, _ = app.AppendExemplar(sRef, labels.EmptyLabels(), e) require.NoError(t, app.Commit()) From f0ec81baddf34cfb2c466816afed6764b52a8b67 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 20 Feb 2022 20:44:25 +0000 Subject: [PATCH 18/35] Update package tsdb/test for new labels.Labels type Signed-off-by: Bryan Boreham --- tsdb/test/labels_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tsdb/test/labels_test.go b/tsdb/test/labels_test.go index 354dbf836..f1ec34f3a 100644 --- a/tsdb/test/labels_test.go +++ b/tsdb/test/labels_test.go @@ -58,9 +58,7 @@ func BenchmarkLabelsClone(b *testing.B) { l := labels.FromMap(m) for i := 0; i < b.N; i++ { - res := make(labels.Labels, len(l)) - copy(res, l) - l = res + l = l.Copy() } } @@ -106,13 +104,13 @@ func BenchmarkLabelSetAccess(b *testing.B) { var v string - for _, l := range ls { + ls.Range(func(l labels.Label) { b.Run(l.Name, func(b *testing.B) { for i := 0; i < b.N; i++ { v = ls.Get(l.Name) } }) - } + }) _ = v } From 543c318ec2054e0be680ad28671b6d30efa314e2 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:17:40 +0000 Subject: [PATCH 19/35] Update package tsdb for new labels.Labels type Signed-off-by: Bryan Boreham --- tsdb/db.go | 2 +- tsdb/exemplar.go | 5 ++++- tsdb/head_append.go | 8 ++++---- tsdb/head_read.go | 6 +++--- tsdb/ooo_head_read.go | 6 +++--- tsdb/querier.go | 5 +---- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tsdb/db.go b/tsdb/db.go index 7a165e431..616213b03 100644 --- a/tsdb/db.go +++ b/tsdb/db.go @@ -1002,7 +1002,7 @@ func (a dbAppender) GetRef(lset labels.Labels, hash uint64) (storage.SeriesRef, if g, ok := a.Appender.(storage.GetRef); ok { return g.GetRef(lset, hash) } - return 0, nil + return 0, labels.EmptyLabels() } func (a dbAppender) Commit() error { diff --git a/tsdb/exemplar.go b/tsdb/exemplar.go index 3718d9591..5ba3567e4 100644 --- a/tsdb/exemplar.go +++ b/tsdb/exemplar.go @@ -226,13 +226,16 @@ func (ce *CircularExemplarStorage) validateExemplar(key []byte, e exemplar.Exemp // Exemplar label length does not include chars involved in text rendering such as quotes // equals sign, or commas. See definition of const ExemplarMaxLabelLength. labelSetLen := 0 - for _, l := range e.Labels { + if err := e.Labels.Validate(func(l labels.Label) error { labelSetLen += utf8.RuneCountInString(l.Name) labelSetLen += utf8.RuneCountInString(l.Value) if labelSetLen > exemplar.ExemplarMaxLabelSetLength { return storage.ErrExemplarLabelLength } + return nil + }); err != nil { + return err } idx, ok := ce.index[string(key)] diff --git a/tsdb/head_append.go b/tsdb/head_append.go index d5003188d..822863276 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -102,7 +102,7 @@ func (a *initAppender) GetRef(lset labels.Labels, hash uint64) (storage.SeriesRe if g, ok := a.app.(storage.GetRef); ok { return g.GetRef(lset, hash) } - return 0, nil + return 0, labels.EmptyLabels() } func (a *initAppender) Commit() error { @@ -312,7 +312,7 @@ func (a *headAppender) Append(ref storage.SeriesRef, lset labels.Labels, t int64 if s == nil { // Ensure no empty labels have gotten through. lset = lset.WithoutEmpty() - if len(lset) == 0 { + if lset.IsEmpty() { return 0, errors.Wrap(ErrInvalidSample, "empty labelset") } @@ -494,7 +494,7 @@ func (a *headAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels if s == nil { // Ensure no empty labels have gotten through. lset = lset.WithoutEmpty() - if len(lset) == 0 { + if lset.IsEmpty() { return 0, errors.Wrap(ErrInvalidSample, "empty labelset") } @@ -650,7 +650,7 @@ var _ storage.GetRef = &headAppender{} func (a *headAppender) GetRef(lset labels.Labels, hash uint64) (storage.SeriesRef, labels.Labels) { s := a.head.series.getByHash(hash, lset) if s == nil { - return 0, nil + return 0, labels.EmptyLabels() } // returned labels must be suitable to pass to Append() return storage.SeriesRef(s.ref), s.lset diff --git a/tsdb/head_read.go b/tsdb/head_read.go index c6017e7b8..0a6aef2bd 100644 --- a/tsdb/head_read.go +++ b/tsdb/head_read.go @@ -155,7 +155,7 @@ func (h *headIndexReader) Series(ref storage.SeriesRef, builder *labels.ScratchB h.head.metrics.seriesNotFound.Inc() return storage.ErrNotFound } - *lbls = append((*lbls)[:0], s.lset...) + lbls.CopyFrom(s.lset) s.Lock() defer s.Unlock() @@ -222,9 +222,9 @@ func (h *headIndexReader) LabelNamesFor(ids ...storage.SeriesRef) ([]string, err if memSeries == nil { return nil, storage.ErrNotFound } - for _, lbl := range memSeries.lset { + memSeries.lset.Range(func(lbl labels.Label) { namesMap[lbl.Name] = struct{}{} - } + }) } names := make([]string, 0, len(namesMap)) for name := range namesMap { diff --git a/tsdb/ooo_head_read.go b/tsdb/ooo_head_read.go index f63607dc9..d0b09963d 100644 --- a/tsdb/ooo_head_read.go +++ b/tsdb/ooo_head_read.go @@ -47,7 +47,7 @@ func NewOOOHeadIndexReader(head *Head, mint, maxt int64) *OOOHeadIndexReader { return &OOOHeadIndexReader{hr} } -func (oh *OOOHeadIndexReader) Series(ref storage.SeriesRef, lbls *labels.Labels, chks *[]chunks.Meta) error { +func (oh *OOOHeadIndexReader) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lbls *labels.Labels, chks *[]chunks.Meta) error { return oh.series(ref, lbls, chks, 0) } @@ -61,7 +61,7 @@ func (oh *OOOHeadIndexReader) series(ref storage.SeriesRef, lbls *labels.Labels, oh.head.metrics.seriesNotFound.Inc() return storage.ErrNotFound } - *lbls = append((*lbls)[:0], s.lset...) + lbls.CopyFrom(s.lset) if chks == nil { return nil @@ -400,7 +400,7 @@ func (ir *OOOCompactionHeadIndexReader) SortedPostings(p index.Postings) index.P return p } -func (ir *OOOCompactionHeadIndexReader) Series(ref storage.SeriesRef, lset *labels.Labels, chks *[]chunks.Meta) error { +func (ir *OOOCompactionHeadIndexReader) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, lset *labels.Labels, chks *[]chunks.Meta) error { return ir.ch.oooIR.series(ref, lset, chks, ir.ch.lastMmapRef) } diff --git a/tsdb/querier.go b/tsdb/querier.go index 4082279ae..34917dad0 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -529,8 +529,7 @@ func (b *blockBaseSeriesSet) Next() bool { intervals = intervals.Add(tombstones.Interval{Mint: b.maxt + 1, Maxt: math.MaxInt64}) } - b.curr.labels = make(labels.Labels, len(b.bufLbls)) - copy(b.curr.labels, b.bufLbls) + b.curr.labels = b.bufLbls.Copy() b.curr.chks = chks b.curr.intervals = intervals @@ -866,7 +865,6 @@ func newBlockSeriesSet(i IndexReader, c ChunkReader, t tombstones.Reader, p inde mint: mint, maxt: maxt, disableTrimming: disableTrimming, - bufLbls: make(labels.Labels, 0, 10), }, } } @@ -898,7 +896,6 @@ func newBlockChunkSeriesSet(id ulid.ULID, i IndexReader, c ChunkReader, t tombst mint: mint, maxt: maxt, disableTrimming: disableTrimming, - bufLbls: make(labels.Labels, 0, 10), }, } } From ce2cfad0cba11a37d1313b6b72675f622e4ce64a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 28 Jun 2022 16:02:08 +0100 Subject: [PATCH 20/35] Update package tsdb/record for new labels.Labels type Implement decoding via labels.ScratchBuilder, which we retain and re-use to reduce memory allocations. Signed-off-by: Bryan Boreham --- tsdb/record/record.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tsdb/record/record.go b/tsdb/record/record.go index 86cc93890..a1f05425f 100644 --- a/tsdb/record/record.go +++ b/tsdb/record/record.go @@ -17,7 +17,6 @@ package record import ( "math" - "sort" "github.com/pkg/errors" @@ -182,7 +181,9 @@ type RefMmapMarker struct { // Decoder decodes series, sample, metadata and tombstone records. // The zero value is ready to use. -type Decoder struct{} +type Decoder struct { + builder labels.ScratchBuilder +} // Type returns the type of the record. // Returns RecordUnknown if no valid record type is found. @@ -267,14 +268,15 @@ func (d *Decoder) Metadata(rec []byte, metadata []RefMetadata) ([]RefMetadata, e // DecodeLabels decodes one set of labels from buf. func (d *Decoder) DecodeLabels(dec *encoding.Decbuf) labels.Labels { - lset := make(labels.Labels, dec.Uvarint()) - - for i := range lset { - lset[i].Name = dec.UvarintStr() - lset[i].Value = dec.UvarintStr() + // TODO: reconsider if this function could be pushed down into labels.Labels to be more efficient. + d.builder.Reset() + nLabels := dec.Uvarint() + for i := 0; i < nLabels; i++ { + lName := dec.UvarintStr() + lValue := dec.UvarintStr() + d.builder.Add(lName, lValue) } - sort.Sort(lset) - return lset + return d.builder.Labels() } // Samples appends samples in rec to the given slice. @@ -525,12 +527,13 @@ func (e *Encoder) Metadata(metadata []RefMetadata, b []byte) []byte { // EncodeLabels encodes the contents of labels into buf. func EncodeLabels(buf *encoding.Encbuf, lbls labels.Labels) { - buf.PutUvarint(len(lbls)) + // TODO: reconsider if this function could be pushed down into labels.Labels to be more efficient. + buf.PutUvarint(lbls.Len()) - for _, l := range lbls { + lbls.Range(func(l labels.Label) { buf.PutUvarintStr(l.Name) buf.PutUvarintStr(l.Value) - } + }) } // Samples appends the encoded samples to b and returns the resulting slice. From 4b6a4d142510444730e3589a04f9d220e49ac03c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:17:29 +0000 Subject: [PATCH 21/35] Update package tsdb tests for new labels.Labels type Signed-off-by: Bryan Boreham --- tsdb/block_test.go | 42 +++++++++++++++++++------------------- tsdb/compact_test.go | 19 ++++++++--------- tsdb/db_test.go | 10 ++++----- tsdb/head_test.go | 24 ++++++++++++++-------- tsdb/ooo_head_read_test.go | 11 ++++++---- tsdb/querier_test.go | 27 +++++++++++++----------- 6 files changed, 72 insertions(+), 61 deletions(-) diff --git a/tsdb/block_test.go b/tsdb/block_test.go index c3a6ff576..380fa68bb 100644 --- a/tsdb/block_test.go +++ b/tsdb/block_test.go @@ -215,10 +215,10 @@ func TestLabelValuesWithMatchers(t *testing.T) { var seriesEntries []storage.Series for i := 0; i < 100; i++ { - seriesEntries = append(seriesEntries, storage.NewListSeries(labels.Labels{ - {Name: "tens", Value: fmt.Sprintf("value%d", i/10)}, - {Name: "unique", Value: fmt.Sprintf("value%d", i)}, - }, []tsdbutil.Sample{sample{100, 0, nil, nil}})) + seriesEntries = append(seriesEntries, storage.NewListSeries(labels.FromStrings( + "tens", fmt.Sprintf("value%d", i/10), + "unique", fmt.Sprintf("value%d", i), + ), []tsdbutil.Sample{sample{100, 0, nil, nil}})) } blockDir := createBlock(t, tmpdir, seriesEntries) @@ -372,11 +372,11 @@ func BenchmarkLabelValuesWithMatchers(b *testing.B) { for i := 0; i < metricCount; i++ { // Note these series are not created in sort order: 'value2' sorts after 'value10'. // This makes a big difference to the benchmark timing. - seriesEntries = append(seriesEntries, storage.NewListSeries(labels.Labels{ - {Name: "a_unique", Value: fmt.Sprintf("value%d", i)}, - {Name: "b_tens", Value: fmt.Sprintf("value%d", i/(metricCount/10))}, - {Name: "c_ninety", Value: fmt.Sprintf("value%d", i/(metricCount/10)/9)}, // "0" for the first 90%, then "1" - }, []tsdbutil.Sample{sample{100, 0, nil, nil}})) + seriesEntries = append(seriesEntries, storage.NewListSeries(labels.FromStrings( + "a_unique", fmt.Sprintf("value%d", i), + "b_tens", fmt.Sprintf("value%d", i/(metricCount/10)), + "c_ninety", fmt.Sprintf("value%d", i/(metricCount/10)/9), // "0" for the first 90%, then "1" + ), []tsdbutil.Sample{sample{100, 0, nil, nil}})) } blockDir := createBlock(b, tmpdir, seriesEntries) @@ -410,23 +410,23 @@ func TestLabelNamesWithMatchers(t *testing.T) { var seriesEntries []storage.Series for i := 0; i < 100; i++ { - seriesEntries = append(seriesEntries, storage.NewListSeries(labels.Labels{ - {Name: "unique", Value: fmt.Sprintf("value%d", i)}, - }, []tsdbutil.Sample{sample{100, 0, nil, nil}})) + seriesEntries = append(seriesEntries, storage.NewListSeries(labels.FromStrings( + "unique", fmt.Sprintf("value%d", i), + ), []tsdbutil.Sample{sample{100, 0, nil, nil}})) if i%10 == 0 { - seriesEntries = append(seriesEntries, storage.NewListSeries(labels.Labels{ - {Name: "tens", Value: fmt.Sprintf("value%d", i/10)}, - {Name: "unique", Value: fmt.Sprintf("value%d", i)}, - }, []tsdbutil.Sample{sample{100, 0, nil, nil}})) + seriesEntries = append(seriesEntries, storage.NewListSeries(labels.FromStrings( + "tens", fmt.Sprintf("value%d", i/10), + "unique", fmt.Sprintf("value%d", i), + ), []tsdbutil.Sample{sample{100, 0, nil, nil}})) } if i%20 == 0 { - seriesEntries = append(seriesEntries, storage.NewListSeries(labels.Labels{ - {Name: "tens", Value: fmt.Sprintf("value%d", i/10)}, - {Name: "twenties", Value: fmt.Sprintf("value%d", i/20)}, - {Name: "unique", Value: fmt.Sprintf("value%d", i)}, - }, []tsdbutil.Sample{sample{100, 0, nil, nil}})) + seriesEntries = append(seriesEntries, storage.NewListSeries(labels.FromStrings( + "tens", fmt.Sprintf("value%d", i/10), + "twenties", fmt.Sprintf("value%d", i/20), + "unique", fmt.Sprintf("value%d", i), + ), []tsdbutil.Sample{sample{100, 0, nil, nil}})) } } diff --git a/tsdb/compact_test.go b/tsdb/compact_test.go index a77d070e8..e7fe998ac 100644 --- a/tsdb/compact_test.go +++ b/tsdb/compact_test.go @@ -1478,11 +1478,11 @@ func TestSparseHistogramSpaceSavings(t *testing.T) { for sid, schema := range allSchemas { for i := 0; i < c.numSeriesPerSchema; i++ { - lbls := labels.Labels{ - {Name: "__name__", Value: fmt.Sprintf("rpc_durations_%d_histogram_seconds", i)}, - {Name: "instance", Value: "localhost:8080"}, - {Name: "job", Value: fmt.Sprintf("sparse_histogram_schema_%s", schemaDescription[sid])}, - } + lbls := labels.FromStrings( + "__name__", fmt.Sprintf("rpc_durations_%d_histogram_seconds", i), + "instance", "localhost:8080", + "job", fmt.Sprintf("sparse_histogram_schema_%s", schemaDescription[sid]), + ) allSparseSeries = append(allSparseSeries, struct { baseLabels labels.Labels hists []*histogram.Histogram @@ -1546,21 +1546,20 @@ func TestSparseHistogramSpaceSavings(t *testing.T) { for it.Next() { numOldSeriesPerHistogram++ b := it.At() - lbls := append(ah.baseLabels, labels.Label{Name: "le", Value: fmt.Sprintf("%.16f", b.Upper)}) + lbls := labels.NewBuilder(ah.baseLabels).Set("le", fmt.Sprintf("%.16f", b.Upper)).Labels(labels.EmptyLabels()) refs[itIdx], err = oldApp.Append(refs[itIdx], lbls, ts, float64(b.Count)) require.NoError(t, err) itIdx++ } + baseName := ah.baseLabels.Get(labels.MetricName) // _count metric. - countLbls := ah.baseLabels.Copy() - countLbls[0].Value = countLbls[0].Value + "_count" + countLbls := labels.NewBuilder(ah.baseLabels).Set(labels.MetricName, baseName+"_count").Labels(labels.EmptyLabels()) _, err = oldApp.Append(0, countLbls, ts, float64(h.Count)) require.NoError(t, err) numOldSeriesPerHistogram++ // _sum metric. - sumLbls := ah.baseLabels.Copy() - sumLbls[0].Value = sumLbls[0].Value + "_sum" + sumLbls := labels.NewBuilder(ah.baseLabels).Set(labels.MetricName, baseName+"_sum").Labels(labels.EmptyLabels()) _, err = oldApp.Append(0, sumLbls, ts, h.Sum) require.NoError(t, err) numOldSeriesPerHistogram++ diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 7eb3e272e..1d95cfe00 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -478,9 +478,9 @@ func TestAmendDatapointCausesError(t *testing.T) { require.NoError(t, app.Commit()) app = db.Appender(ctx) - _, err = app.Append(0, labels.Labels{{Name: "a", Value: "b"}}, 0, 0) + _, err = app.Append(0, labels.FromStrings("a", "b"), 0, 0) require.NoError(t, err) - _, err = app.Append(0, labels.Labels{{Name: "a", Value: "b"}}, 0, 1) + _, err = app.Append(0, labels.FromStrings("a", "b"), 0, 1) require.Equal(t, storage.ErrDuplicateSampleForTimestamp, err) require.NoError(t, app.Rollback()) @@ -498,15 +498,15 @@ func TestAmendDatapointCausesError(t *testing.T) { } app = db.Appender(ctx) - _, err = app.AppendHistogram(0, labels.Labels{{Name: "a", Value: "c"}}, 0, h.Copy()) + _, err = app.AppendHistogram(0, labels.FromStrings("a", "c"), 0, h.Copy()) require.NoError(t, err) require.NoError(t, app.Commit()) app = db.Appender(ctx) - _, err = app.AppendHistogram(0, labels.Labels{{Name: "a", Value: "c"}}, 0, h.Copy()) + _, err = app.AppendHistogram(0, labels.FromStrings("a", "c"), 0, h.Copy()) require.NoError(t, err) h.Schema = 2 - _, err = app.AppendHistogram(0, labels.Labels{{Name: "a", Value: "c"}}, 0, h.Copy()) + _, err = app.AppendHistogram(0, labels.FromStrings("a", "c"), 0, h.Copy()) require.Equal(t, storage.ErrDuplicateSampleForTimestamp, err) require.NoError(t, app.Rollback()) } diff --git a/tsdb/head_test.go b/tsdb/head_test.go index f10dc9f64..cc0133b9d 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -388,7 +388,12 @@ func TestHead_HighConcurrencyReadAndWrite(t *testing.T) { querySeriesRef = (querySeriesRef + 1) % seriesCnt lbls := labelSets[querySeriesRef] - samples, err := queryHead(ts-qryRange, ts, lbls[0]) + // lbls has a single entry; extract it so we can run a query. + var lbl labels.Label + lbls.Range(func(l labels.Label) { + lbl = l + }) + samples, err := queryHead(ts-qryRange, ts, lbl) if err != nil { return false, err } @@ -1133,8 +1138,9 @@ func TestDelete_e2e(t *testing.T) { require.NoError(t, hb.Delete(r.Mint, r.Maxt, del.ms...)) } matched := labels.Slice{} - for _, ls := range lbls { + for _, l := range lbls { s := labels.Selector(del.ms) + ls := labels.New(l...) if s.Matches(ls) { matched = append(matched, ls) } @@ -2808,7 +2814,7 @@ func TestWaitForPendingReadersInTimeRange(t *testing.T) { } func TestAppendHistogram(t *testing.T) { - l := labels.Labels{{Name: "a", Value: "b"}} + l := labels.FromStrings("a", "b") for _, numHistograms := range []int{1, 10, 150, 200, 250, 300} { t.Run(fmt.Sprintf("%d", numHistograms), func(t *testing.T) { head, _ := newTestHead(t, 1000, false, false) @@ -2863,7 +2869,7 @@ func TestHistogramInWALAndMmapChunk(t *testing.T) { require.NoError(t, head.Init(0)) // Series with only histograms. - s1 := labels.Labels{{Name: "a", Value: "b1"}} + s1 := labels.FromStrings("a", "b1") k1 := s1.String() numHistograms := 450 exp := map[string][]tsdbutil.Sample{} @@ -2895,7 +2901,7 @@ func TestHistogramInWALAndMmapChunk(t *testing.T) { require.Greater(t, expHeadChunkSamples, 0) // Series with mix of histograms and float. - s2 := labels.Labels{{Name: "a", Value: "b2"}} + s2 := labels.FromStrings("a", "b2") k2 := s2.String() app = head.Appender(context.Background()) ts := 0 @@ -3256,7 +3262,7 @@ func TestHistogramMetrics(t *testing.T) { for x := 0; x < 5; x++ { expHSeries++ - l := labels.Labels{{Name: "a", Value: fmt.Sprintf("b%d", x)}} + l := labels.FromStrings("a", fmt.Sprintf("b%d", x)) for i, h := range GenerateTestHistograms(10) { app := head.Appender(context.Background()) _, err := app.AppendHistogram(0, l, int64(i), h) @@ -3279,7 +3285,7 @@ func TestHistogramMetrics(t *testing.T) { } func TestHistogramStaleSample(t *testing.T) { - l := labels.Labels{{Name: "a", Value: "b"}} + l := labels.FromStrings("a", "b") numHistograms := 20 head, _ := newTestHead(t, 100000, false, false) t.Cleanup(func() { @@ -3374,7 +3380,7 @@ func TestHistogramStaleSample(t *testing.T) { } func TestHistogramCounterResetHeader(t *testing.T) { - l := labels.Labels{{Name: "a", Value: "b"}} + l := labels.FromStrings("a", "b") head, _ := newTestHead(t, 1000, false, false) t.Cleanup(func() { require.NoError(t, head.Close()) @@ -3486,7 +3492,7 @@ func TestAppendingDifferentEncodingToSameSeries(t *testing.T) { db.DisableCompactions() hists := GenerateTestHistograms(10) - lbls := labels.Labels{{Name: "a", Value: "b"}} + lbls := labels.FromStrings("a", "b") type result struct { t int64 diff --git a/tsdb/ooo_head_read_test.go b/tsdb/ooo_head_read_test.go index b489c6e57..c48e95e46 100644 --- a/tsdb/ooo_head_read_test.go +++ b/tsdb/ooo_head_read_test.go @@ -358,12 +358,13 @@ func TestOOOHeadIndexReader_Series(t *testing.T) { var chks []chunks.Meta var respLset labels.Labels - err := ir.Series(storage.SeriesRef(s1ID), &respLset, &chks) + var b labels.ScratchBuilder + err := ir.Series(storage.SeriesRef(s1ID), &b, &respLset, &chks) require.NoError(t, err) require.Equal(t, s1Lset, respLset) require.Equal(t, expChunks, chks) - err = ir.Series(storage.SeriesRef(s1ID+1), &respLset, &chks) + err = ir.Series(storage.SeriesRef(s1ID+1), &b, &respLset, &chks) require.Equal(t, storage.ErrNotFound, err) }) } @@ -841,7 +842,8 @@ func TestOOOHeadChunkReader_Chunk(t *testing.T) { ir := NewOOOHeadIndexReader(db.head, tc.queryMinT, tc.queryMaxT) var chks []chunks.Meta var respLset labels.Labels - err := ir.Series(s1Ref, &respLset, &chks) + var b labels.ScratchBuilder + err := ir.Series(s1Ref, &b, &respLset, &chks) require.NoError(t, err) require.Equal(t, len(tc.expChunksSamples), len(chks)) @@ -1004,7 +1006,8 @@ func TestOOOHeadChunkReader_Chunk_ConsistentQueryResponseDespiteOfHeadExpanding( ir := NewOOOHeadIndexReader(db.head, tc.queryMinT, tc.queryMaxT) var chks []chunks.Meta var respLset labels.Labels - err := ir.Series(s1Ref, &respLset, &chks) + var b labels.ScratchBuilder + err := ir.Series(s1Ref, &b, &respLset, &chks) require.NoError(t, err) require.Equal(t, len(tc.expChunksSamples), len(chks)) diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index 9933b7158..eb0c847fd 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -142,14 +142,14 @@ func createIdxChkReaders(t *testing.T, tc []seriesSamples) (IndexReader, ChunkRe postings.Add(storage.SeriesRef(i), ls) - for _, l := range ls { + ls.Range(func(l labels.Label) { vs, present := lblIdx[l.Name] if !present { vs = map[string]struct{}{} lblIdx[l.Name] = vs } vs[l.Value] = struct{}{} - } + }) } require.NoError(t, postings.Iter(func(l labels.Label, p index.Postings) error { @@ -1168,10 +1168,10 @@ func (m *mockIndex) AddSeries(ref storage.SeriesRef, l labels.Labels, chunks ... if _, ok := m.series[ref]; ok { return errors.Errorf("series with reference %d already added", ref) } - for _, lbl := range l { + l.Range(func(lbl labels.Label) { m.symbols[lbl.Name] = struct{}{} m.symbols[lbl.Value] = struct{}{} - } + }) s := series{l: l} // Actual chunk data is not stored in the index. @@ -1238,9 +1238,9 @@ func (m mockIndex) LabelValueFor(id storage.SeriesRef, label string) (string, er func (m mockIndex) LabelNamesFor(ids ...storage.SeriesRef) ([]string, error) { namesMap := make(map[string]bool) for _, id := range ids { - for _, lbl := range m.series[id].l { + m.series[id].l.Range(func(lbl labels.Label) { namesMap[lbl.Name] = true - } + }) } names := make([]string, 0, len(namesMap)) for name := range namesMap { @@ -1275,7 +1275,7 @@ func (m mockIndex) Series(ref storage.SeriesRef, builder *labels.ScratchBuilder, if !ok { return storage.ErrNotFound } - *lset = append((*lset)[:0], s.l...) + lset.CopyFrom(s.l) *chks = append((*chks)[:0], s.chunks...) return nil @@ -1297,9 +1297,9 @@ func (m mockIndex) LabelNames(matchers ...*labels.Matcher) ([]string, error) { } } if matches { - for _, lbl := range series.l { + series.l.Range(func(lbl labels.Label) { names[lbl.Name] = struct{}{} - } + }) } } } @@ -1974,7 +1974,7 @@ func BenchmarkQueries(b *testing.B) { // Add some common labels to make the matchers select these series. { - var commonLbls labels.Labels + var commonLbls []labels.Label for _, selector := range selectors { switch selector.Type { case labels.MatchEqual: @@ -1985,8 +1985,11 @@ func BenchmarkQueries(b *testing.B) { } for i := range commonLbls { s := series[i].(*storage.SeriesEntry) - allLabels := append(commonLbls, s.Labels()...) - newS := storage.NewListSeries(allLabels, nil) + allLabels := commonLbls + s.Labels().Range(func(l labels.Label) { + allLabels = append(allLabels, l) + }) + newS := storage.NewListSeries(labels.New(allLabels...), nil) newS.SampleIteratorFn = s.SampleIteratorFn series[i] = newS From d6b97f631a74f73afbde6ac6796cd9a07430a437 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:21:57 +0000 Subject: [PATCH 22/35] Update package storage for new labels.Labels type Signed-off-by: Bryan Boreham --- storage/merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/merge.go b/storage/merge.go index 78a0125db..ba45b12bd 100644 --- a/storage/merge.go +++ b/storage/merge.go @@ -699,7 +699,7 @@ func (c *compactChunkIterator) Next() bool { // 1:1 duplicates, skip it. } else { // We operate on same series, so labels does not matter here. - overlapping = append(overlapping, newChunkToSeriesDecoder(nil, next)) + overlapping = append(overlapping, newChunkToSeriesDecoder(labels.EmptyLabels(), next)) if next.MaxTime > oMaxTime { oMaxTime = next.MaxTime } @@ -716,7 +716,7 @@ func (c *compactChunkIterator) Next() bool { } // Add last as it's not yet included in overlap. We operate on same series, so labels does not matter here. - iter = NewSeriesToChunkEncoder(c.mergeFunc(append(overlapping, newChunkToSeriesDecoder(nil, c.curr))...)).Iterator(nil) + iter = NewSeriesToChunkEncoder(c.mergeFunc(append(overlapping, newChunkToSeriesDecoder(labels.EmptyLabels(), c.curr))...)).Iterator(nil) if !iter.Next() { if c.err = iter.Err(); c.err != nil { return false From 56fefcd8120b6ce9df925bd299fb23660fdda4c0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 9 Mar 2022 22:14:58 +0000 Subject: [PATCH 23/35] Update package promql for new labels.Labels type We use `labels.Builder` to parse metrics, to avoid depending on the internal implementation. This is not efficient, but the feature is only used in tests. It wasn't efficient previously either - calling `Sort()` after adding each label. `createLabelsForAbsentFunction` also uses a Builder now, and gets an extra `map` to replace the previous `Has()` usage. Signed-off-by: Bryan Boreham Fix up promql to compile with changes to Labels --- promql/engine.go | 12 ++++++------ promql/functions.go | 27 ++++++++++++++------------- promql/parser/generated_parser.y | 10 +++++----- promql/parser/generated_parser.y.go | 29 +++++++++++++++-------------- promql/test.go | 4 ++-- 5 files changed, 42 insertions(+), 40 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 0225f78d2..0455779a4 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -1567,7 +1567,7 @@ func (ev *evaluator) eval(expr parser.Expr) (parser.Value, storage.Warnings) { case *parser.NumberLiteral: return ev.rangeEval(nil, func(v []parser.Value, _ [][]EvalSeriesHelper, enh *EvalNodeHelper) (Vector, storage.Warnings) { - return append(enh.Out, Sample{Point: Point{V: e.Val}}), nil + return append(enh.Out, Sample{Point: Point{V: e.Val}, Metric: labels.EmptyLabels()}), nil }) case *parser.StringLiteral: @@ -2190,7 +2190,7 @@ func resultMetric(lhs, rhs labels.Labels, op parser.ItemType, matching *parser.V } } - ret := enh.lb.Labels(nil) + ret := enh.lb.Labels(labels.EmptyLabels()) enh.resultMetric[str] = ret return ret } @@ -2230,7 +2230,7 @@ func (ev *evaluator) VectorscalarBinop(op parser.ItemType, lhs Vector, rhs Scala } func dropMetricName(l labels.Labels) labels.Labels { - return labels.NewBuilder(l).Del(labels.MetricName).Labels(nil) + return labels.NewBuilder(l).Del(labels.MetricName).Labels(labels.EmptyLabels()) } // scalarBinop evaluates a binary operation between two Scalars. @@ -2357,7 +2357,7 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without } } - lb := labels.NewBuilder(nil) + lb := labels.NewBuilder(labels.EmptyLabels()) var buf []byte for si, s := range vec { metric := s.Metric @@ -2365,7 +2365,7 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without if op == parser.COUNT_VALUES { lb.Reset(metric) lb.Set(valueLabel, strconv.FormatFloat(s.V, 'f', -1, 64)) - metric = lb.Labels(nil) + metric = lb.Labels(labels.EmptyLabels()) // We've changed the metric so we have to recompute the grouping key. recomputeGroupingKey = true @@ -2389,7 +2389,7 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without } else { lb.Keep(grouping...) } - m := lb.Labels(nil) + m := lb.Labels(labels.EmptyLabels()) newAgg := &groupedAggregation{ labels: m, value: s.V, diff --git a/promql/functions.go b/promql/functions.go index d481cb735..c5922002b 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -957,7 +957,7 @@ func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *Ev if !ok { sample.Metric = labels.NewBuilder(sample.Metric). Del(excludedLabels...). - Labels(nil) + Labels(labels.EmptyLabels()) mb = &metricWithBuckets{sample.Metric, nil} enh.signatureToMetricWithBuckets[string(enh.lblBuf)] = mb @@ -1077,7 +1077,7 @@ func funcLabelReplace(vals []parser.Value, args parser.Expressions, enh *EvalNod if len(res) > 0 { lb.Set(dst, string(res)) } - outMetric = lb.Labels(nil) + outMetric = lb.Labels(labels.EmptyLabels()) enh.Dmn[h] = outMetric } } @@ -1145,7 +1145,7 @@ func funcLabelJoin(vals []parser.Value, args parser.Expressions, enh *EvalNodeHe lb.Set(dst, strval) } - outMetric = lb.Labels(nil) + outMetric = lb.Labels(labels.EmptyLabels()) enh.Dmn[h] = outMetric } @@ -1383,7 +1383,7 @@ func (s *vectorByReverseValueHeap) Pop() interface{} { // createLabelsForAbsentFunction returns the labels that are uniquely and exactly matched // in a given expression. It is used in the absent functions. func createLabelsForAbsentFunction(expr parser.Expr) labels.Labels { - m := labels.Labels{} + b := labels.NewBuilder(labels.EmptyLabels()) var lm []*labels.Matcher switch n := expr.(type) { @@ -1392,25 +1392,26 @@ func createLabelsForAbsentFunction(expr parser.Expr) labels.Labels { case *parser.MatrixSelector: lm = n.VectorSelector.(*parser.VectorSelector).LabelMatchers default: - return m + return labels.EmptyLabels() } - empty := []string{} + // The 'has' map implements backwards-compatibility for historic behaviour: + // e.g. in `absent(x{job="a",job="b",foo="bar"})` then `job` is removed from the output. + // Note this gives arguably wrong behaviour for `absent(x{job="a",job="a",foo="bar"})`. + has := make(map[string]bool, len(lm)) for _, ma := range lm { if ma.Name == labels.MetricName { continue } - if ma.Type == labels.MatchEqual && !m.Has(ma.Name) { - m = labels.NewBuilder(m).Set(ma.Name, ma.Value).Labels(nil) + if ma.Type == labels.MatchEqual && !has[ma.Name] { + b.Set(ma.Name, ma.Value) + has[ma.Name] = true } else { - empty = append(empty, ma.Name) + b.Del(ma.Name) } } - for _, v := range empty { - m = labels.NewBuilder(m).Del(v).Labels(nil) - } - return m + return b.Labels(labels.EmptyLabels()) } func stringFromArg(e parser.Expr) string { diff --git a/promql/parser/generated_parser.y b/promql/parser/generated_parser.y index 433f45259..461e854ac 100644 --- a/promql/parser/generated_parser.y +++ b/promql/parser/generated_parser.y @@ -16,13 +16,13 @@ package parser import ( "math" - "sort" "strconv" "time" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/value" ) + %} %union { @@ -32,6 +32,7 @@ import ( matcher *labels.Matcher label labels.Label labels labels.Labels + lblList []labels.Label strings []string series []SequenceValue uint uint64 @@ -138,10 +139,9 @@ START_METRIC_SELECTOR // Type definitions for grammar rules. %type label_match_list %type label_matcher - %type aggregate_op grouping_label match_op maybe_label metric_identifier unary_op at_modifier_preprocessors - -%type label_set label_set_list metric +%type label_set metric +%type label_set_list %type