From d344a6c8b28f01cffee971b4f8a9f92d625819fe Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 23 May 2023 15:58:55 +0200 Subject: [PATCH 1/4] Querier: Refine error messages for per-tenant query limits Refine error messages for per-tenant query limits, informing the user of the preferred strategy for not hitting the limit, in addition to how they may tweak the limit. Affected limits: * max_fetched_chunks_per_query * max_fetched_chunk_bytes_per_query * max_fetched_series_per_query Signed-off-by: Arve Knudsen --- CHANGELOG.md | 1 + pkg/util/globalerror/errors.go | 9 +++++++++ pkg/util/limiter/query_limiter.go | 11 ++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cb2b8ebb9d..4b1806a34be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * [ENHANCEMENT] Cardinality API: When zone aware replication is enabled, the label values cardinality API can now tolerate single zone failure #5178 * [ENHANCEMENT] Distributor: optimize sending requests to ingesters when incoming requests don't need to be modified. For now this feature can be disabled by setting `-timeseries-unmarshal-caching-optimization-enabled=false`. #5137 * [ENHANCEMENT] Query-frontend: added "response_size_bytes" field to "query stats" log. #5196 +* [ENHANCEMENT] Querier: Refine error messages for per-tenant query limits, informing the user of the preferred strategy for not hitting the limit, in addition to how they may tweak the limit. #5059 ### Mixin * [CHANGE] Dashboards: show all workloads in selected namespace on "rollout progress" dashboard. #5113 diff --git a/pkg/util/globalerror/errors.go b/pkg/util/globalerror/errors.go index 744e6c90eb5..103711c2ad0 100644 --- a/pkg/util/globalerror/errors.go +++ b/pkg/util/globalerror/errors.go @@ -87,6 +87,15 @@ func (id ID) MessageWithPerTenantLimitConfig(msg, flag string, addFlags ...strin return fmt.Sprintf("%s (%s%s). To adjust the related per-tenant limit%s, configure %s, or contact your service administrator.", msg, errPrefix, id, plural, flagsList) } +// MessageWithStrategyAndPerTenantLimitConfig returns the provided msg, appending the error id and a +// suggestion on which strategy to follow for hopefully not hitting the limit, plus which configuration +// flag(s) to otherwise change the per-tenant limit. +func (id ID) MessageWithStrategyAndPerTenantLimitConfig(msg, strategy, flag string, addFlags ...string) string { + flagsList, plural := buildFlagsList(flag, addFlags...) + return fmt.Sprintf("%s (%s%s). %s. Otherwise, to adjust the related per-tenant limit%s, configure %s, or contact your service administrator.", + msg, errPrefix, id, strategy, plural, flagsList) +} + func buildFlagsList(flag string, addFlags ...string) (string, string) { var sb strings.Builder sb.WriteString("-") diff --git a/pkg/util/limiter/query_limiter.go b/pkg/util/limiter/query_limiter.go index da0e72adafd..ce16a04689f 100644 --- a/pkg/util/limiter/query_limiter.go +++ b/pkg/util/limiter/query_limiter.go @@ -19,18 +19,23 @@ import ( type queryLimiterCtxKey struct{} +const cardinalityStrategy = "Consider reducing the time range and/or cardinality of the query. To reduce the query cardinality, you can add more label matchers to the query, or increase the number of query shards" + var ( ctxKey = &queryLimiterCtxKey{} - MaxSeriesHitMsgFormat = globalerror.MaxSeriesPerQuery.MessageWithPerTenantLimitConfig( + MaxSeriesHitMsgFormat = globalerror.MaxSeriesPerQuery.MessageWithStrategyAndPerTenantLimitConfig( "the query exceeded the maximum number of series (limit: %d series)", + cardinalityStrategy, validation.MaxSeriesPerQueryFlag, ) - MaxChunkBytesHitMsgFormat = globalerror.MaxChunkBytesPerQuery.MessageWithPerTenantLimitConfig( + MaxChunkBytesHitMsgFormat = globalerror.MaxChunkBytesPerQuery.MessageWithStrategyAndPerTenantLimitConfig( "the query exceeded the aggregated chunks size limit (limit: %d bytes)", + cardinalityStrategy, validation.MaxChunkBytesPerQueryFlag, ) - MaxChunksPerQueryLimitMsgFormat = globalerror.MaxChunksPerQuery.MessageWithPerTenantLimitConfig( + MaxChunksPerQueryLimitMsgFormat = globalerror.MaxChunksPerQuery.MessageWithStrategyAndPerTenantLimitConfig( "the query exceeded the maximum number of chunks (limit: %d chunks)", + cardinalityStrategy, validation.MaxChunksPerQueryFlag, ) ) From 6d4264d5ad7e23ed71f534c8dc09cdf86c0eed2d Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Mon, 5 Jun 2023 16:08:51 +0200 Subject: [PATCH 2/4] Update pkg/util/limiter/query_limiter.go Co-authored-by: Charles Korn --- pkg/util/limiter/query_limiter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/limiter/query_limiter.go b/pkg/util/limiter/query_limiter.go index ce16a04689f..a24197eec8d 100644 --- a/pkg/util/limiter/query_limiter.go +++ b/pkg/util/limiter/query_limiter.go @@ -19,7 +19,7 @@ import ( type queryLimiterCtxKey struct{} -const cardinalityStrategy = "Consider reducing the time range and/or cardinality of the query. To reduce the query cardinality, you can add more label matchers to the query, or increase the number of query shards" +const cardinalityStrategy = "Consider reducing the time range and/or number of series selected by the query, and/or increase the number of query shards. To reduce the number of series selected, add more label matchers to the query. To increase the number of query shards, increase the value of -" var ( ctxKey = &queryLimiterCtxKey{} From 474db424411c267f7a4f656b0f92527f32c74019 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Mon, 5 Jun 2023 16:12:18 +0200 Subject: [PATCH 3/4] Don't mention increasing query shards Signed-off-by: Arve Knudsen --- pkg/util/globalerror/errors.go | 2 +- pkg/util/limiter/query_limiter.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/globalerror/errors.go b/pkg/util/globalerror/errors.go index 103711c2ad0..01fcd299df9 100644 --- a/pkg/util/globalerror/errors.go +++ b/pkg/util/globalerror/errors.go @@ -88,7 +88,7 @@ func (id ID) MessageWithPerTenantLimitConfig(msg, flag string, addFlags ...strin } // MessageWithStrategyAndPerTenantLimitConfig returns the provided msg, appending the error id and a -// suggestion on which strategy to follow for hopefully not hitting the limit, plus which configuration +// suggestion on which strategy to follow to try not hitting the limit, plus which configuration // flag(s) to otherwise change the per-tenant limit. func (id ID) MessageWithStrategyAndPerTenantLimitConfig(msg, strategy, flag string, addFlags ...string) string { flagsList, plural := buildFlagsList(flag, addFlags...) diff --git a/pkg/util/limiter/query_limiter.go b/pkg/util/limiter/query_limiter.go index a24197eec8d..95d00f9c019 100644 --- a/pkg/util/limiter/query_limiter.go +++ b/pkg/util/limiter/query_limiter.go @@ -19,7 +19,7 @@ import ( type queryLimiterCtxKey struct{} -const cardinalityStrategy = "Consider reducing the time range and/or number of series selected by the query, and/or increase the number of query shards. To reduce the number of series selected, add more label matchers to the query. To increase the number of query shards, increase the value of -" +const cardinalityStrategy = "Consider reducing the time range and/or number of series selected by the query. One way to reduce the number of selected series is to add more label matchers to the query" var ( ctxKey = &queryLimiterCtxKey{} From d46ba1169a8e3f8841633f82a210db30eb283379 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 8 Jun 2023 09:14:57 +0200 Subject: [PATCH 4/4] Fix tests Signed-off-by: Arve Knudsen --- pkg/ingester/client/streaming_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ingester/client/streaming_test.go b/pkg/ingester/client/streaming_test.go index 6ece336d16a..6bfb84133a9 100644 --- a/pkg/ingester/client/streaming_test.go +++ b/pkg/ingester/client/streaming_test.go @@ -222,12 +222,12 @@ func TestSeriesChunksStreamReader_ChunksLimits(t *testing.T) { "query selects too many chunks": { maxChunks: 2, maxChunkBytes: 200, - expectedError: "attempted to read series at index 0 from stream, but the stream has failed: the query exceeded the maximum number of chunks (limit: 2 chunks) (err-mimir-max-chunks-per-query). To adjust the related per-tenant limit, configure -querier.max-fetched-chunks-per-query, or contact your service administrator.", + expectedError: "attempted to read series at index 0 from stream, but the stream has failed: the query exceeded the maximum number of chunks (limit: 2 chunks) (err-mimir-max-chunks-per-query). Consider reducing the time range and/or number of series selected by the query. One way to reduce the number of selected series is to add more label matchers to the query. Otherwise, to adjust the related per-tenant limit, configure -querier.max-fetched-chunks-per-query, or contact your service administrator.", }, "query selects too many chunk bytes": { maxChunks: 4, maxChunkBytes: 100, - expectedError: "attempted to read series at index 0 from stream, but the stream has failed: the query exceeded the aggregated chunks size limit (limit: 100 bytes) (err-mimir-max-chunks-bytes-per-query). To adjust the related per-tenant limit, configure -querier.max-fetched-chunk-bytes-per-query, or contact your service administrator.", + expectedError: "attempted to read series at index 0 from stream, but the stream has failed: the query exceeded the aggregated chunks size limit (limit: 100 bytes) (err-mimir-max-chunks-bytes-per-query). Consider reducing the time range and/or number of series selected by the query. One way to reduce the number of selected series is to add more label matchers to the query. Otherwise, to adjust the related per-tenant limit, configure -querier.max-fetched-chunk-bytes-per-query, or contact your service administrator.", }, }