From 80ac0d7c82cc325932c1934ad9f7e55e8ed3e479 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 23 Dec 2022 20:24:20 +0000 Subject: [PATCH 1/3] promql: add benchmark for match against blank string Blank strings are not handled efficiently by tsdb. Signed-off-by: Bryan Boreham --- promql/bench_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/promql/bench_test.go b/promql/bench_test.go index 6fb20d1ab..8abfcfdd2 100644 --- a/promql/bench_test.go +++ b/promql/bench_test.go @@ -174,6 +174,15 @@ func rangeQueryCases() []benchCase { { expr: "a_X + on(l) group_right a_one", }, + // Label compared to blank string. + { + expr: "count({__name__!=\"\"})", + steps: 1, + }, + { + expr: "count({__name__!=\"\",l=\"\"})", + steps: 1, + }, } // X in an expr will be replaced by different metric sizes. From cf92cd26880b5bd1c694d5a0cf644eea76b5f3ea Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 4 Jan 2023 17:45:53 +0000 Subject: [PATCH 2/3] tsdb: sort values for Postings only when required In the head and in v1 postings on disk, it makes no difference whether postings are sorted. Only for v2 does the code step through in order. So, move the sorting to where it is required, and thus skip it entirely in the head. Label values in on-disk blocks are already sorted, but `slices.Sort` is very fast on already-sorted data so we don't bother checking. Signed-off-by: Bryan Boreham --- tsdb/block.go | 2 +- tsdb/index/index.go | 1 + tsdb/querier.go | 18 ------------------ 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/tsdb/block.go b/tsdb/block.go index 401ab59e0..d7b344ab5 100644 --- a/tsdb/block.go +++ b/tsdb/block.go @@ -72,7 +72,7 @@ type IndexReader interface { // Postings returns the postings list iterator for the label pairs. // The Postings here contain the offsets to the series inside the index. // Found IDs are not strictly required to point to a valid Series, e.g. - // during background garbage collections. Input values must be sorted. + // during background garbage collections. Postings(name string, values ...string) (index.Postings, error) // SortedPostings returns a postings list that is reordered to be sorted diff --git a/tsdb/index/index.go b/tsdb/index/index.go index 407b5380f..4d68f240a 100644 --- a/tsdb/index/index.go +++ b/tsdb/index/index.go @@ -1643,6 +1643,7 @@ func (r *Reader) Postings(name string, values ...string) (Postings, error) { return EmptyPostings(), nil } + slices.Sort(values) // Values must be in order so we can step through the table on disk. res := make([]Postings, 0, len(values)) skip := 0 valueIndex := 0 diff --git a/tsdb/querier.go b/tsdb/querier.go index 16e774b0b..f6d3ec16c 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -21,7 +21,6 @@ import ( "github.com/oklog/ulid" "github.com/pkg/errors" - "golang.org/x/exp/slices" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" @@ -322,7 +321,6 @@ func postingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Postings, erro if m.Type == labels.MatchRegexp { setMatches := findSetMatches(m.GetRegexString()) if len(setMatches) > 0 { - slices.Sort(setMatches) return ix.Postings(m.Name, setMatches...) } } @@ -333,14 +331,9 @@ func postingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Postings, erro } var res []string - lastVal, isSorted := "", true for _, val := range vals { if m.Matches(val) { res = append(res, val) - if isSorted && val < lastVal { - isSorted = false - } - lastVal = val } } @@ -348,9 +341,6 @@ func postingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Postings, erro return index.EmptyPostings(), nil } - if !isSorted { - slices.Sort(res) - } return ix.Postings(m.Name, res...) } @@ -362,20 +352,12 @@ func inversePostingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Posting } var res []string - lastVal, isSorted := "", true for _, val := range vals { if !m.Matches(val) { res = append(res, val) - if isSorted && val < lastVal { - isSorted = false - } - lastVal = val } } - if !isSorted { - slices.Sort(res) - } return ix.Postings(m.Name, res...) } From e61348d9f3ac4e1e579db244ee9bab48fc18aaba Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 5 Jan 2023 14:05:29 +0000 Subject: [PATCH 3/3] tsdb/index: fast-track postings for label="" We need to special-case ""="" too, which is used in some tests to mean "everything". Signed-off-by: Bryan Boreham --- tsdb/querier.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tsdb/querier.go b/tsdb/querier.go index f6d3ec16c..042fe76de 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -239,7 +239,14 @@ func PostingsForMatchers(ix IndexReader, ms ...*labels.Matcher) (index.Postings, } for _, m := range ms { - if labelMustBeSet[m.Name] { + if m.Name == "" && m.Value == "" { // Special-case for AllPostings, used in tests at least. + k, v := index.AllPostingsKey() + allPostings, err := ix.Postings(k, v) + if err != nil { + return nil, err + } + its = append(its, allPostings) + } else if labelMustBeSet[m.Name] { // If this matcher must be non-empty, we can be smarter. matchesEmpty := m.Matches("") isNot := m.Type == labels.MatchNotEqual || m.Type == labels.MatchNotRegexp @@ -352,9 +359,14 @@ func inversePostingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Posting } var res []string - for _, val := range vals { - if !m.Matches(val) { - res = append(res, val) + // If the inverse match is ="", we just want all the values. + if m.Type == labels.MatchEqual && m.Value == "" { + res = vals + } else { + for _, val := range vals { + if !m.Matches(val) { + res = append(res, val) + } } }