Remove query_store_for_labels_enabled configuration (#5984)

* Deprecate query_store_for_labels_enabled

Signed-off-by: alanprot <[email protected]>

* Remove getseries test with old timestamps - that should return data from ingsters

Signed-off-by: alanprot <[email protected]>

* changelog

Signed-off-by: alanprot <[email protected]>

---------

Signed-off-by: alanprot <[email protected]>

Author: alanprot

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## master / unreleased
 
 * [CHANGE] Upgrade Dockerfile Node version from 14x to 18x. #5906
+* [CHANGE] Ingester: Remove `-querier.query-store-for-labels-enabled` flag. Querying long-term store for labels is always enabled. #5984
 * [ENHANCEMENT] Query Frontend/Querier: Added store gateway postings touched count and touched size in Querier stats and log in Query Frontend. #5892
 * [ENHANCEMENT] Query Frontend/Querier: Returns `warnings` on prometheus query responses. #5916
 * [ENHANCEMENT] Ingester: Allowing to configure `-blocks-storage.tsdb.head-compaction-interval` flag up to 30 min and add a jitter on the first head compaction. #5919 #5928

docs/blocks-storage/querier.md

@@ -117,12 +117,6 @@ querier:
   # CLI flag: -querier.query-ingesters-within
   [query_ingesters_within: <duration> | default = 0s]
 
-  # Deprecated (Querying long-term store for labels will be always enabled in
-  # the future.): Query long-term store for series, label values and label names
-  # APIs.
-  # CLI flag: -querier.query-store-for-labels-enabled
-  [query_store_for_labels_enabled: <boolean> | default = false]
-
   # Enable returning samples stats per steps in query response.
   # CLI flag: -querier.per-step-stats-enabled
   [per_step_stats_enabled: <boolean> | default = false]

docs/configuration/config-file-reference.md

@@ -3644,11 +3644,6 @@ The `querier_config` configures the Cortex querier.
 # CLI flag: -querier.query-ingesters-within
 [query_ingesters_within: <duration> | default = 0s]
 
-# Deprecated (Querying long-term store for labels will be always enabled in the
-# future.): Query long-term store for series, label values and label names APIs.
-# CLI flag: -querier.query-store-for-labels-enabled
-[query_store_for_labels_enabled: <boolean> | default = false]
-
 # Enable returning samples stats per steps in query response.
 # CLI flag: -querier.per-step-stats-enabled
 [per_step_stats_enabled: <boolean> | default = false]

integration/query_frontend_test.go

@@ -361,18 +361,6 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) {
 			require.Equal(t, http.StatusBadRequest, resp.StatusCode)
 		}
 
