From 88fe58d592e31c9d91a556f3d337fff41150b6dc Mon Sep 17 00:00:00 2001 From: francoposa Date: Thu, 22 Jun 2023 11:14:03 -0700 Subject: [PATCH 1/7] issues/5253 move cache-unaligned-requests flag/config declaration & generate doc --- cmd/mimir/config-descriptor.json | 22 ++++---- .../configuration-parameters/index.md | 8 +-- pkg/frontend/querymiddleware/roundtrip.go | 3 +- pkg/util/validation/limits.go | 51 ++++++++++--------- 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 9c113000141..6d5497b001f 100644 --- a/cmd/mimir/config-descriptor.json +++ b/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", diff --git a/docs/sources/mimir/references/configuration-parameters/index.md b/docs/sources/mimir/references/configuration-parameters/index.md index 2af80740478..dbe1e930bfd 100644 --- a/docs/sources/mimir/references/configuration-parameters/index.md +++ b/docs/sources/mimir/references/configuration-parameters/index.md @@ -1266,10 +1266,6 @@ results_cache: # CLI flag: -query-frontend.parallelize-shardable-queries [parallelize_shardable_queries: | default = false] -# (advanced) Cache requests that are not step-aligned. -# CLI flag: -query-frontend.cache-unaligned-requests -[cache_unaligned_requests: | 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: | default = 0s] +# (advanced) Cache requests that are not step-aligned. +# CLI flag: -query-frontend.cache-unaligned-requests +[cache_unaligned_requests: | 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 diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 033a8f51732..23bffccb87f 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -42,7 +42,7 @@ type Config struct { 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"` + CacheUnalignedRequests bool `yaml:"cache_unaligned_requests" category:"advanced" doc:"hidden"` // TODO: Deprecated in Mimir 2.10.0, remove in Mimir 2.12.0 TargetSeriesPerShard uint64 `yaml:"query_sharding_target_series_per_shard"` // CacheSplitter allows to inject a CacheSplitter to use for generating cache keys. @@ -59,7 +59,6 @@ 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) diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 7f942e6aa02..32bc2859375 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -26,30 +26,31 @@ import ( ) const ( - MaxSeriesPerMetricFlag = "ingester.max-global-series-per-metric" - MaxMetadataPerMetricFlag = "ingester.max-global-metadata-per-metric" - MaxSeriesPerUserFlag = "ingester.max-global-series-per-user" - MaxMetadataPerUserFlag = "ingester.max-global-metadata-per-user" - MaxChunksPerQueryFlag = "querier.max-fetched-chunks-per-query" - MaxChunkBytesPerQueryFlag = "querier.max-fetched-chunk-bytes-per-query" - MaxSeriesPerQueryFlag = "querier.max-fetched-series-per-query" - maxLabelNamesPerSeriesFlag = "validation.max-label-names-per-series" - maxLabelNameLengthFlag = "validation.max-length-label-name" - maxLabelValueLengthFlag = "validation.max-length-label-value" - maxMetadataLengthFlag = "validation.max-metadata-length" - maxNativeHistogramBucketsFlag = "validation.max-native-histogram-buckets" - creationGracePeriodFlag = "validation.create-grace-period" - maxPartialQueryLengthFlag = "querier.max-partial-query-length" - maxTotalQueryLengthFlag = "query-frontend.max-total-query-length" - maxQueryExpressionSizeBytesFlag = "query-frontend.max-query-expression-size-bytes" - requestRateFlag = "distributor.request-rate-limit" - requestBurstSizeFlag = "distributor.request-burst-size" - ingestionRateFlag = "distributor.ingestion-rate-limit" - ingestionBurstSizeFlag = "distributor.ingestion-burst-size" - HATrackerMaxClustersFlag = "distributor.ha-tracker.max-clusters" - resultsCacheTTLFlag = "query-frontend.results-cache-ttl" - resultsCacheTTLForOutOfOrderWindowFlag = "query-frontend.results-cache-ttl-for-out-of-order-time-window" - QueryIngestersWithinFlag = "querier.query-ingesters-within" + MaxSeriesPerMetricFlag = "ingester.max-global-series-per-metric" + MaxMetadataPerMetricFlag = "ingester.max-global-metadata-per-metric" + MaxSeriesPerUserFlag = "ingester.max-global-series-per-user" + MaxMetadataPerUserFlag = "ingester.max-global-metadata-per-user" + MaxChunksPerQueryFlag = "querier.max-fetched-chunks-per-query" + MaxChunkBytesPerQueryFlag = "querier.max-fetched-chunk-bytes-per-query" + MaxSeriesPerQueryFlag = "querier.max-fetched-series-per-query" + maxLabelNamesPerSeriesFlag = "validation.max-label-names-per-series" + maxLabelNameLengthFlag = "validation.max-length-label-name" + maxLabelValueLengthFlag = "validation.max-length-label-value" + maxMetadataLengthFlag = "validation.max-metadata-length" + maxNativeHistogramBucketsFlag = "validation.max-native-histogram-buckets" + creationGracePeriodFlag = "validation.create-grace-period" + maxPartialQueryLengthFlag = "querier.max-partial-query-length" + maxTotalQueryLengthFlag = "query-frontend.max-total-query-length" + maxQueryExpressionSizeBytesFlag = "query-frontend.max-query-expression-size-bytes" + requestRateFlag = "distributor.request-rate-limit" + requestBurstSizeFlag = "distributor.request-burst-size" + ingestionRateFlag = "distributor.ingestion-rate-limit" + ingestionBurstSizeFlag = "distributor.ingestion-burst-size" + HATrackerMaxClustersFlag = "distributor.ha-tracker.max-clusters" + resultsCacheTTLFlag = "query-frontend.results-cache-ttl" + resultsCacheTTLForOutOfOrderWindowFlag = "query-frontend.results-cache-ttl-for-out-of-order-time-window" + resultsCacheForUnalignedQueryEnabledFlag = "query-frontend.cache-unaligned-requests" + QueryIngestersWithinFlag = "querier.query-ingesters-within" // MinCompactorPartialBlockDeletionDelay is the minimum partial blocks deletion delay that can be configured in Mimir. MinCompactorPartialBlockDeletionDelay = 4 * time.Hour @@ -126,6 +127,7 @@ type Limits struct { ResultsCacheTTL model.Duration `yaml:"results_cache_ttl" json:"results_cache_ttl" category:"experimental"` ResultsCacheTTLForOutOfOrderTimeWindow model.Duration `yaml:"results_cache_ttl_for_out_of_order_time_window" json:"results_cache_ttl_for_out_of_order_time_window" category:"experimental"` ResultsCacheTTLForCardinalityQuery model.Duration `yaml:"results_cache_ttl_for_cardinality_query" json:"results_cache_ttl_for_cardinality_query" category:"experimental"` + ResultsCacheForUnalignedQueryEnabled bool `yaml:"cache_unaligned_requests" json:"cache_unaligned_requests" category:"advanced"` MaxQueryExpressionSizeBytes int `yaml:"max_query_expression_size_bytes" json:"max_query_expression_size_bytes" category:"experimental"` // Cardinality @@ -261,6 +263,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { _ = l.ResultsCacheTTLForOutOfOrderTimeWindow.Set("10m") f.Var(&l.ResultsCacheTTLForOutOfOrderTimeWindow, resultsCacheTTLForOutOfOrderWindowFlag, fmt.Sprintf("Time to live duration for cached query results if query falls into out-of-order time window. This is lower than -%s so that incoming out-of-order samples are returned in the query results sooner.", resultsCacheTTLFlag)) f.Var(&l.ResultsCacheTTLForCardinalityQuery, "query-frontend.results-cache-ttl-for-cardinality-query", "Time to live duration for cached cardinality query results. The value 0 disables the cache.") + f.BoolVar(&l.ResultsCacheForUnalignedQueryEnabled, resultsCacheForUnalignedQueryEnabledFlag, false, "Cache requests that are not step-aligned.") f.IntVar(&l.MaxQueryExpressionSizeBytes, maxQueryExpressionSizeBytesFlag, 0, "Max size of the raw query, in bytes. 0 to not apply a limit to the size of the query.") // Store-gateway. From e7b13071ceb83d8e064e0f8ddfa75f7162d83f3a Mon Sep 17 00:00:00 2001 From: francoposa Date: Wed, 28 Jun 2023 10:43:13 -0700 Subject: [PATCH 2/7] issues/5253 decide cache-unaligned-requests with limits overrides instead of global config --- pkg/frontend/querymiddleware/limits.go | 3 ++ pkg/frontend/querymiddleware/roundtrip.go | 1 - .../querymiddleware/split_and_cache.go | 44 +++++++++---------- pkg/util/validation/limits.go | 14 ++++++ 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/pkg/frontend/querymiddleware/limits.go b/pkg/frontend/querymiddleware/limits.go index 08df1cededb..bb65e83310a 100644 --- a/pkg/frontend/querymiddleware/limits.go +++ b/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(user string) bool } type limitsMiddleware struct { diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 23bffccb87f..0fcd4722ee5 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -219,7 +219,6 @@ func newQueryTripperware( cfg.SplitQueriesByInterval > 0, cfg.CacheResults, cfg.SplitQueriesByInterval, - cfg.CacheUnalignedRequests, limits, codec, c, diff --git a/pkg/frontend/querymiddleware/split_and_cache.go b/pkg/frontend/querymiddleware/split_and_cache.go index e187bcbffca..c63c33f3fb6 100644 --- a/pkg/frontend/querymiddleware/split_and_cache.go +++ b/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 } diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 32bc2859375..2ccd42a7513 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -837,6 +837,10 @@ func (o *Overrides) ResultsCacheTTLForCardinalityQuery(user string) time.Duratio return time.Duration(o.getOverridesForUser(user).ResultsCacheTTLForCardinalityQuery) } +func (o *Overrides) ResultsCacheForUnalignedQueryEnabled(user string) bool { + return o.getOverridesForUser(user).ResultsCacheForUnalignedQueryEnabled +} + func (o *Overrides) getOverridesForUser(userID string) *Limits { if o.tenantLimits != nil { l := o.tenantLimits.ByUserID(userID) @@ -847,6 +851,16 @@ func (o *Overrides) getOverridesForUser(userID string) *Limits { return o.defaultLimits } +// AllTrueBooleansPerTenant returns true only if limit func is true for all given tenants +func AllTrueBooleansPerTenant(tenantIDs []string, f func(string) bool) bool { + for _, tenantID := range tenantIDs { + if !f(tenantID) { + return false + } + } + return true +} + // SmallestPositiveIntPerTenant is returning the minimal positive value of the // supplied limit function for all given tenants. func SmallestPositiveIntPerTenant(tenantIDs []string, f func(string) int) int { From 5eae6fdfe98a24a52096f0fb622b7de843167e5a Mon Sep 17 00:00:00 2001 From: francoposa Date: Wed, 28 Jun 2023 10:59:34 -0700 Subject: [PATCH 3/7] issues/5253 passing linting and compiling tests; tests still fail --- pkg/frontend/querymiddleware/limits.go | 2 +- pkg/frontend/querymiddleware/limits_test.go | 45 +++++++++++-------- .../querymiddleware/split_and_cache_test.go | 9 ---- pkg/util/validation/limits.go | 4 +- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pkg/frontend/querymiddleware/limits.go b/pkg/frontend/querymiddleware/limits.go index bb65e83310a..39f863b32b8 100644 --- a/pkg/frontend/querymiddleware/limits.go +++ b/pkg/frontend/querymiddleware/limits.go @@ -90,7 +90,7 @@ type Limits interface { ResultsCacheTTLForCardinalityQuery(userID string) time.Duration // ResultsCacheForUnalignedQueryEnabled returns whether to cache results for queries that are not step-aligned - ResultsCacheForUnalignedQueryEnabled(user string) bool + ResultsCacheForUnalignedQueryEnabled(userID string) bool } type limitsMiddleware struct { diff --git a/pkg/frontend/querymiddleware/limits_test.go b/pkg/frontend/querymiddleware/limits_test.go index e3256d1dd53..e8be1499018 100644 --- a/pkg/frontend/querymiddleware/limits_test.go +++ b/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 } diff --git a/pkg/frontend/querymiddleware/split_and_cache_test.go b/pkg/frontend/querymiddleware/split_and_cache_test.go index b9321c06593..5c216267717 100644 --- a/pkg/frontend/querymiddleware/split_and_cache_test.go +++ b/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, @@ -485,7 +482,6 @@ func TestSplitAndCacheMiddleware_ResultsCache_EnabledCachingOfStepUnalignedReque 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}, newTestPrometheusCodec(), cacheBackend, @@ -647,7 +643,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 +851,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 +1133,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 +1178,6 @@ func TestSplitAndCacheMiddleware_StoreAndFetchCacheExtents(t *testing.T) { false, true, 24*time.Hour, - false, mockLimits{ resultsCacheTTL: 1 * time.Hour, resultsCacheOutOfOrderWindowTTL: 10 * time.Minute, @@ -1271,7 +1263,6 @@ func TestSplitAndCacheMiddleware_WrapMultipleTimes(t *testing.T) { false, true, 24*time.Hour, - false, mockLimits{}, newTestPrometheusCodec(), cache.NewMockCache(), diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index 2ccd42a7513..b833ca612f0 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -837,8 +837,8 @@ func (o *Overrides) ResultsCacheTTLForCardinalityQuery(user string) time.Duratio return time.Duration(o.getOverridesForUser(user).ResultsCacheTTLForCardinalityQuery) } -func (o *Overrides) ResultsCacheForUnalignedQueryEnabled(user string) bool { - return o.getOverridesForUser(user).ResultsCacheForUnalignedQueryEnabled +func (o *Overrides) ResultsCacheForUnalignedQueryEnabled(userID string) bool { + return o.getOverridesForUser(userID).ResultsCacheForUnalignedQueryEnabled } func (o *Overrides) getOverridesForUser(userID string) *Limits { From 5c3222b6f60ccea4af33a693d46797f83ecf3659 Mon Sep 17 00:00:00 2001 From: francoposa Date: Wed, 28 Jun 2023 13:58:43 -0700 Subject: [PATCH 4/7] issues/5253 passing tests --- pkg/frontend/querymiddleware/split_and_cache_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/frontend/querymiddleware/split_and_cache_test.go b/pkg/frontend/querymiddleware/split_and_cache_test.go index 5c216267717..2d7eeadd93b 100644 --- a/pkg/frontend/querymiddleware/split_and_cache_test.go +++ b/pkg/frontend/querymiddleware/split_and_cache_test.go @@ -478,11 +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, - mockLimits{maxCacheFreshness: 10 * time.Minute, resultsCacheTTL: resultsCacheTTL, resultsCacheOutOfOrderWindowTTL: resultsCacheLowerTTL}, + limits, newTestPrometheusCodec(), cacheBackend, ConstSplitter(day), From 76a7592ee3c96e31ee25e8277764fd9f148a31f3 Mon Sep 17 00:00:00 2001 From: francoposa Date: Thu, 29 Jun 2023 07:33:38 -0700 Subject: [PATCH 5/7] issues/5253 pr nits --- pkg/frontend/querymiddleware/roundtrip.go | 16 +++---- pkg/util/validation/limits.go | 51 +++++++++++------------ 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 0fcd4722ee5..88757a690cf 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -36,14 +36,14 @@ const ( // 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" doc:"hidden"` // TODO: Deprecated in Mimir 2.10.0, remove in Mimir 2.12.0 - 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"` // TODO: Deprecated in Mimir 2.10.0, remove in Mimir 2.12.0 + 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. diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index b833ca612f0..32f5f85d59c 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -26,31 +26,30 @@ import ( ) const ( - MaxSeriesPerMetricFlag = "ingester.max-global-series-per-metric" - MaxMetadataPerMetricFlag = "ingester.max-global-metadata-per-metric" - MaxSeriesPerUserFlag = "ingester.max-global-series-per-user" - MaxMetadataPerUserFlag = "ingester.max-global-metadata-per-user" - MaxChunksPerQueryFlag = "querier.max-fetched-chunks-per-query" - MaxChunkBytesPerQueryFlag = "querier.max-fetched-chunk-bytes-per-query" - MaxSeriesPerQueryFlag = "querier.max-fetched-series-per-query" - maxLabelNamesPerSeriesFlag = "validation.max-label-names-per-series" - maxLabelNameLengthFlag = "validation.max-length-label-name" - maxLabelValueLengthFlag = "validation.max-length-label-value" - maxMetadataLengthFlag = "validation.max-metadata-length" - maxNativeHistogramBucketsFlag = "validation.max-native-histogram-buckets" - creationGracePeriodFlag = "validation.create-grace-period" - maxPartialQueryLengthFlag = "querier.max-partial-query-length" - maxTotalQueryLengthFlag = "query-frontend.max-total-query-length" - maxQueryExpressionSizeBytesFlag = "query-frontend.max-query-expression-size-bytes" - requestRateFlag = "distributor.request-rate-limit" - requestBurstSizeFlag = "distributor.request-burst-size" - ingestionRateFlag = "distributor.ingestion-rate-limit" - ingestionBurstSizeFlag = "distributor.ingestion-burst-size" - HATrackerMaxClustersFlag = "distributor.ha-tracker.max-clusters" - resultsCacheTTLFlag = "query-frontend.results-cache-ttl" - resultsCacheTTLForOutOfOrderWindowFlag = "query-frontend.results-cache-ttl-for-out-of-order-time-window" - resultsCacheForUnalignedQueryEnabledFlag = "query-frontend.cache-unaligned-requests" - QueryIngestersWithinFlag = "querier.query-ingesters-within" + MaxSeriesPerMetricFlag = "ingester.max-global-series-per-metric" + MaxMetadataPerMetricFlag = "ingester.max-global-metadata-per-metric" + MaxSeriesPerUserFlag = "ingester.max-global-series-per-user" + MaxMetadataPerUserFlag = "ingester.max-global-metadata-per-user" + MaxChunksPerQueryFlag = "querier.max-fetched-chunks-per-query" + MaxChunkBytesPerQueryFlag = "querier.max-fetched-chunk-bytes-per-query" + MaxSeriesPerQueryFlag = "querier.max-fetched-series-per-query" + maxLabelNamesPerSeriesFlag = "validation.max-label-names-per-series" + maxLabelNameLengthFlag = "validation.max-length-label-name" + maxLabelValueLengthFlag = "validation.max-length-label-value" + maxMetadataLengthFlag = "validation.max-metadata-length" + maxNativeHistogramBucketsFlag = "validation.max-native-histogram-buckets" + creationGracePeriodFlag = "validation.create-grace-period" + maxPartialQueryLengthFlag = "querier.max-partial-query-length" + maxTotalQueryLengthFlag = "query-frontend.max-total-query-length" + maxQueryExpressionSizeBytesFlag = "query-frontend.max-query-expression-size-bytes" + requestRateFlag = "distributor.request-rate-limit" + requestBurstSizeFlag = "distributor.request-burst-size" + ingestionRateFlag = "distributor.ingestion-rate-limit" + ingestionBurstSizeFlag = "distributor.ingestion-burst-size" + HATrackerMaxClustersFlag = "distributor.ha-tracker.max-clusters" + resultsCacheTTLFlag = "query-frontend.results-cache-ttl" + resultsCacheTTLForOutOfOrderWindowFlag = "query-frontend.results-cache-ttl-for-out-of-order-time-window" + QueryIngestersWithinFlag = "querier.query-ingesters-within" // MinCompactorPartialBlockDeletionDelay is the minimum partial blocks deletion delay that can be configured in Mimir. MinCompactorPartialBlockDeletionDelay = 4 * time.Hour @@ -263,7 +262,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { _ = l.ResultsCacheTTLForOutOfOrderTimeWindow.Set("10m") f.Var(&l.ResultsCacheTTLForOutOfOrderTimeWindow, resultsCacheTTLForOutOfOrderWindowFlag, fmt.Sprintf("Time to live duration for cached query results if query falls into out-of-order time window. This is lower than -%s so that incoming out-of-order samples are returned in the query results sooner.", resultsCacheTTLFlag)) f.Var(&l.ResultsCacheTTLForCardinalityQuery, "query-frontend.results-cache-ttl-for-cardinality-query", "Time to live duration for cached cardinality query results. The value 0 disables the cache.") - f.BoolVar(&l.ResultsCacheForUnalignedQueryEnabled, resultsCacheForUnalignedQueryEnabledFlag, false, "Cache requests that are not step-aligned.") + f.BoolVar(&l.ResultsCacheForUnalignedQueryEnabled, "query-frontend.cache-unaligned-requests", false, "Cache requests that are not step-aligned.") f.IntVar(&l.MaxQueryExpressionSizeBytes, maxQueryExpressionSizeBytesFlag, 0, "Max size of the raw query, in bytes. 0 to not apply a limit to the size of the query.") // Store-gateway. From 11d21f6ecffbd9414aa71c4300f2d7f3b85a4bac Mon Sep 17 00:00:00 2001 From: francoposa Date: Thu, 29 Jun 2023 07:50:52 -0700 Subject: [PATCH 6/7] issues/5253 changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3942b8cc9c8..0b2dfc0aee6 100644 --- a/CHANGELOG.md +++ b/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"}` From 38941de313d46ec2219a4705ba2d948a656c58dc Mon Sep 17 00:00:00 2001 From: francoposa Date: Fri, 30 Jun 2023 15:27:08 -0500 Subject: [PATCH 7/7] issues/5253 ensure a non-default setting for the deprecated frontend config overrides a default setting for the new limit config until removal in v2.12 --- pkg/frontend/querymiddleware/roundtrip.go | 12 +++++++++++- pkg/mimir/modules.go | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/frontend/querymiddleware/roundtrip.go b/pkg/frontend/querymiddleware/roundtrip.go index 88757a690cf..c5e48fc0317 100644 --- a/pkg/frontend/querymiddleware/roundtrip.go +++ b/pkg/frontend/querymiddleware/roundtrip.go @@ -32,6 +32,10 @@ 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. @@ -42,7 +46,7 @@ type Config struct { 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"` // TODO: Deprecated in Mimir 2.10.0, remove in Mimir 2.12.0 + 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. @@ -62,6 +66,12 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { 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. diff --git a/pkg/mimir/modules.go b/pkg/mimir/modules.go index 90f00711818..57840522e87 100644 --- a/pkg/mimir/modules.go +++ b/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)