issues/5253 convert cache-unaligned-requests to per-tenant config

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [CHANGE] Store-gateway: skip verifying index header integrity upon loading. To enable verification set `blocks_storage.bucket_store.index_header.verify_on_load: true`.
 * [CHANGE] Querier: change the default value of the experimental `-querier.streaming-chunks-per-ingester-buffer-size` flag to 256. #5203
 * [CHANGE] Querier: only initiate query requests to ingesters in the `ACTIVE` state in the ring. #5342
+* [CHANGE] Querier: `-query-frontend.cache-unaligned-requests` has been moved from a global flag to a per-tenant override. #5312
 * [FEATURE] Cardinality API: Add a new `count_method` parameter which enables counting active series #5136
 * [FEATURE] Query-frontend: added experimental support to cache cardinality query responses. The cache will be used when `-query-frontend.cache-results` is enabled and `-query-frontend.results-cache-ttl-for-cardinality-query` set to a value greater than 0. The following metrics have been added to track the query results cache hit ratio per `request_type`: #5212 #5235
   * `cortex_frontend_query_result_cache_requests_total{request_type="query_range|cardinality"}`

cmd/mimir/config-descriptor.json

@@ -3214,6 +3214,17 @@
           "fieldType": "duration",
           "fieldCategory": "experimental"
         },
+        {
+          "kind": "field",
+          "name": "cache_unaligned_requests",
+          "required": false,
+          "desc": "Cache requests that are not step-aligned.",
+          "fieldValue": null,
+          "fieldDefaultValue": false,
+          "fieldFlag": "query-frontend.cache-unaligned-requests",
+          "fieldType": "boolean",
+          "fieldCategory": "advanced"
+        },
         {
           "kind": "field",
           "name": "max_query_expression_size_bytes",
@@ -4840,17 +4851,6 @@
           "fieldFlag": "query-frontend.parallelize-shardable-queries",
           "fieldType": "boolean"
         },
