Skip to content

Commit

Permalink
Fix query_timeout warning messages (#10083)
Browse files Browse the repository at this point in the history
**What this PR does / why we need it**:
This PR fixes the warning messages that are logged in relation to the
timeouts migration.

The messages reference the per-tenant config, but the config in question
in the `AdjustForTimeoutsMigration` function is the global
`limits_config`. This is a source of confusion for users.


**Which issue(s) this PR fixes**:
Fixes #9801
  • Loading branch information
Mohamed-Amine Bouqsimi authored Aug 3, 2023
1 parent 2cfe8e2 commit 2fcde18
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
4 changes: 2 additions & 2 deletions docs/sources/configure/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -2446,8 +2446,8 @@ The `limits_config` block configures global and per-tenant limits in Loki.
[query_ready_index_num_days: <int> | default = 0]

# Timeout when querying backends (ingesters or storage) during the execution of
# a query request. If a specific per-tenant timeout is used, this timeout is
# ignored.
# a query request. When a specific per-tenant timeout is used, the global
# timeout is ignored.
# CLI flag: -querier.query-timeout
[query_timeout: <duration> | default = 1m]

Expand Down
16 changes: 8 additions & 8 deletions pkg/loki/loki.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,35 +280,35 @@ func (c *Config) Validate() error {
// - If only the querier:engine:timeout was explicitly configured, warn the user and use it everywhere.
func AdjustForTimeoutsMigration(c *Config) error {
engineTimeoutIsDefault := c.Querier.Engine.Timeout == logql.DefaultEngineTimeout
perTenantTimeoutIsDefault := c.LimitsConfig.QueryTimeout.String() == validation.DefaultPerTenantQueryTimeout
if engineTimeoutIsDefault && perTenantTimeoutIsDefault {
globalTimeoutIsDefault := c.LimitsConfig.QueryTimeout.String() == validation.DefaultPerTenantQueryTimeout
if engineTimeoutIsDefault && globalTimeoutIsDefault {
if err := c.LimitsConfig.QueryTimeout.Set(c.Querier.Engine.Timeout.String()); err != nil {
return fmt.Errorf("couldn't set per-tenant query_timeout as the engine timeout value: %w", err)
return fmt.Errorf("couldn't set global query_timeout as the engine timeout value: %w", err)
}
level.Warn(util_log.Logger).Log("msg",
fmt.Sprintf(
"per-tenant timeout not configured, using default engine timeout (%q). This behavior will change in the next major to always use the default per-tenant timeout (%q).",
"global timeout not configured, using default engine timeout (%q). This behavior will change in the next major to always use the default global timeout (%q).",
c.Querier.Engine.Timeout.String(),
c.LimitsConfig.QueryTimeout.String(),
),
)
return nil
}

if !perTenantTimeoutIsDefault && !engineTimeoutIsDefault {
if !globalTimeoutIsDefault && !engineTimeoutIsDefault {
level.Warn(util_log.Logger).Log("msg",
fmt.Sprintf(
"using configured per-tenant timeout (%q) as the default (can be overridden per-tenant in the limits_config). Configured engine timeout (%q) is deprecated and will be ignored.",
"using configured global timeout (%q) as the default (can be overridden per-tenant in the limits_config). Configured engine timeout (%q) is deprecated and will be ignored.",
c.LimitsConfig.QueryTimeout.String(),
c.Querier.Engine.Timeout.String(),
),
)
return nil
}

if perTenantTimeoutIsDefault && !engineTimeoutIsDefault {
if globalTimeoutIsDefault && !engineTimeoutIsDefault {
if err := c.LimitsConfig.QueryTimeout.Set(c.Querier.Engine.Timeout.String()); err != nil {
return fmt.Errorf("couldn't set per-tenant query_timeout as the engine timeout value: %w", err)
return fmt.Errorf("couldn't set global query_timeout as the engine timeout value: %w", err)
}
level.Warn(util_log.Logger).Log("msg",
fmt.Sprintf(
Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
_ = l.MaxQueryRange.Set("0s")
f.Var(&l.MaxQueryRange, "querier.max-query-range", "Limit the length of the [range] inside a range query. Default is 0 or unlimited")
_ = l.QueryTimeout.Set(DefaultPerTenantQueryTimeout)
f.Var(&l.QueryTimeout, "querier.query-timeout", "Timeout when querying backends (ingesters or storage) during the execution of a query request. If a specific per-tenant timeout is used, this timeout is ignored.")
f.Var(&l.QueryTimeout, "querier.query-timeout", "Timeout when querying backends (ingesters or storage) during the execution of a query request. When a specific per-tenant timeout is used, the global timeout is ignored.")

_ = l.MaxQueryLookback.Set("0s")
f.Var(&l.MaxQueryLookback, "querier.max-query-lookback", "Limit how far back in time series data and metadata can be queried, up until lookback duration ago. This limit is enforced in the query frontend, the querier and the ruler. If the requested time range is outside the allowed range, the request will not fail, but will be modified to only query data within the allowed time range. The default value of 0 does not set a limit.")
Expand Down

0 comments on commit 2fcde18

Please sign in to comment.