honor mint,maxt if sp is null for other querier implementations

Signed-off-by: Abdurrahman J. Allawala <aallawala1@gmail.com>

Author: aallawala

pkg/querier/chunk_store_queryable.go

@@ -44,14 +44,19 @@ func (q *chunkStoreQuerier) Select(_ bool, sp *storage.SelectHints, matchers ...
 		return storage.ErrSeriesSet(err)
 	}
 
+	minT, maxT := q.mint, q.maxt
+	if sp != nil {
+		minT, maxT = sp.Start, sp.End
+	}
+
 	// We will hit this for /series lookup when -querier.query-store-for-labels-enabled is set.
 	// If we don't skip here, it'll make /series lookups extremely slow as all the chunks will be loaded.
 	// That flag is only to be set with blocks storage engine, and this is a protective measure.
-	if sp == nil || sp.Func == "series" {
+	if sp != nil && sp.Func == "series" {
 		return storage.EmptySeriesSet()
 	}
 
-	chunks, err := q.store.Get(q.ctx, userID, model.Time(sp.Start), model.Time(sp.End), matchers...)
+	chunks, err := q.store.Get(q.ctx, userID, model.Time(minT), model.Time(maxT), matchers...)
 	if err != nil {
 		return storage.ErrSeriesSet(err)
 	}

pkg/querier/distributor_queryable.go

@@ -83,23 +83,26 @@ func (q *distributorQuerier) Select(_ bool, sp *storage.SelectHints, matchers ..
 	log, ctx := spanlogger.New(q.ctx, "distributorQuerier.Select")
 	defer log.Span.Finish()
 
-	// Kludge: Prometheus passes nil SelectParams if it is doing a 'series' operation,
-	// which needs only metadata. For this specific case we shouldn't apply the queryIngestersWithin
+	minT, maxT := q.mint, q.maxt
+	if sp != nil {
+		minT, maxT = sp.Start, sp.End
+	}
+
+	// If the querier receives a 'series' query, it means only metadata is needed.
+	// For this specific case we shouldn't apply the queryIngestersWithin
 	// time range manipulation, otherwise we'll end up returning no series at all for
 	// older time ranges (while in Cortex we do ignore the start/end and always return
 	// series in ingesters).
 	// Also, in the recent versions of Prometheus, we pass in the hint but with Func set to "series".
 	// See: https://github.com/prometheus/prometheus/pull/8050
-	if sp == nil || sp.Func == "series" {
+	if sp != nil && sp.Func == "series" {
 		ms, err := q.distributor.MetricsForLabelMatchers(ctx, model.Time(q.mint), model.Time(q.maxt), matchers...)
 		if err != nil {
 			return storage.ErrSeriesSet(err)
 		}
 		return series.MetricsToSeriesSet(ms)
 	}
 
-	minT, maxT := sp.Start, sp.End
-
 	// If queryIngestersWithin is enabled, we do manipulate the query mint to query samples up until
 	// now - queryIngestersWithin, because older time ranges are covered by the storage. This
 	// optimization is particularly important for the blocks storage where the blocks retention in the

pkg/querier/distributor_queryable_test.go

@@ -126,10 +126,10 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
 				querier, err := queryable.Querier(ctx, testData.queryMinT, testData.queryMaxT)
 				require.NoError(t, err)
 
-				// Select hints are not passed by Prometheus when querying /series.
+				// Select hints are passed by Prometheus when querying /series.
 				var hints *storage.SelectHints
-				if !testData.querySeries {
-					hints = &storage.SelectHints{Start: testData.queryMinT, End: testData.queryMaxT}
+				if testData.querySeries {
+					hints = &storage.SelectHints{Func: "series"}
 				}
 
 				seriesSet := querier.Select(true, hints)

pkg/querier/querier.go

@@ -300,17 +300,18 @@ func (q querier) Select(_ bool, sp *storage.SelectHints, matchers ...*labels.Mat
 		level.Debug(log).Log("start", util.TimeFromMillis(sp.Start).UTC().String(), "end", util.TimeFromMillis(sp.End).UTC().String(), "step", sp.Step, "matchers", matchers)
 	}
 
-	// If the querier receives a 'series' query, it means only metadata is needed.
-	// Here we expect that metadataQuerier querier will handle that.
-	// Also, in the recent versions of Prometheus, we pass in the hint but with Func set to "series".
-	// See: https://github.com/prometheus/prometheus/pull/8050
-	if sp != nil && sp.Func == "series" && !q.queryStoreForLabels {
+	if sp == nil {
+		// if SelectHints is null, rely on minT, maxT of querier to scope in range for Select stmt
+		sp = &storage.SelectHints{Start: q.mint, End: q.maxt}
+	} else if sp.Func == "series" && !q.queryStoreForLabels {
+		// Else if the querier receives a 'series' query, it means only metadata is needed.
+		// Here we expect that metadataQuerier querier will handle that.
+		// Also, in the recent versions of Prometheus, we pass in the hint but with Func set to "series".
+		// See: https://github.com/prometheus/prometheus/pull/8050
+
 		// In this case, the query time range has already been validated when the querier has been
 		// created.
 		return q.metadataQuerier.Select(true, sp, matchers...)
-	} else if sp == nil {
-		// if SelectHints is null, rely on minT, maxT of querier to scope in range for Select stmt
-		sp = &storage.SelectHints{Start: q.mint, End: q.maxt}
 	}
 
 	userID, err := tenant.TenantID(ctx)