-        {
-          "kind": "field",
-          "name": "cache_unaligned_requests",
-          "required": false,
-          "desc": "Cache requests that are not step-aligned.",
-          "fieldValue": null,
-          "fieldDefaultValue": false,
-          "fieldFlag": "query-frontend.cache-unaligned-requests",
-          "fieldType": "boolean",
-          "fieldCategory": "advanced"
-        },
         {
           "kind": "field",
           "name": "query_sharding_target_series_per_shard",

docs/sources/mimir/references/configuration-parameters/index.md

@@ -1266,10 +1266,6 @@ results_cache:
 # CLI flag: -query-frontend.parallelize-shardable-queries
 [parallelize_shardable_queries: <boolean> | default = false]
 
-# (advanced) Cache requests that are not step-aligned.
-# CLI flag: -query-frontend.cache-unaligned-requests
-[cache_unaligned_requests: <boolean> | default = false]
-
 # How many series a single sharded partial query should load at most. This is
 # not a strict requirement guaranteed to be honoured by query sharding, but a
 # hint given to the query sharding when the query execution is initially
@@ -2897,6 +2893,10 @@ The `limits` block configures default and per-tenant limits imposed by component
 # CLI flag: -query-frontend.results-cache-ttl-for-cardinality-query
 [results_cache_ttl_for_cardinality_query: <duration> | default = 0s]
 
+# (advanced) Cache requests that are not step-aligned.
+# CLI flag: -query-frontend.cache-unaligned-requests
+[cache_unaligned_requests: <boolean> | default = false]
+
 # (experimental) Max size of the raw query, in bytes. 0 to not apply a limit to
 # the size of the query.
 # CLI flag: -query-frontend.max-query-expression-size-bytes

pkg/frontend/querymiddleware/limits.go

@@ -88,6 +88,9 @@ type Limits interface {
 
 	// ResultsCacheTTLForCardinalityQuery returns TTL for cached results for cardinality queries.
 	ResultsCacheTTLForCardinalityQuery(userID string) time.Duration
+
+	// ResultsCacheForUnalignedQueryEnabled returns whether to cache results for queries that are not step-aligned
+	ResultsCacheForUnalignedQueryEnabled(userID string) bool
 }
 
 type limitsMiddleware struct {

pkg/frontend/querymiddleware/limits_test.go

@@ -419,6 +419,10 @@ func (m multiTenantMockLimits) ResultsCacheTTLForCardinalityQuery(userID string)
 	return m.byTenant[userID].resultsCacheTTLForCardinalityQuery
 }
 
+func (m multiTenantMockLimits) ResultsCacheForUnalignedQueryEnabled(userID string) bool {
+	return m.byTenant[userID].resultsCacheForUnalignedQueryEnabled
+}
+
 func (m multiTenantMockLimits) CreationGracePeriod(userID string) time.Duration {
 	return m.byTenant[userID].creationGracePeriod
 }
@@ -428,24 +432,25 @@ func (m multiTenantMockLimits) NativeHistogramsIngestionEnabled(userID string) b
 }
 
 type mockLimits struct {
-	maxQueryLookback                   time.Duration
-	maxQueryLength                     time.Duration
-	maxTotalQueryLength                time.Duration
-	maxQueryExpressionSizeBytes        int
-	maxCacheFreshness                  time.Duration
-	maxQueryParallelism                int
-	maxShardedQueries                  int
-	maxRegexpSizeBytes                 int
-	splitInstantQueriesInterval        time.Duration
-	totalShards                        int
-	compactorShards                    int
-	compactorBlocksRetentionPeriod     time.Duration
-	outOfOrderTimeWindow               time.Duration
-	creationGracePeriod                time.Duration
-	nativeHistogramsIngestionEnabled   bool
-	resultsCacheTTL                    time.Duration
-	resultsCacheOutOfOrderWindowTTL    time.Duration
-	resultsCacheTTLForCardinalityQuery time.Duration
+	maxQueryLookback                     time.Duration
+	maxQueryLength                       time.Duration
+	maxTotalQueryLength                  time.Duration
+	maxQueryExpressionSizeBytes          int
+	maxCacheFreshness                    time.Duration
+	maxQueryParallelism                  int
+	maxShardedQueries                    int
+	maxRegexpSizeBytes                   int
+	splitInstantQueriesInterval          time.Duration
+	totalShards                          int
+	compactorShards                      int
+	compactorBlocksRetentionPeriod       time.Duration
+	outOfOrderTimeWindow                 time.Duration
+	creationGracePeriod                  time.Duration
+	nativeHistogramsIngestionEnabled     bool
+	resultsCacheTTL                      time.Duration
+	resultsCacheOutOfOrderWindowTTL      time.Duration
+	resultsCacheTTLForCardinalityQuery   time.Duration
+	resultsCacheForUnalignedQueryEnabled bool
 }
 
 func (m mockLimits) MaxQueryLookback(string) time.Duration {
@@ -514,6 +519,10 @@ func (m mockLimits) ResultsCacheTTLForCardinalityQuery(string) time.Duration {
 	return m.resultsCacheTTLForCardinalityQuery
 }
 
+func (m mockLimits) ResultsCacheForUnalignedQueryEnabled(string) bool {
+	return m.resultsCacheForUnalignedQueryEnabled
+}
+
 func (m mockLimits) CreationGracePeriod(string) time.Duration {
 	return m.creationGracePeriod
 }

pkg/frontend/querymiddleware/roundtrip.go

@@ -32,18 +32,22 @@ const (
 	instantQueryPathSuffix           = "/query"
 	cardinalityLabelNamesPathSuffix  = "/cardinality/label_names"
 	cardinalityLabelValuesPathSuffix = "/cardinality/label_values"
+
+	// DefaultDeprecatedCacheUnalignedRequests is the default value for the deprecated querier frontend config DeprecatedCacheUnalignedRequests
+	// which has been moved to a per-tenant limit; TODO remove in Mimir 2.12
+	DefaultDeprecatedCacheUnalignedRequests = false
 )
 
 // Config for query_range middleware chain.
 type Config struct {
-	SplitQueriesByInterval time.Duration `yaml:"split_queries_by_interval" category:"advanced"`
-	AlignQueriesWithStep   bool          `yaml:"align_queries_with_step"`
-	ResultsCacheConfig     `yaml:"results_cache"`
-	CacheResults           bool   `yaml:"cache_results"`
-	MaxRetries             int    `yaml:"max_retries" category:"advanced"`
-	ShardedQueries         bool   `yaml:"parallelize_shardable_queries"`
-	CacheUnalignedRequests bool   `yaml:"cache_unaligned_requests" category:"advanced"`
-	TargetSeriesPerShard   uint64 `yaml:"query_sharding_target_series_per_shard"`
+	SplitQueriesByInterval           time.Duration `yaml:"split_queries_by_interval" category:"advanced"`
+	AlignQueriesWithStep             bool          `yaml:"align_queries_with_step"`
+	ResultsCacheConfig               `yaml:"results_cache"`
+	CacheResults                     bool   `yaml:"cache_results"`
+	MaxRetries                       int    `yaml:"max_retries" category:"advanced"`
+	ShardedQueries                   bool   `yaml:"parallelize_shardable_queries"`
+	DeprecatedCacheUnalignedRequests bool   `yaml:"cache_unaligned_requests" category:"advanced" doc:"hidden"` // Deprecated: Deprecated in Mimir 2.10.0, remove in Mimir 2.12.0 (https://github.com/grafana/mimir/issues/5253)
+	TargetSeriesPerShard             uint64 `yaml:"query_sharding_target_series_per_shard"`
 
 	// CacheSplitter allows to inject a CacheSplitter to use for generating cache keys.
 	// If nil, the querymiddleware package uses a ConstSplitter with SplitQueriesByInterval.
@@ -59,10 +63,15 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
 	f.BoolVar(&cfg.AlignQueriesWithStep, "query-frontend.align-queries-with-step", false, "Mutate incoming queries to align their start and end with their step.")
 	f.BoolVar(&cfg.CacheResults, "query-frontend.cache-results", false, "Cache query results.")
 	f.BoolVar(&cfg.ShardedQueries, "query-frontend.parallelize-shardable-queries", false, "True to enable query sharding.")
-	f.BoolVar(&cfg.CacheUnalignedRequests, "query-frontend.cache-unaligned-requests", false, "Cache requests that are not step-aligned.")
 	f.Uint64Var(&cfg.TargetSeriesPerShard, "query-frontend.query-sharding-target-series-per-shard", 0, "How many series a single sharded partial query should load at most. This is not a strict requirement guaranteed to be honoured by query sharding, but a hint given to the query sharding when the query execution is initially planned. 0 to disable cardinality-based hints.")
 	f.StringVar(&cfg.QueryResultResponseFormat, "query-frontend.query-result-response-format", formatProtobuf, fmt.Sprintf("Format to use when retrieving query results from queriers. Supported values: %s", strings.Join(allFormats, ", ")))
 	cfg.ResultsCacheConfig.RegisterFlags(f)
+
+	// The query-frontend.cache-unaligned-requests flag has been moved to the limits.go file
+	// cfg.DeprecatedCacheUnalignedRequests is set to the default here for clarity
+	// and consistency with the process for migrating limits to per-tenant config
+	// TODO: Remove in Mimir 2.12.0
+	cfg.DeprecatedCacheUnalignedRequests = DefaultDeprecatedCacheUnalignedRequests
 }
 
 // Validate validates the config.
@@ -220,7 +229,6 @@ func newQueryTripperware(
 			cfg.SplitQueriesByInterval > 0,
 			cfg.CacheResults,
 			cfg.SplitQueriesByInterval,
-			cfg.CacheUnalignedRequests,
 			limits,
 			codec,
 			c,

pkg/frontend/querymiddleware/split_and_cache.go

@@ -91,12 +91,11 @@ type splitAndCacheMiddleware struct {
 	splitInterval time.Duration
 
 	// Results caching.
-	cacheEnabled           bool
-	cacheUnalignedRequests bool
-	cache                  cache.Cache
-	splitter               CacheSplitter
-	extractor              Extractor
-	shouldCacheReq         shouldCacheFn
+	cacheEnabled   bool
+	cache          cache.Cache
+	splitter       CacheSplitter
+	extractor      Extractor
+	shouldCacheReq shouldCacheFn
 
 	// Can be set from tests
 	currentTime func() time.Time
@@ -107,7 +106,6 @@ func newSplitAndCacheMiddleware(
 	splitEnabled bool,
 	cacheEnabled bool,
 	splitInterval time.Duration,
-	cacheUnalignedRequests bool,
 	limits Limits,
 	merger Merger,
 	cache cache.Cache,
@@ -120,20 +118,19 @@ func newSplitAndCacheMiddleware(
 
 	return MiddlewareFunc(func(next Handler) Handler {
 		return &splitAndCacheMiddleware{
-			splitEnabled:           splitEnabled,
-			cacheEnabled:           cacheEnabled,
-			cacheUnalignedRequests: cacheUnalignedRequests,
-			next:                   next,
-			limits:                 limits,
-			merger:                 merger,
-			splitInterval:          splitInterval,
-			metrics:                metrics,
-			cache:                  cache,
-			splitter:               splitter,
-			extractor:              extractor,
-			shouldCacheReq:         shouldCacheReq,
-			logger:                 logger,
-			currentTime:            time.Now,
+			splitEnabled:   splitEnabled,
+			cacheEnabled:   cacheEnabled,
+			next:           next,
+			limits:         limits,
+			merger:         merger,
+			splitInterval:  splitInterval,
+			metrics:        metrics,
+			cache:          cache,
+			splitter:       splitter,
+			extractor:      extractor,
+			shouldCacheReq: shouldCacheReq,
+			logger:         logger,
+			currentTime:    time.Now,
 		}
 	})
 }
@@ -154,6 +151,7 @@ func (s *splitAndCacheMiddleware) Do(ctx context.Context, req Request) (Response
 	isCacheEnabled := s.cacheEnabled && (s.shouldCacheReq == nil || s.shouldCacheReq(req))
 	maxCacheFreshness := validation.MaxDurationPerTenant(tenantIDs, s.limits.MaxCacheFreshness)
 	maxCacheTime := int64(model.Now().Add(-maxCacheFreshness))
+	cacheUnalignedRequests := validation.AllTrueBooleansPerTenant(tenantIDs, s.limits.ResultsCacheForUnalignedQueryEnabled)
 
 	// Lookup the results cache.
 	if isCacheEnabled {
@@ -165,7 +163,7 @@ func (s *splitAndCacheMiddleware) Do(ctx context.Context, req Request) (Response
 
 		for _, splitReq := range splitReqs {
 			// Do not try to pick response from cache at all if the request is not cachable.
-			if cachable, reason := isRequestCachable(splitReq.orig, maxCacheTime, s.cacheUnalignedRequests, s.logger); !cachable {
+			if cachable, reason := isRequestCachable(splitReq.orig, maxCacheTime, cacheUnalignedRequests, s.logger); !cachable {
 				splitReq.downstreamRequests = []Request{splitReq.orig}
 				s.metrics.queryResultCacheSkippedCount.WithLabelValues(reason).Inc()
 				continue
@@ -247,7 +245,7 @@ func (s *splitAndCacheMiddleware) Do(ctx context.Context, req Request) (Response
 			}
 
 			// Skip caching if the request is not cachable.
-			if cachable, _ := isRequestCachable(splitReq.orig, maxCacheTime, s.cacheUnalignedRequests, s.logger); !cachable {
+			if cachable, _ := isRequestCachable(splitReq.orig, maxCacheTime, cacheUnalignedRequests, s.logger); !cachable {
 				continue
 			}
 

pkg/frontend/querymiddleware/split_and_cache_test.go

@@ -170,7 +170,6 @@ func TestSplitAndCacheMiddleware_SplitByInterval(t *testing.T) {
 		true,
 		false, // Cache disabled.
 		24*time.Hour,
-		false,
 		mockLimits{},
 		codec,
 		nil,
@@ -242,7 +241,6 @@ func TestSplitAndCacheMiddleware_ResultsCache(t *testing.T) {
 		true,
 		true,
 		24*time.Hour,
-		false,
 		mockLimits{maxCacheFreshness: 10 * time.Minute, resultsCacheTTL: resultsCacheTTL, resultsCacheOutOfOrderWindowTTL: resultsCacheLowerTTL},
 		newTestPrometheusCodec(),
 		cacheBackend,
@@ -375,7 +373,6 @@ func TestSplitAndCacheMiddleware_ResultsCache_ShouldNotLookupCacheIfStepIsNotAli
 		true,
 		true,
 		24*time.Hour,
-		false,
 		mockLimits{maxCacheFreshness: 10 * time.Minute},
 		newTestPrometheusCodec(),
 		cacheBackend,
@@ -481,12 +478,18 @@ func TestSplitAndCacheMiddleware_ResultsCache_ShouldNotLookupCacheIfStepIsNotAli
 func TestSplitAndCacheMiddleware_ResultsCache_EnabledCachingOfStepUnalignedRequest(t *testing.T) {
 	cacheBackend := cache.NewInstrumentedMockCache()
 
+	limits := mockLimits{
+		maxCacheFreshness:                    10 * time.Minute,
+		resultsCacheTTL:                      resultsCacheTTL,
+		resultsCacheOutOfOrderWindowTTL:      resultsCacheLowerTTL,
+		resultsCacheForUnalignedQueryEnabled: true,
+	}
+
 	mw := newSplitAndCacheMiddleware(
 		true,
 		true,
 		24*time.Hour,
-		true, // caching of step-unaligned requests is enabled in this test.
-		mockLimits{maxCacheFreshness: 10 * time.Minute, resultsCacheTTL: resultsCacheTTL, resultsCacheOutOfOrderWindowTTL: resultsCacheLowerTTL},
+		limits,
 		newTestPrometheusCodec(),
 		cacheBackend,
 		ConstSplitter(day),
@@ -647,7 +650,6 @@ func TestSplitAndCacheMiddleware_ResultsCache_ShouldNotCacheRequestEarlierThanMa
 				false, // No interval splitting.
 				true,
 				24*time.Hour,
-				false,
 				mockLimits{maxCacheFreshness: maxCacheFreshness, resultsCacheTTL: resultsCacheTTL, resultsCacheOutOfOrderWindowTTL: resultsCacheLowerTTL},
 				newTestPrometheusCodec(),
 				cacheBackend,
@@ -856,7 +858,6 @@ func TestSplitAndCacheMiddleware_ResultsCacheFuzzy(t *testing.T) {
 					testData.splitEnabled,
 					testData.cacheEnabled,
 					24*time.Hour,
-					testData.cacheUnaligned,
 					mockLimits{
 						maxCacheFreshness:   testData.maxCacheFreshness,
 						maxQueryParallelism: testData.maxQueryParallelism,
@@ -1139,7 +1140,6 @@ func TestSplitAndCacheMiddleware_ResultsCache_ExtentsEdgeCases(t *testing.T) {
 				false, // No splitting.
 				true,
 				24*time.Hour,
-				false,
 				mockLimits{resultsCacheTTL: resultsCacheTTL, resultsCacheOutOfOrderWindowTTL: resultsCacheLowerTTL},
 				newTestPrometheusCodec(),
 				cacheBackend,
@@ -1185,7 +1185,6 @@ func TestSplitAndCacheMiddleware_StoreAndFetchCacheExtents(t *testing.T) {
 		false,
 		true,
 		24*time.Hour,
-		false,
 		mockLimits{
 			resultsCacheTTL:                 1 * time.Hour,
 			resultsCacheOutOfOrderWindowTTL: 10 * time.Minute,
@@ -1271,7 +1270,6 @@ func TestSplitAndCacheMiddleware_WrapMultipleTimes(t *testing.T) {
 		false,
 		true,
 		24*time.Hour,
-		false,
 		mockLimits{},
 		newTestPrometheusCodec(),
 		cache.NewMockCache(),

pkg/mimir/modules.go

@@ -278,6 +278,14 @@ func (t *Mimir) initRuntimeConfig() (services.Service, error) {
 		t.Cfg.LimitsConfig.QueryIngestersWithin = model.Duration(t.Cfg.Querier.QueryIngestersWithin)
 	}
 
+	// DeprecatedCacheUnalignedRequests is moving from a global config that can in the frontend yaml to a limit config
+	// We need to preserve the option in the frontend yaml for two releases
+	// If the frontend config is configured by the user, the default limit is overwritten
+	// TODO: Remove in Mimir 2.12.0
+	if t.Cfg.Frontend.QueryMiddleware.DeprecatedCacheUnalignedRequests != querymiddleware.DefaultDeprecatedCacheUnalignedRequests {
+		t.Cfg.LimitsConfig.ResultsCacheForUnalignedQueryEnabled = t.Cfg.Frontend.QueryMiddleware.DeprecatedCacheUnalignedRequests
+	}
+
 	// make sure to set default limits before we start loading configuration into memory
 	validation.SetDefaultLimitsForYAMLUnmarshalling(t.Cfg.LimitsConfig)
 	ingester.SetDefaultInstanceLimitsForYAMLUnmarshalling(t.Cfg.Ingester.DefaultLimits)