-		// In this test we do ensure that the /series start/end time is ignored and Cortex
-		// always returns series in ingesters memory. No need to repeat it for each user.
-		if userID == 0 {
-			start := now.Add(-1000 * time.Hour)
-			end := now.Add(-999 * time.Hour)
-
-			result, err := c.Series([]string{"series_1"}, start, end)
-			require.NoError(t, err)
-			require.Len(t, result, 1)
-			assert.Equal(t, model.LabelSet{labels.MetricName: "series_1"}, result[0])
-		}
-
 		// No need to repeat the query 400 test for each user.
 		if userID == 0 {
 			start := time.Unix(1595846748, 806*1e6)
@@ -400,7 +388,7 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) {
 
 	wg.Wait()
 
-	extra := float64(3)
+	extra := float64(2)
 	if cfg.testMissingMetricName {
 		extra++
 	}

pkg/cortex/modules.go

@@ -403,7 +403,6 @@ func (t *Cortex) initIngesterService() (serv services.Service, err error) {
 	t.Cfg.Ingester.DistributorShardingStrategy = t.Cfg.Distributor.ShardingStrategy
 	t.Cfg.Ingester.DistributorShardByAllLabels = t.Cfg.Distributor.ShardByAllLabels
 	t.Cfg.Ingester.InstanceLimitsFn = ingesterInstanceLimits(t.RuntimeConfig)
-	t.Cfg.Ingester.QueryStoreForLabels = t.Cfg.Querier.QueryStoreForLabels
 	t.Cfg.Ingester.QueryIngestersWithin = t.Cfg.Querier.QueryIngestersWithin
 	t.tsdbIngesterConfig()
 

pkg/ingester/ingester.go

@@ -120,7 +120,6 @@ type Config struct {
 	DistributorShardByAllLabels bool   `yaml:"-"`
 
 	// Injected at runtime and read from querier config.
-	QueryStoreForLabels  bool          `yaml:"-"`
 	QueryIngestersWithin time.Duration `yaml:"-"`
 
 	DefaultLimits    InstanceLimits         `yaml:"instance_limits"`
@@ -1426,7 +1425,7 @@ func (i *Ingester) labelsValuesCommon(ctx context.Context, req *client.LabelValu
 		return &client.LabelValuesResponse{}, cleanup, nil
 	}
 
-	mint, maxt, err := metadataQueryRange(startTimestampMs, endTimestampMs, db, i.cfg.QueryStoreForLabels, i.cfg.QueryIngestersWithin)
+	mint, maxt, err := metadataQueryRange(startTimestampMs, endTimestampMs, db, i.cfg.QueryIngestersWithin)
 	if err != nil {
 		return nil, cleanup, err
 	}
@@ -1506,7 +1505,7 @@ func (i *Ingester) labelNamesCommon(ctx context.Context, req *client.LabelNamesR
 		return &client.LabelNamesResponse{}, cleanup, nil
 	}
 
-	mint, maxt, err := metadataQueryRange(req.StartTimestampMs, req.EndTimestampMs, db, i.cfg.QueryStoreForLabels, i.cfg.QueryIngestersWithin)
+	mint, maxt, err := metadataQueryRange(req.StartTimestampMs, req.EndTimestampMs, db, i.cfg.QueryIngestersWithin)
 	if err != nil {
 		return nil, cleanup, err
 	}
@@ -1586,7 +1585,7 @@ func (i *Ingester) metricsForLabelMatchersCommon(ctx context.Context, req *clien
 		return nil, cleanup, err
 	}
 
-	mint, maxt, err := metadataQueryRange(req.StartTimestampMs, req.EndTimestampMs, db, i.cfg.QueryStoreForLabels, i.cfg.QueryIngestersWithin)
+	mint, maxt, err := metadataQueryRange(req.StartTimestampMs, req.EndTimestampMs, db, i.cfg.QueryIngestersWithin)
 	if err != nil {
 		return nil, cleanup, err
 	}
@@ -2755,8 +2754,8 @@ func (i *Ingester) flushHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // metadataQueryRange returns the best range to query for metadata queries based on the timerange in the ingester.
-func metadataQueryRange(queryStart, queryEnd int64, db *userTSDB, queryStoreForLabels bool, queryIngestersWithin time.Duration) (mint, maxt int64, err error) {
-	if queryIngestersWithin > 0 && queryStoreForLabels {
+func metadataQueryRange(queryStart, queryEnd int64, db *userTSDB, queryIngestersWithin time.Duration) (mint, maxt int64, err error) {
+	if queryIngestersWithin > 0 {
 		// If the feature for querying metadata from store-gateway is enabled,
 		// then we don't want to manipulate the mint and maxt.
 		return queryStart, queryEnd, nil

pkg/ingester/ingester_test.go

@@ -2184,7 +2184,6 @@ func Test_Ingester_MetricsForLabelMatchers(t *testing.T) {
 		to                   int64
 		matchers             []*client.LabelMatchers
 		expected             []*cortexpb.Metric
-		queryStoreForLabels  bool
 		queryIngestersWithin time.Duration
 	}{
 		"should return an empty response if no metric match": {
@@ -2254,7 +2253,6 @@ func Test_Ingester_MetricsForLabelMatchers(t *testing.T) {
 			expected: []*cortexpb.Metric{
 				{Labels: cortexpb.FromLabelsToLabelAdapters(fixtures[0].lbls)},
 			},
-			queryStoreForLabels:  true,
 			queryIngestersWithin: time.Hour,
 		},
 		"should not return duplicated metrics on overlapping matchers": {
@@ -2323,7 +2321,6 @@ func Test_Ingester_MetricsForLabelMatchers(t *testing.T) {
 				EndTimestampMs:   testData.to,
 				MatchersSet:      testData.matchers,
 			}
-			i.cfg.QueryStoreForLabels = testData.queryStoreForLabels
 			i.cfg.QueryIngestersWithin = testData.queryIngestersWithin
 			res, err := i.MetricsForLabelMatchers(ctx, req)
 			require.NoError(t, err)

pkg/querier/distributor_queryable.go

@@ -36,13 +36,12 @@ type Distributor interface {
 	MetricsMetadata(ctx context.Context) ([]scrape.MetricMetadata, error)
 }
 
-func newDistributorQueryable(distributor Distributor, streamingMetdata bool, iteratorFn chunkIteratorFunc, queryIngestersWithin time.Duration, queryStoreForLabels bool) QueryableWithFilter {
+func newDistributorQueryable(distributor Distributor, streamingMetdata bool, iteratorFn chunkIteratorFunc, queryIngestersWithin time.Duration) QueryableWithFilter {
 	return distributorQueryable{
 		distributor:          distributor,
 		streamingMetdata:     streamingMetdata,
 		iteratorFn:           iteratorFn,
 		queryIngestersWithin: queryIngestersWithin,
-		queryStoreForLabels:  queryStoreForLabels,
 	}
 }
 
@@ -51,7 +50,6 @@ type distributorQueryable struct {
 	streamingMetdata     bool
 	iteratorFn           chunkIteratorFunc
 	queryIngestersWithin time.Duration
-	queryStoreForLabels  bool
 }
 
 func (d distributorQueryable) Querier(mint, maxt int64) (storage.Querier, error) {
@@ -62,7 +60,6 @@ func (d distributorQueryable) Querier(mint, maxt int64) (storage.Querier, error)
 		streamingMetadata:    d.streamingMetdata,
 		chunkIterFn:          d.iteratorFn,
 		queryIngestersWithin: d.queryIngestersWithin,
-		queryStoreForLabels:  d.queryStoreForLabels,
 	}, nil
 }
 
@@ -77,7 +74,6 @@ type distributorQuerier struct {
 	streamingMetadata    bool
 	chunkIterFn          chunkIteratorFunc
 	queryIngestersWithin time.Duration
-	queryStoreForLabels  bool
 }
 
 // Select implements storage.Querier interface.
@@ -91,19 +87,11 @@ func (q *distributorQuerier) Select(ctx context.Context, sortSeries bool, sp *st
 		minT, maxT = sp.Start, sp.End
 	}
 
-	// If the querier receives a 'series' query, it means only metadata is needed.
-	// For the specific case where queryStoreForLabels is disabled
-	// 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).
-	shouldNotQueryStoreForMetadata := (sp != nil && sp.Func == "series" && !q.queryStoreForLabels)
-
-	// If queryIngestersWithin is enabled, we do manipulate the query mint to query samples up until
+	// We should 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
 	// ingesters could be way higher than queryIngestersWithin.
-	if q.queryIngestersWithin > 0 && !shouldNotQueryStoreForMetadata {
+	if q.queryIngestersWithin > 0 {
 		now := time.Now()
 		origMinT := minT
 		minT = max(minT, util.TimeToMillis(now.Add(-q.queryIngestersWithin)))

pkg/querier/distributor_queryable_test.go

@@ -34,7 +34,6 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
 
 	tests := map[string]struct {
 		querySeries          bool
-		queryStoreForLabels  bool
 		queryIngestersWithin time.Duration
 		queryMinT            int64
 		queryMaxT            int64
@@ -69,19 +68,10 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
 			expectedMinT:         0,
 			expectedMaxT:         0,
 		},
-		"should not manipulate query time range if queryIngestersWithin is enabled and query max time is older, but the query is for /series": {
+		"should manipulate query time range if queryIngestersWithin is enabled": {
 			querySeries:          true,
 			queryIngestersWithin: time.Hour,
 			queryMinT:            util.TimeToMillis(now.Add(-100 * time.Minute)),
-			queryMaxT:            util.TimeToMillis(now.Add(-90 * time.Minute)),
-			expectedMinT:         util.TimeToMillis(now.Add(-100 * time.Minute)),
-			expectedMaxT:         util.TimeToMillis(now.Add(-90 * time.Minute)),
-		},
-		"should manipulate query time range if queryIngestersWithin is enabled and queryStoreForLabels is enabled": {
-			querySeries:          true,
-			queryStoreForLabels:  true,
-			queryIngestersWithin: time.Hour,
-			queryMinT:            util.TimeToMillis(now.Add(-100 * time.Minute)),
 			queryMaxT:            util.TimeToMillis(now.Add(-30 * time.Minute)),
 			expectedMinT:         util.TimeToMillis(now.Add(-60 * time.Minute)),
 			expectedMaxT:         util.TimeToMillis(now.Add(-30 * time.Minute)),
@@ -100,7 +90,7 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
 				distributor.On("MetricsForLabelMatchersStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]model.Metric{}, nil)
 
 				ctx := user.InjectOrgID(context.Background(), "test")
-				queryable := newDistributorQueryable(distributor, streamingMetadataEnabled, nil, testData.queryIngestersWithin, testData.queryStoreForLabels)
+				queryable := newDistributorQueryable(distributor, streamingMetadataEnabled, nil, testData.queryIngestersWithin)
 				querier, err := queryable.Querier(testData.queryMinT, testData.queryMaxT)
 				require.NoError(t, err)
 
@@ -139,7 +129,7 @@ func TestDistributorQueryableFilter(t *testing.T) {
 	t.Parallel()
 
 	d := &MockDistributor{}
-	dq := newDistributorQueryable(d, false, nil, 1*time.Hour, true)
+	dq := newDistributorQueryable(d, false, nil, 1*time.Hour)
 
 	now := time.Now()
 
@@ -189,7 +179,7 @@ func TestIngesterStreaming(t *testing.T) {
 		nil)
 
 	ctx := user.InjectOrgID(context.Background(), "0")
-	queryable := newDistributorQueryable(d, true, batch.NewChunkMergeIterator, 0, true)
+	queryable := newDistributorQueryable(d, true, batch.NewChunkMergeIterator, 0)
 	querier, err := queryable.Querier(mint, maxt)
 	require.NoError(t, err)
 
@@ -230,7 +220,7 @@ func TestDistributorQuerier_LabelNames(t *testing.T) {
 			d.On("MetricsForLabelMatchersStream", mock.Anything, model.Time(mint), model.Time(maxt), someMatchers).
 				Return(metrics, nil)
 
-			queryable := newDistributorQueryable(d, streamingEnabled, nil, 0, true)
+			queryable := newDistributorQueryable(d, streamingEnabled, nil, 0)
 			querier, err := queryable.Querier(mint, maxt)
 			require.NoError(t, err)
 

pkg/querier/querier.go

@@ -47,7 +47,6 @@ type Config struct {
 	IngesterMetadataStreaming bool          `yaml:"ingester_metadata_streaming"`
 	MaxSamples                int           `yaml:"max_samples"`
 	QueryIngestersWithin      time.Duration `yaml:"query_ingesters_within"`
-	QueryStoreForLabels       bool          `yaml:"query_store_for_labels_enabled"`
 	AtModifierEnabled         bool          `yaml:"at_modifier_enabled" doc:"hidden"`
 	EnablePerStepStats        bool          `yaml:"per_step_stats_enabled"`
 
@@ -103,14 +102,15 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
 	flagext.DeprecatedFlag(f, "querier.iterators", "Deprecated: Use iterators to execute query. This flag is no longer functional; Batch iterator is always enabled instead.", util_log.Logger)
 	//lint:ignore faillint Need to pass the global logger like this for warning on deprecated methods
 	flagext.DeprecatedFlag(f, "querier.batch-iterators", "Deprecated: Use batch iterators to execute query. This flag is no longer functional; Batch iterator is always enabled now.", util_log.Logger)
+	//lint:ignore faillint Need to pass the global logger like this for warning on deprecated methods
+	flagext.DeprecatedFlag(f, "querier.query-store-for-labels-enabled", "Deprecated: Querying long-term store is always enabled.", util_log.Logger)
 
 	cfg.StoreGatewayClient.RegisterFlagsWithPrefix("querier.store-gateway-client", f)
 	f.IntVar(&cfg.MaxConcurrent, "querier.max-concurrent", 20, "The maximum number of concurrent queries.")
 	f.DurationVar(&cfg.Timeout, "querier.timeout", 2*time.Minute, "The timeout for a query.")
 	f.BoolVar(&cfg.IngesterMetadataStreaming, "querier.ingester-metadata-streaming", false, "Use streaming RPCs for metadata APIs from ingester.")
 	f.IntVar(&cfg.MaxSamples, "querier.max-samples", 50e6, "Maximum number of samples a single query can load into memory.")
 	f.DurationVar(&cfg.QueryIngestersWithin, "querier.query-ingesters-within", 0, "Maximum lookback beyond which queries are not sent to ingester. 0 means all queries are sent to ingester.")
-	f.BoolVar(&cfg.QueryStoreForLabels, "querier.query-store-for-labels-enabled", false, "Deprecated (Querying long-term store for labels will be always enabled in the future.): Query long-term store for series, label values and label names APIs.")
 	f.BoolVar(&cfg.EnablePerStepStats, "querier.per-step-stats-enabled", false, "Enable returning samples stats per steps in query response.")
 	f.DurationVar(&cfg.MaxQueryIntoFuture, "querier.max-query-into-future", 10*time.Minute, "Maximum duration into the future you can query. 0 to disable.")
 	f.DurationVar(&cfg.DefaultEvaluationInterval, "querier.default-evaluation-interval", time.Minute, "The default evaluation interval or step size for subqueries.")
@@ -159,7 +159,7 @@ func getChunksIteratorFunction(_ Config) chunkIteratorFunc {
 func New(cfg Config, limits *validation.Overrides, distributor Distributor, stores []QueryableWithFilter, reg prometheus.Registerer, logger log.Logger) (storage.SampleAndChunkQueryable, storage.ExemplarQueryable, promql.QueryEngine) {
 	iteratorFunc := getChunksIteratorFunction(cfg)
 
-	distributorQueryable := newDistributorQueryable(distributor, cfg.IngesterMetadataStreaming, iteratorFunc, cfg.QueryIngestersWithin, cfg.QueryStoreForLabels)
+	distributorQueryable := newDistributorQueryable(distributor, cfg.IngesterMetadataStreaming, iteratorFunc, cfg.QueryIngestersWithin)
 
 	ns := make([]QueryableWithFilter, len(stores))
 	for ix, s := range stores {
@@ -261,7 +261,6 @@ func NewQueryable(distributor QueryableWithFilter, stores []QueryableWithFilter,
 			chunkIterFn:          chunkIterFn,
 			limits:               limits,
 			maxQueryIntoFuture:   cfg.MaxQueryIntoFuture,
-			queryStoreForLabels:  cfg.QueryStoreForLabels,
 			ignoreMaxQueryLength: cfg.IgnoreMaxQueryLength,
 			distributor:          distributor,
 			stores:               stores,
@@ -277,12 +276,11 @@ type querier struct {
 	now         time.Time
 	mint, maxt  int64
 
-	limits              *validation.Overrides
-	maxQueryIntoFuture  time.Duration
-	queryStoreForLabels bool
-	distributor         QueryableWithFilter
-	stores              []QueryableWithFilter
-	limiterHolder       *limiterHolder
+	limits             *validation.Overrides
+	maxQueryIntoFuture time.Duration
+	distributor        QueryableWithFilter
+	stores             []QueryableWithFilter
+	limiterHolder      *limiterHolder
 
 	ignoreMaxQueryLength bool
 }
@@ -361,15 +359,6 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
 		}
 		// if SelectHints is null, rely on minT, maxT of querier to scope in range for Select stmt
 		sp = &storage.SelectHints{Start: mint, End: 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 metadataQuerier.Select(ctx, true, sp, matchers...)
 	}
 
 	// Validate query time range. Even if the time range has already been validated when we created
@@ -433,7 +422,7 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
 
 // LabelValues implements storage.Querier.
 func (q querier) LabelValues(ctx context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
-	ctx, stats, _, _, _, metadataQuerier, queriers, err := q.setupFromCtx(ctx)
+	ctx, stats, _, _, _, _, queriers, err := q.setupFromCtx(ctx)
 	if err == errEmptyTimeRange {
 		return nil, nil, nil
 	} else if err != nil {
@@ -444,10 +433,6 @@ func (q querier) LabelValues(ctx context.Context, name string, matchers ...*labe
 		stats.AddQueryStorageWallTime(time.Since(startT))
 	}()
 
-	if !q.queryStoreForLabels {
-		return metadataQuerier.LabelValues(ctx, name, matchers...)
-	}
-
 	if len(queriers) == 1 {
 		return queriers[0].LabelValues(ctx, name, matchers...)
 	}
@@ -487,7 +472,7 @@ func (q querier) LabelValues(ctx context.Context, name string, matchers ...*labe
 }
 
 func (q querier) LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
-	ctx, stats, _, _, _, metadataQuerier, queriers, err := q.setupFromCtx(ctx)
+	ctx, stats, _, _, _, _, queriers, err := q.setupFromCtx(ctx)
 	if err == errEmptyTimeRange {
 		return nil, nil, nil
 	} else if err != nil {
@@ -498,10 +483,6 @@ func (q querier) LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([
 		stats.AddQueryStorageWallTime(time.Since(startT))
 	}()
 
-	if !q.queryStoreForLabels {
-		return metadataQuerier.LabelNames(ctx, matchers...)
-	}
-
 	if len(queriers) == 1 {
 		return queriers[0].LabelNames(ctx, matchers...)
 	}