Fix OOO Head LabelValues and PostingsForMatchers

OOOHeadIndexReader was using the headIndexReader PostingsForMatchers()
and LabelValues() implementation which lead to a very subtle bug that
led to wrong query results.

headIndexReader LabelValues() implementation checks if the query
timerange overlaps with the head maxt and mint and if it doesnt it
returns an empty list of values. Since this code was also used by the
ooo head it led to wrong results that we were not able to see in tests
because our queries where always from MinInt64 to MaxInt64. This commit
also adds a new test that performs multiple time range queries to make
sure this never happens again.

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

tsdb/db_test.go

@@ -4196,7 +4196,7 @@ func TestOOOAppendAndQuery(t *testing.T) {
 	s2 := labels.FromStrings("foo", "bar2")
 
 	minutes := func(m int64) int64 { return m * time.Minute.Milliseconds() }
-	expSamples := make(map[string][]tsdbutil.Sample)
+	appendedSamples := make(map[string][]tsdbutil.Sample)
 	totalSamples := 0
 	addSample := func(lbls labels.Labels, fromMins, toMins int64, faceError bool) {
 		app := db.Appender(context.Background())
@@ -4209,7 +4209,7 @@ func TestOOOAppendAndQuery(t *testing.T) {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
-				expSamples[key] = append(expSamples[key], sample{t: min, v: val})
+				appendedSamples[key] = append(appendedSamples[key], sample{t: min, v: val})
 				totalSamples++
 			}
 		}
@@ -4220,17 +4220,30 @@ func TestOOOAppendAndQuery(t *testing.T) {
 		}
 	}
 
-	testQuery := func() {
-		querier, err := db.Querier(context.TODO(), math.MinInt64, math.MaxInt64)
+	testQuery := func(from, to int64) {
+		querier, err := db.Querier(context.TODO(), from, to)
 		require.NoError(t, err)
 
 		seriesSet := query(t, querier, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar."))
 
-		for k, v := range expSamples {
+		for k, v := range appendedSamples {
 			sort.Slice(v, func(i, j int) bool {
 				return v[i].T() < v[j].T()
 			})
-			expSamples[k] = v
+			appendedSamples[k] = v
+		}
+
+		expSamples := make(map[string][]tsdbutil.Sample)
+		for k, samples := range appendedSamples {
+			for _, s := range samples {
+				if s.T() < from {
+					continue
+				}
+				if s.T() > to {
+					continue
+				}
+				expSamples[k] = append(expSamples[k], s)
+			}
 		}
 		require.Equal(t, expSamples, seriesSet)
 		require.Equal(t, float64(totalSamples-2), prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamplesAppended), "number of ooo appended samples mismatch")
@@ -4245,40 +4258,43 @@ func TestOOOAppendAndQuery(t *testing.T) {
 	addSample(s1, 300, 300, false)
 	addSample(s2, 290, 290, false)
 	require.Equal(t, float64(2), prom_testutil.ToFloat64(db.head.metrics.chunksCreated))
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// Some ooo samples.
 	addSample(s1, 250, 260, false)
 	addSample(s2, 255, 265, false)
 	verifyOOOMinMaxTimes(250, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
+	testQuery(minutes(250), minutes(265)) // Test querying ono data time range
+	testQuery(minutes(290), minutes(300)) // Test querying in-order data time range
+	testQuery(minutes(250), minutes(300)) // Test querying the entire range
 
 	// Out of time window.
 	addSample(s1, 59, 59, true)
 	addSample(s2, 49, 49, true)
 	verifyOOOMinMaxTimes(250, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// At the edge of time window, also it would be "out of bound" without the ooo support.
 	addSample(s1, 60, 65, false)
 	verifyOOOMinMaxTimes(60, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// This sample is not within the time window w.r.t. the head's maxt, but it is within the window
 	// w.r.t. the series' maxt. But we consider only head's maxt.
 	addSample(s2, 59, 59, true)
 	verifyOOOMinMaxTimes(60, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// Now the sample is within time window w.r.t. the head's maxt.
 	addSample(s2, 60, 65, false)
 	verifyOOOMinMaxTimes(60, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// Out of time window again.
 	addSample(s1, 59, 59, true)
 	addSample(s2, 49, 49, true)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 
 	// Generating some m-map chunks. The m-map chunks here are in such a way
 	// that when sorted w.r.t. mint, the last chunk's maxt is not the overall maxt
@@ -4287,7 +4303,7 @@ func TestOOOAppendAndQuery(t *testing.T) {
 	addSample(s1, 180, 249, false)
 	require.Equal(t, float64(6), prom_testutil.ToFloat64(db.head.metrics.chunksCreated))
 	verifyOOOMinMaxTimes(60, 265)
-	testQuery()
+	testQuery(math.MinInt64, math.MaxInt64)
 }
 
 func TestOOODisabled(t *testing.T) {

tsdb/ooo_head_read.go

@@ -134,6 +134,27 @@ func (oh *OOOHeadIndexReader) series(ref storage.SeriesRef, lbls *labels.Labels,
 	return nil
 }
 
+// PostingsForMatchers needs to be overridden so that the right IndexReader
+// implementation gets passed down to the PostingsForMatchers call.
+func (oh *OOOHeadIndexReader) PostingsForMatchers(concurrent bool, ms ...*labels.Matcher) (index.Postings, error) {
+	return oh.head.pfmc.PostingsForMatchers(oh, concurrent, ms...)
+}
+
+// LabelValues needs to be overridden from the headIndexReader implementation due
+// to the check that happens at the beginning where we make sure that the query
+// interval overlaps with the head minooot and maxooot.
+func (oh *OOOHeadIndexReader) LabelValues(name string, matchers ...*labels.Matcher) ([]string, error) {
+	if oh.maxt < oh.head.MinOOOTime() || oh.mint > oh.head.MaxOOOTime() {
+		return []string{}, nil
+	}
+
+	if len(matchers) == 0 {
+		return oh.head.postings.LabelValues(name), nil
+	}
+
+	return labelValuesWithMatchers(oh, name, matchers...)
+}
+
 type chunkMetaAndChunkDiskMapperRef struct {
 	meta     chunks.Meta
 	ref      chunks.ChunkDiskMapperRef