Merge pull request #11809 from bboreham/dont-sort-postings-values

tsdb: sort values for Postings only when required

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.

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

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

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"
@@ -240,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
@@ -322,7 +328,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 +338,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 +348,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 +359,17 @@ func inversePostingsForMatcher(ix IndexReader, m *labels.Matcher) (index.Posting
 	}
 
 	var res []string
-	lastVal, isSorted := "", true
+	// If the inverse match is ="", we just want all the values.
-	for _, val := range vals {
+	if m.Type == labels.MatchEqual && m.Value == "" {
-		if !m.Matches(val) {
+		res = vals
-			res = append(res, val)
+	} else {
-			if isSorted && val < lastVal {
+		for _, val := range vals {
-				isSorted = false
+			if !m.Matches(val) {
+				res = append(res, val)
 			}
-			lastVal = val
 		}
 	}
 
-	if !isSorted {
-		slices.Sort(res)
-	}
 	return ix.Postings(m.Name, res...)
 }