From 33a86d2f77bd53c62618a8a734d9307780747a33 Mon Sep 17 00:00:00 2001 From: Charlie Le Date: Thu, 5 Nov 2026 14:20:29 -0800 Subject: [PATCH 1/4] Keep the 1.21 query range and HA failover timeout settings working #7160/#7323 moved query_ingesters_within, query_store_after and shuffle_sharding_ingesters_lookback_period from querier to limits, and renamed their flags from -querier.* to -limits.*. #7481 moved the ha_tracker_failover_timeout YAML field from distributor.ha_tracker to limits. Neither left the old names in place, so a 1.21 config that uses them fails at startup on 1.22. Restore the old YAML fields and flags as hidden, deprecated settings that are scheduled for removal in v1.24.0. Config.Validate copies a set old value into its limit before the limits are validated and before they become the runtime config defaults, so tenant overrides inherit it. When the old and the new name are both set to different values, startup fails rather than picking one. cortex.New logs a warning and increments deprecated_flags_inuse_total for each one used: Validate runs before the logger is initialised, so a warning logged there is lost. Also correct the -limits.query-ingesters-within help text, which said "less than or equal to" close-idle-tsdb-timeout while #7323 rejects equal, and spell out that new startup check in the CHANGELOG. Signed-off-by: Charlie Le --- CHANGELOG.md | 5 +- docs/configuration/config-file-reference.md | 4 +- pkg/cortex/cortex.go | 55 +++++++++++++++ pkg/cortex/cortex_test.go | 76 +++++++++++++++++++++ pkg/ha/ha_tracker.go | 5 ++ pkg/querier/querier.go | 10 +++ pkg/util/validation/limits.go | 2 +- schemas/cortex-config-schema.json | 2 +- 8 files changed, 153 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bfc9ac8532..6e635c80f55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,10 +34,11 @@ - `-ingester.max-series-per-query` (a chunks-storage limit, ignored since blocks storage; use `-querier.max-fetched-series-per-query`) * [CHANGE] Ingester: Formally deprecate `-blocks-storage.tsdb.max-exemplars`, scheduled for removal in v1.24.0. Use the per-tenant `max_exemplars` limit instead. The flag still works as the global fallback when `max_exemplars` is 0, but setting it now logs a warning and increments `deprecated_flags_inuse_total`. #7793 * [CHANGE] Ingester: Graduate native histogram ingestion (`-blocks-storage.tsdb.enable-native-histograms`) from experimental. #7789 -* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7160 #7323 +* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting an old and a new name to different values fails at startup. #7160 #7323 #PRNUM +* [CHANGE] Querier: `query_ingesters_within` must now be strictly less than `-blocks-storage.tsdb.close-idle-tsdb-timeout` when both are positive. Earlier releases did not check this, and documented the two being equal as valid, so a config that sets them equal now makes Cortex **exit at startup** with `invalid query routing config: tenant default: query_ingesters_within (6h50m0s) must be less than close_idle_tsdb_timeout (6h50m0s)`. The check also runs on every per-tenant override in the runtime config (on the `all`, `distributor`, `querier` and `ruler` targets), so a single tenant override that sets `query_ingesters_within` at or above `close_idle_tsdb_timeout` makes the whole runtime config fail to load: Cortex exits at startup, and on an already-running process every reload fails, pinning the last good overrides and dropping `cortex_runtime_config_last_reload_successful` to 0. Before upgrading, raise `close_idle_tsdb_timeout` above the largest `query_ingesters_within` in your config and runtime overrides, or set it to 0 to disable closing idle TSDBs. The existing checks that `query_store_after` is less than `query_ingesters_within`, and that `shuffle_sharding_ingesters_lookback_period` is at least `query_store_after`, now also apply to per-tenant overrides. #7323 * [CHANGE] Alertmanager: Remove the obsolete startup migration of local state files into per-tenant directories (scheduled for removal in 1.11.0). Upgrading from a release older than 1.9.0 with a persisted local state directory now requires upgrading to an intermediate release first, so the migration can run. #7513 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446 -* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. #7481 +* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. The YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. The old field still works as the default for the new limit, but is deprecated and will be removed in v1.24.0: using it logs a warning and increments `deprecated_flags_inuse_total`. Setting the old field and a non-default limit to different values fails at startup. #7481 #PRNUM * [FEATURE] Parquet: Support sharded parquet file conversion and querying. #7610 * [FEATURE] Parquet Converter: Add experimental `-parquet-converter.max-num-columns` flag to automatically shard parquet files when the number of columns exceeds the configured limit. This prevents failures when a TSDB block has more unique label names than the parquet library's column limit (32767). #7624 * [FEATURE] Distributor: Add experimental `-distributor.num-query-workers` flag to use a goroutine worker pool for query fan-out calls to ingesters. Reuses pre-grown goroutine stacks to eliminate the `runtime.copystack` overhead (~8% CPU) observed on rulers with wide ingester fan-out. Falls back to spawning a new goroutine when no worker is available. #7623 diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index 805fe51dee6..f0e3045344f 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -4805,8 +4805,8 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s # Maximum lookback duration for querying data from ingesters. Queries for data # older than this will only query the long-term storage. This is a per-tenant -# limit that can be overridden in the runtime configuration. Should be less than -# or equal to close-idle-tsdb-timeout. +# limit that can be overridden in the runtime configuration. Must be less than +# -blocks-storage.tsdb.close-idle-tsdb-timeout when both are positive. # CLI flag: -limits.query-ingesters-within [query_ingesters_within: | default = 0s] diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 744a67eabb2..7daceb15476 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -10,6 +10,7 @@ import ( "reflect" "slices" "strings" + "time" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -242,6 +243,10 @@ func (c *Config) Validate(log log.Logger) error { return err } + if err := c.applyDeprecatedLimits(); err != nil { + return err + } + switch c.NameValidationScheme { case model.LegacyValidation, model.UTF8Validation: default: @@ -329,6 +334,54 @@ func (c *Config) isModuleEnabled(m string) bool { return slices.Contains(c.Target, m) } +type deprecatedLimit struct { + oldName, newName string + old time.Duration + limit *model.Duration + def model.Duration +} + +// deprecatedLimits lists the settings that moved into the per-tenant limits in 1.22, with +// their old location. The old one is kept so that 1.21 configs keep working. +func (c *Config) deprecatedLimits() []deprecatedLimit { + defaults := validation.Limits{} + flagext.DefaultValues(&defaults) + + return []deprecatedLimit{ + {"querier.query_ingesters_within", "limits.query_ingesters_within", c.Querier.DeprecatedQueryIngestersWithin, &c.LimitsConfig.QueryIngestersWithin, defaults.QueryIngestersWithin}, + {"querier.query_store_after", "limits.query_store_after", c.Querier.DeprecatedQueryStoreAfter, &c.LimitsConfig.QueryStoreAfter, defaults.QueryStoreAfter}, + {"querier.shuffle_sharding_ingesters_lookback_period", "limits.shuffle_sharding_ingesters_lookback_period", c.Querier.DeprecatedShuffleShardingIngestersLookbackPeriod, &c.LimitsConfig.ShuffleShardingIngestersLookbackPeriod, defaults.ShuffleShardingIngestersLookbackPeriod}, + {"distributor.ha_tracker.ha_tracker_failover_timeout", "limits.ha_tracker_failover_timeout", c.Distributor.HATrackerConfig.DeprecatedFailoverTimeout, &c.LimitsConfig.HATrackerFailoverTimeout, defaults.HATrackerFailoverTimeout}, + } +} + +// applyDeprecatedLimits copies each deprecated setting that is set into its limit, unless the +// limit is set to a different value. It must run before the limits are validated and before +// they become the defaults for the runtime config overrides. +func (c *Config) applyDeprecatedLimits() error { + for _, d := range c.deprecatedLimits() { + switch { + case d.old == 0: + case *d.limit == d.def: + *d.limit = model.Duration(d.old) + case time.Duration(*d.limit) != d.old: + return fmt.Errorf("%s (%s) and the deprecated %s (%s) are both set: remove %s", d.newName, time.Duration(*d.limit), d.oldName, d.old, d.oldName) + } + } + return nil +} + +// warnDeprecatedLimits logs each deprecated setting that is set. It is separate from +// applyDeprecatedLimits because Validate runs before the logger is initialised. +func (c *Config) warnDeprecatedLimits(logger log.Logger) { + for _, d := range c.deprecatedLimits() { + if d.old != 0 { + flagext.DeprecatedFlagsUsed.Inc() + level.Warn(logger).Log("msg", "running with DEPRECATED config "+d.oldName+", use "+d.newName+" instead, it will be removed in v1.24.0") + } + } +} + // validateYAMLEmptyNodes ensure that no empty node has been specified in the YAML config file. // When an empty node is defined in YAML, the YAML parser sets the whole struct to its zero value // and so we loose all default values. It's very difficult to detect this case for the user, so we @@ -442,6 +495,8 @@ func New(cfg Config) (*Cortex, error) { os.Exit(0) } + cfg.warnDeprecatedLimits(util_log.Logger) + // Swap out the default resolver to support multiple tenant IDs separated by a '|' if cfg.TenantFederation.Enabled { users.WithDefaultResolver(users.NewMultiResolver()) diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 6c44feb303d..97f374dc1b1 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -21,6 +21,7 @@ import ( "go.uber.org/atomic" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "gopkg.in/yaml.v2" "github.com/cortexproject/cortex/pkg/alertmanager" "github.com/cortexproject/cortex/pkg/alertmanager/alertstore" @@ -304,6 +305,81 @@ func TestConfigValidation(t *testing.T) { } } +func TestConfigValidation_DeprecatedLimits(t *testing.T) { + // The CLI flags and YAML fields that moved into the limits in 1.22 must keep working. + for _, tc := range []struct { + name string + args []string + yaml string + expectedError string + check func(t *testing.T, cfg *Config) + }{ + { + name: "old querier YAML fields set the default limits", + yaml: "querier:\n query_ingesters_within: 13h\n query_store_after: 12h\n shuffle_sharding_ingesters_lookback_period: 13h\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(13*time.Hour), cfg.LimitsConfig.QueryIngestersWithin) + assert.Equal(t, model.Duration(12*time.Hour), cfg.LimitsConfig.QueryStoreAfter) + assert.Equal(t, model.Duration(13*time.Hour), cfg.LimitsConfig.ShuffleShardingIngestersLookbackPeriod) + }, + }, + { + name: "old querier CLI flags set the default limits", + args: []string{"-querier.query-ingesters-within=13h", "-querier.query-store-after=12h", "-querier.shuffle-sharding-ingesters-lookback-period=13h"}, + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(13*time.Hour), cfg.LimitsConfig.QueryIngestersWithin) + assert.Equal(t, model.Duration(12*time.Hour), cfg.LimitsConfig.QueryStoreAfter) + assert.Equal(t, model.Duration(13*time.Hour), cfg.LimitsConfig.ShuffleShardingIngestersLookbackPeriod) + }, + }, + { + name: "old HA tracker YAML field sets the default limit", + yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(45*time.Second), cfg.LimitsConfig.HATrackerFailoverTimeout) + }, + }, + { + name: "old and new set to the same value", + yaml: "querier:\n query_store_after: 12h\nlimits:\n query_store_after: 12h\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(12*time.Hour), cfg.LimitsConfig.QueryStoreAfter) + }, + }, + { + name: "old and new set to different values", + yaml: "querier:\n query_store_after: 12h\nlimits:\n query_store_after: 6h\n", + expectedError: "limits.query_store_after (6h0m0s) and the deprecated querier.query_store_after (12h0m0s) are both set: remove querier.query_store_after", + }, + { + name: "old HA tracker field and a non-default limit set to different values", + yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s\nlimits:\n ha_tracker_failover_timeout: 1m\n", + expectedError: "limits.ha_tracker_failover_timeout (1m0s) and the deprecated distributor.ha_tracker.ha_tracker_failover_timeout (45s) are both set: remove distributor.ha_tracker.ha_tracker_failover_timeout", + }, + { + name: "old fields are validated like the new ones", + yaml: "querier:\n query_ingesters_within: 12h\n query_store_after: 12h\n", + expectedError: "tenant default: query_store_after (12h0m0s) must be less than query_ingesters_within (12h0m0s)", + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := &Config{} + fs := flag.NewFlagSet("", flag.PanicOnError) + cfg.RegisterFlags(fs) + require.NoError(t, fs.Parse(tc.args)) + require.NoError(t, yaml.UnmarshalStrict([]byte(tc.yaml), cfg)) + + err := cfg.Validate(nil) + if tc.expectedError != "" { + require.ErrorContains(t, err, tc.expectedError) + return + } + require.NoError(t, err) + tc.check(t, cfg) + }) + } +} + func TestGrpcAuthMiddleware(t *testing.T) { prepareGlobalMetricsRegistry(t) diff --git a/pkg/ha/ha_tracker.go b/pkg/ha/ha_tracker.go index 6c9722ea8a8..662d752210f 100644 --- a/pkg/ha/ha_tracker.go +++ b/pkg/ha/ha_tracker.go @@ -63,6 +63,11 @@ type HATrackerConfig struct { // is more than this duration. UpdateTimeout time.Duration `yaml:"ha_tracker_update_timeout"` UpdateTimeoutJitterMax time.Duration `yaml:"ha_tracker_update_timeout_jitter_max"` + // Deprecated: moved to the ha_tracker_failover_timeout limit, and scheduled for removal + // in v1.24.0. Kept so that 1.21 configs still start. YAML only: the + // -distributor.ha-tracker.failover-timeout flag now sets the limit. A non-zero value is + // applied as the default limit by cortex.Config.Validate. + DeprecatedFailoverTimeout time.Duration `yaml:"ha_tracker_failover_timeout" doc:"hidden"` // EnableStartupSync controls whether to fetch all tracked keys from the KV store // on startup to populate the local cache. // This prevents duplicate GET calls for the same key while the cache is cold, diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index dc9edd6dc56..fb2356c6794 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -71,6 +71,13 @@ type Config struct { // series is considered stale. LookbackDelta time.Duration `yaml:"lookback_delta"` + // Deprecated: moved to the per-tenant limits of the same name, and scheduled for removal + // in v1.24.0. Kept so that 1.21 configs still start. A non-zero value is applied as the + // default limit by cortex.Config.Validate. + DeprecatedQueryIngestersWithin time.Duration `yaml:"query_ingesters_within" doc:"hidden"` + DeprecatedQueryStoreAfter time.Duration `yaml:"query_store_after" doc:"hidden"` + DeprecatedShuffleShardingIngestersLookbackPeriod time.Duration `yaml:"shuffle_sharding_ingesters_lookback_period" doc:"hidden"` + // Blocks storage only. StoreGatewayAddresses string `yaml:"store_gateway_addresses"` StoreGatewayClient ClientConfig `yaml:"store_gateway_client"` @@ -148,6 +155,9 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.Int64Var(&cfg.StoreGatewaySeriesBatchSize, "querier.store-gateway-series-batch-size", 1, "[Experimental] The maximum number of series to be batched in a single gRPC response message from Store Gateways. A value of 0 or 1 disables batching.") f.IntVar(&cfg.IngesterQueryMaxAttempts, "querier.ingester-query-max-attempts", 1, "The maximum number of times we attempt fetching data from ingesters for retryable errors (ex. partial data returned).") f.DurationVar(&cfg.LookbackDelta, "querier.lookback-delta", 5*time.Minute, "Time since the last sample after which a time series is considered stale and ignored by expression evaluations.") + f.DurationVar(&cfg.DeprecatedQueryIngestersWithin, "querier.query-ingesters-within", 0, "Deprecated (use -limits.query-ingesters-within instead) and will be removed in v1.24.0.") + f.DurationVar(&cfg.DeprecatedQueryStoreAfter, "querier.query-store-after", 0, "Deprecated (use -limits.query-store-after instead) and will be removed in v1.24.0.") + f.DurationVar(&cfg.DeprecatedShuffleShardingIngestersLookbackPeriod, "querier.shuffle-sharding-ingesters-lookback-period", 0, "Deprecated (use -limits.shuffle-sharding-ingesters-lookback-period instead) and will be removed in v1.24.0.") f.Int64Var(&cfg.MaxSubQuerySteps, "querier.max-subquery-steps", 0, "Max number of steps allowed for every subquery expression in query. Number of steps is calculated using subquery range / step. A value > 0 enables it.") f.BoolVar(&cfg.IgnoreMaxQueryLength, "querier.ignore-max-query-length", false, "If enabled, ignore max query length check at Querier select method. Users can choose to ignore it since the validation can be done before Querier evaluation like at Query Frontend or Ruler.") f.BoolVar(&cfg.EnablePromQLExperimentalFunctions, "querier.enable-promql-experimental-functions", false, "[Experimental] If true, experimental promQL functions are enabled.") diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index afe223b533c..cf66b88cd86 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -327,7 +327,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) { f.IntVar(&l.MaxFetchedDataBytesPerQuery, "querier.max-fetched-data-bytes-per-query", 0, "The maximum combined size of all data that a query can fetch from each ingester and storage. This limit is enforced in the querier and ruler for `query`, `query_range` and `series` APIs. 0 to disable.") _ = l.QueryIngestersWithin.Set("0") - f.Var(&l.QueryIngestersWithin, "limits.query-ingesters-within", "Maximum lookback duration for querying data from ingesters. Queries for data older than this will only query the long-term storage. This is a per-tenant limit that can be overridden in the runtime configuration. Should be less than or equal to close-idle-tsdb-timeout.") + f.Var(&l.QueryIngestersWithin, "limits.query-ingesters-within", "Maximum lookback duration for querying data from ingesters. Queries for data older than this will only query the long-term storage. This is a per-tenant limit that can be overridden in the runtime configuration. Must be less than -blocks-storage.tsdb.close-idle-tsdb-timeout when both are positive.") _ = l.QueryStoreAfter.Set("0") f.Var(&l.QueryStoreAfter, "limits.query-store-after", "Minimum age of data before querying the long-term storage. Queries for data younger than this will only query ingesters. This is a per-tenant limit that can be overridden in the runtime configuration.") diff --git a/schemas/cortex-config-schema.json b/schemas/cortex-config-schema.json index 363f6362d0b..1b60cb48336 100644 --- a/schemas/cortex-config-schema.json +++ b/schemas/cortex-config-schema.json @@ -6092,7 +6092,7 @@ }, "query_ingesters_within": { "default": "0s", - "description": "Maximum lookback duration for querying data from ingesters. Queries for data older than this will only query the long-term storage. This is a per-tenant limit that can be overridden in the runtime configuration. Should be less than or equal to close-idle-tsdb-timeout.", + "description": "Maximum lookback duration for querying data from ingesters. Queries for data older than this will only query the long-term storage. This is a per-tenant limit that can be overridden in the runtime configuration. Must be less than -blocks-storage.tsdb.close-idle-tsdb-timeout when both are positive.", "type": "string", "x-cli-flag": "limits.query-ingesters-within", "x-format": "duration" From 02c9e8e2f8b303f2b15614e80e22b310912fff6b Mon Sep 17 00:00:00 2001 From: Charlie Le Date: Thu, 5 Nov 2026 14:21:00 -0800 Subject: [PATCH 2/4] CHANGELOG: reference #7919 Signed-off-by: Charlie Le --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e635c80f55..e7f581576bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,11 +34,11 @@ - `-ingester.max-series-per-query` (a chunks-storage limit, ignored since blocks storage; use `-querier.max-fetched-series-per-query`) * [CHANGE] Ingester: Formally deprecate `-blocks-storage.tsdb.max-exemplars`, scheduled for removal in v1.24.0. Use the per-tenant `max_exemplars` limit instead. The flag still works as the global fallback when `max_exemplars` is 0, but setting it now logs a warning and increments `deprecated_flags_inuse_total`. #7793 * [CHANGE] Ingester: Graduate native histogram ingestion (`-blocks-storage.tsdb.enable-native-histograms`) from experimental. #7789 -* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting an old and a new name to different values fails at startup. #7160 #7323 #PRNUM +* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting an old and a new name to different values fails at startup. #7160 #7323 #7919 * [CHANGE] Querier: `query_ingesters_within` must now be strictly less than `-blocks-storage.tsdb.close-idle-tsdb-timeout` when both are positive. Earlier releases did not check this, and documented the two being equal as valid, so a config that sets them equal now makes Cortex **exit at startup** with `invalid query routing config: tenant default: query_ingesters_within (6h50m0s) must be less than close_idle_tsdb_timeout (6h50m0s)`. The check also runs on every per-tenant override in the runtime config (on the `all`, `distributor`, `querier` and `ruler` targets), so a single tenant override that sets `query_ingesters_within` at or above `close_idle_tsdb_timeout` makes the whole runtime config fail to load: Cortex exits at startup, and on an already-running process every reload fails, pinning the last good overrides and dropping `cortex_runtime_config_last_reload_successful` to 0. Before upgrading, raise `close_idle_tsdb_timeout` above the largest `query_ingesters_within` in your config and runtime overrides, or set it to 0 to disable closing idle TSDBs. The existing checks that `query_store_after` is less than `query_ingesters_within`, and that `shuffle_sharding_ingesters_lookback_period` is at least `query_store_after`, now also apply to per-tenant overrides. #7323 * [CHANGE] Alertmanager: Remove the obsolete startup migration of local state files into per-tenant directories (scheduled for removal in 1.11.0). Upgrading from a release older than 1.9.0 with a persisted local state directory now requires upgrading to an intermediate release first, so the migration can run. #7513 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446 -* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. The YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. The old field still works as the default for the new limit, but is deprecated and will be removed in v1.24.0: using it logs a warning and increments `deprecated_flags_inuse_total`. Setting the old field and a non-default limit to different values fails at startup. #7481 #PRNUM +* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. The YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. The old field still works as the default for the new limit, but is deprecated and will be removed in v1.24.0: using it logs a warning and increments `deprecated_flags_inuse_total`. Setting the old field and a non-default limit to different values fails at startup. #7481 #7919 * [FEATURE] Parquet: Support sharded parquet file conversion and querying. #7610 * [FEATURE] Parquet Converter: Add experimental `-parquet-converter.max-num-columns` flag to automatically shard parquet files when the number of columns exceeds the configured limit. This prevents failures when a TSDB block has more unique label names than the parquet library's column limit (32767). #7624 * [FEATURE] Distributor: Add experimental `-distributor.num-query-workers` flag to use a goroutine worker pool for query fan-out calls to ingesters. Reuses pre-grown goroutine stacks to eliminate the `runtime.copystack` overhead (~8% CPU) observed on rulers with wide ingester fan-out. Falls back to spawning a new goroutine when no worker is available. #7623 From 6c8cf78e08f5d13b37d52524fd604a1e66a85ce1 Mon Sep 17 00:00:00 2001 From: Charlie Le Date: Thu, 5 Nov 2026 15:09:13 -0800 Subject: [PATCH 3/4] Drop the Deprecated: marker from the restored config fields staticcheck (SA1019) flags every read of a field whose doc comment starts with Deprecated:, and cortex.Config reads these to apply them. Other config fields that Cortex still reads, such as MaxExemplars, do not use the marker either. Signed-off-by: Charlie Le --- pkg/ha/ha_tracker.go | 2 +- pkg/querier/querier.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ha/ha_tracker.go b/pkg/ha/ha_tracker.go index 662d752210f..60878b2ba80 100644 --- a/pkg/ha/ha_tracker.go +++ b/pkg/ha/ha_tracker.go @@ -63,7 +63,7 @@ type HATrackerConfig struct { // is more than this duration. UpdateTimeout time.Duration `yaml:"ha_tracker_update_timeout"` UpdateTimeoutJitterMax time.Duration `yaml:"ha_tracker_update_timeout_jitter_max"` - // Deprecated: moved to the ha_tracker_failover_timeout limit, and scheduled for removal + // This moved to the ha_tracker_failover_timeout limit, and is scheduled for removal // in v1.24.0. Kept so that 1.21 configs still start. YAML only: the // -distributor.ha-tracker.failover-timeout flag now sets the limit. A non-zero value is // applied as the default limit by cortex.Config.Validate. diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index fb2356c6794..6b2ec5c1ed5 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -71,7 +71,7 @@ type Config struct { // series is considered stale. LookbackDelta time.Duration `yaml:"lookback_delta"` - // Deprecated: moved to the per-tenant limits of the same name, and scheduled for removal + // These moved to the per-tenant limits of the same name, and are scheduled for removal // in v1.24.0. Kept so that 1.21 configs still start. A non-zero value is applied as the // default limit by cortex.Config.Validate. DeprecatedQueryIngestersWithin time.Duration `yaml:"query_ingesters_within" doc:"hidden"` From 72455145b252ccb0c4cdf85a84f7d945ca0433cc Mon Sep 17 00:00:00 2001 From: Charlie Le Date: Fri, 6 Nov 2026 15:15:52 -0800 Subject: [PATCH 4/4] Give command-line flags precedence over the deprecated YAML fields The fallback applied a deprecated value whenever the new limit was at its default, so it could not tell a flag passed explicitly at its default value from one left unset. With distributor: ha_tracker: ha_tracker_failover_timeout: 45s and -distributor.ha-tracker.failover-timeout=30s, 1.21 uses 30s, because a command-line flag overrides the YAML file, while this PR used 45s. The same happened to an explicit -limits.query-store-after=0 next to an old querier.query_store_after. Record where each setting was set instead of comparing values: the YAML keys from LoadConfig (SetYAMLKeys, next to SetYAMLRootNodeStates) and the flags passed on the command line (SetCommandLineFlags, after parsing). A flag on the command line now wins over the YAML file whichever name each uses, as in 1.21, and the old and new name must agree only when both are set in the same place. A config built in code has no recorded sources and keeps the value-based rule. The deprecated fields are also omitempty, so -print.config and /config show them only when they are set. Each case was checked against a v1.21.0 build with -print.config. Signed-off-by: Charlie Le --- CHANGELOG.md | 4 +- cmd/cortex/main.go | 7 ++++ cmd/cortex/main_test.go | 6 +++ pkg/cortex/cortex.go | 84 +++++++++++++++++++++++++++++++++------ pkg/cortex/cortex_test.go | 59 ++++++++++++++++++++++++++- pkg/ha/ha_tracker.go | 2 +- pkg/querier/querier.go | 6 +-- 7 files changed, 147 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f581576bd..43c6a83ad09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,11 +34,11 @@ - `-ingester.max-series-per-query` (a chunks-storage limit, ignored since blocks storage; use `-querier.max-fetched-series-per-query`) * [CHANGE] Ingester: Formally deprecate `-blocks-storage.tsdb.max-exemplars`, scheduled for removal in v1.24.0. Use the per-tenant `max_exemplars` limit instead. The flag still works as the global fallback when `max_exemplars` is 0, but setting it now logs a warning and increments `deprecated_flags_inuse_total`. #7793 * [CHANGE] Ingester: Graduate native histogram ingestion (`-blocks-storage.tsdb.enable-native-histograms`) from experimental. #7789 -* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting an old and a new name to different values fails at startup. #7160 #7323 #7919 +* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. As before, a flag on the command line wins over the YAML file, whether it uses the old or the new name. Setting the old and the new name to different values in the same place, both in the YAML file or both on the command line, fails at startup. #7160 #7323 #7919 * [CHANGE] Querier: `query_ingesters_within` must now be strictly less than `-blocks-storage.tsdb.close-idle-tsdb-timeout` when both are positive. Earlier releases did not check this, and documented the two being equal as valid, so a config that sets them equal now makes Cortex **exit at startup** with `invalid query routing config: tenant default: query_ingesters_within (6h50m0s) must be less than close_idle_tsdb_timeout (6h50m0s)`. The check also runs on every per-tenant override in the runtime config (on the `all`, `distributor`, `querier` and `ruler` targets), so a single tenant override that sets `query_ingesters_within` at or above `close_idle_tsdb_timeout` makes the whole runtime config fail to load: Cortex exits at startup, and on an already-running process every reload fails, pinning the last good overrides and dropping `cortex_runtime_config_last_reload_successful` to 0. Before upgrading, raise `close_idle_tsdb_timeout` above the largest `query_ingesters_within` in your config and runtime overrides, or set it to 0 to disable closing idle TSDBs. The existing checks that `query_store_after` is less than `query_ingesters_within`, and that `shuffle_sharding_ingesters_lookback_period` is at least `query_store_after`, now also apply to per-tenant overrides. #7323 * [CHANGE] Alertmanager: Remove the obsolete startup migration of local state files into per-tenant directories (scheduled for removal in 1.11.0). Upgrading from a release older than 1.9.0 with a persisted local state directory now requires upgrading to an intermediate release first, so the migration can run. #7513 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446 -* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. The YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. The old field still works as the default for the new limit, but is deprecated and will be removed in v1.24.0: using it logs a warning and increments `deprecated_flags_inuse_total`. Setting the old field and a non-default limit to different values fails at startup. #7481 #7919 +* [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. The YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. The old field still works, but is deprecated and will be removed in v1.24.0: using it logs a warning and increments `deprecated_flags_inuse_total`. As before, the flag on the command line wins over either YAML field. Setting the old and the new YAML field to different values fails at startup. #7481 #7919 * [FEATURE] Parquet: Support sharded parquet file conversion and querying. #7610 * [FEATURE] Parquet Converter: Add experimental `-parquet-converter.max-num-columns` flag to automatically shard parquet files when the number of columns exceeds the configured limit. This prevents failures when a TSDB block has more unique label names than the parquet library's column limit (32767). #7624 * [FEATURE] Distributor: Add experimental `-distributor.num-query-workers` flag to use a goroutine worker pool for query fan-out calls to ingesters. Reuses pre-grown goroutine stacks to eliminate the `runtime.copystack` overhead (~8% CPU) observed on rulers with wide ingester fan-out. Falls back to spawning a new goroutine when no worker is available. #7623 diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 77c3f8fab56..1e8e186c873 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -131,6 +131,8 @@ func main() { return } + cfg.SetCommandLineFlags(flag.CommandLine) + // Validate the config once both the config file has been loaded // and CLI flags parsed. err = cfg.Validate(util_log.Logger) @@ -264,6 +266,11 @@ func LoadConfig(filename string, expandENV bool, cfg *cortex.Config) error { return errors.Wrap(err, "Error parsing config file") } + err = cfg.SetYAMLKeys(buf) + if err != nil { + return errors.Wrap(err, "Error parsing config file") + } + err = yaml.UnmarshalStrict(buf, cfg) if err != nil { return errors.Wrap(err, "Error parsing config file") diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index 30f06067754..928779ac574 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -62,6 +62,12 @@ func TestFlagParsing(t *testing.T) { stdoutMessage: "target: distributor\n", }, + "new flag on the command line overrides the deprecated YAML field, as in 1.21": { + yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s", + arguments: []string{"-distributor.ha-tracker.failover-timeout=30s"}, + stdoutMessage: "ha_tracker_failover_timeout: 30s\n", + }, + "user visible module listing": { arguments: []string{"-modules"}, stdoutMessage: "ingester *\n", diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 7daceb15476..25c5049ba02 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -101,6 +101,8 @@ type Config struct { HTTPPrefix string `yaml:"http_prefix"` NameValidationScheme model.ValidationScheme `yaml:"name_validation_scheme"` yamlRootNodeStates map[string]yamlRootNodeState + yamlKeys map[string]bool // dotted paths of the keys set in the YAML file + cliFlags map[string]bool // names of the flags set on the command line ResourceMonitor configs.ResourceMonitor `yaml:"resource_monitor"` @@ -171,6 +173,41 @@ func (c *Config) SetYAMLRootNodeStates(buf []byte) error { return nil } +// SetYAMLKeys records the dotted path of every key set in the YAML config file, so that +// validation can tell a setting the file sets from one left at its default. +func (c *Config) SetYAMLKeys(buf []byte) error { + var root map[any]any + if err := yaml.Unmarshal(buf, &root); err != nil { + return err + } + + c.yamlKeys = map[string]bool{} + var walk func(prefix string, node map[any]any) + walk = func(prefix string, node map[any]any) { + for key, value := range node { + name, ok := key.(string) + if !ok { + continue + } + c.yamlKeys[prefix+name] = true + if child, ok := value.(map[any]any); ok { + walk(prefix+name+".", child) + } + } + } + walk("", root) + return nil +} + +// SetCommandLineFlags records the flags set on the command line, so that validation can give +// them precedence over the YAML config file. Call it after fs has been parsed. +func (c *Config) SetCommandLineFlags(fs *flag.FlagSet) { + c.cliFlags = map[string]bool{} + fs.Visit(func(f *flag.Flag) { + c.cliFlags[f.Name] = true + }) +} + func isEmptyYAMLRootNode(value any) bool { if value == nil { return true @@ -335,7 +372,8 @@ func (c *Config) isModuleEnabled(m string) bool { } type deprecatedLimit struct { - oldName, newName string + oldName, newName string // YAML paths + oldFlag, newFlag string // CLI flag names; oldFlag is empty when the flag name did not change old time.Duration limit *model.Duration def model.Duration @@ -348,24 +386,44 @@ func (c *Config) deprecatedLimits() []deprecatedLimit { flagext.DefaultValues(&defaults) return []deprecatedLimit{ - {"querier.query_ingesters_within", "limits.query_ingesters_within", c.Querier.DeprecatedQueryIngestersWithin, &c.LimitsConfig.QueryIngestersWithin, defaults.QueryIngestersWithin}, - {"querier.query_store_after", "limits.query_store_after", c.Querier.DeprecatedQueryStoreAfter, &c.LimitsConfig.QueryStoreAfter, defaults.QueryStoreAfter}, - {"querier.shuffle_sharding_ingesters_lookback_period", "limits.shuffle_sharding_ingesters_lookback_period", c.Querier.DeprecatedShuffleShardingIngestersLookbackPeriod, &c.LimitsConfig.ShuffleShardingIngestersLookbackPeriod, defaults.ShuffleShardingIngestersLookbackPeriod}, - {"distributor.ha_tracker.ha_tracker_failover_timeout", "limits.ha_tracker_failover_timeout", c.Distributor.HATrackerConfig.DeprecatedFailoverTimeout, &c.LimitsConfig.HATrackerFailoverTimeout, defaults.HATrackerFailoverTimeout}, + {"querier.query_ingesters_within", "limits.query_ingesters_within", "querier.query-ingesters-within", "limits.query-ingesters-within", c.Querier.DeprecatedQueryIngestersWithin, &c.LimitsConfig.QueryIngestersWithin, defaults.QueryIngestersWithin}, + {"querier.query_store_after", "limits.query_store_after", "querier.query-store-after", "limits.query-store-after", c.Querier.DeprecatedQueryStoreAfter, &c.LimitsConfig.QueryStoreAfter, defaults.QueryStoreAfter}, + {"querier.shuffle_sharding_ingesters_lookback_period", "limits.shuffle_sharding_ingesters_lookback_period", "querier.shuffle-sharding-ingesters-lookback-period", "limits.shuffle-sharding-ingesters-lookback-period", c.Querier.DeprecatedShuffleShardingIngestersLookbackPeriod, &c.LimitsConfig.ShuffleShardingIngestersLookbackPeriod, defaults.ShuffleShardingIngestersLookbackPeriod}, + {"distributor.ha_tracker.ha_tracker_failover_timeout", "limits.ha_tracker_failover_timeout", "", "distributor.ha-tracker.failover-timeout", c.Distributor.HATrackerConfig.DeprecatedFailoverTimeout, &c.LimitsConfig.HATrackerFailoverTimeout, defaults.HATrackerFailoverTimeout}, + } +} + +// sources reports where the deprecated and the new setting were set. Without the sources +// recorded by SetYAMLKeys and SetCommandLineFlags, as when the config is built in code, a +// setting counts as set when it is not at its default value. +func (c *Config) sources(d deprecatedLimit) (oldCLI, oldYAML, newCLI, newYAML bool) { + if c.yamlKeys == nil && c.cliFlags == nil { + return false, d.old != 0, false, *d.limit != d.def } + return d.oldFlag != "" && c.cliFlags[d.oldFlag], c.yamlKeys[d.oldName], c.cliFlags[d.newFlag], c.yamlKeys[d.newName] } -// applyDeprecatedLimits copies each deprecated setting that is set into its limit, unless the -// limit is set to a different value. It must run before the limits are validated and before -// they become the defaults for the runtime config overrides. +// applyDeprecatedLimits copies each deprecated setting that is set into its limit, with the +// precedence that CLI flags have over the YAML file: a setting on the command line wins over +// one in the file, whichever name each uses. Within one source, the old and the new name +// must agree. It must run before the limits are validated and before they become the +// defaults for the runtime config overrides. func (c *Config) applyDeprecatedLimits() error { for _, d := range c.deprecatedLimits() { + oldCLI, oldYAML, newCLI, newYAML := c.sources(d) + switch { - case d.old == 0: - case *d.limit == d.def: + case oldCLI && newCLI, !oldCLI && !newCLI && oldYAML && newYAML: + if time.Duration(*d.limit) != d.old { + oldName, newName := d.oldName, d.newName + if oldCLI { + oldName, newName = "-"+d.oldFlag, "-"+d.newFlag + } + return fmt.Errorf("%s (%s) and the deprecated %s (%s) are both set: remove %s", newName, time.Duration(*d.limit), oldName, d.old, oldName) + } + case newCLI: + case oldCLI, oldYAML && !newYAML: *d.limit = model.Duration(d.old) - case time.Duration(*d.limit) != d.old: - return fmt.Errorf("%s (%s) and the deprecated %s (%s) are both set: remove %s", d.newName, time.Duration(*d.limit), d.oldName, d.old, d.oldName) } } return nil @@ -375,7 +433,7 @@ func (c *Config) applyDeprecatedLimits() error { // applyDeprecatedLimits because Validate runs before the logger is initialised. func (c *Config) warnDeprecatedLimits(logger log.Logger) { for _, d := range c.deprecatedLimits() { - if d.old != 0 { + if oldCLI, oldYAML, _, _ := c.sources(d); oldCLI || oldYAML { flagext.DeprecatedFlagsUsed.Inc() level.Warn(logger).Log("msg", "running with DEPRECATED config "+d.oldName+", use "+d.newName+" instead, it will be removed in v1.24.0") } diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 97f374dc1b1..457fa59b636 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -352,10 +352,46 @@ func TestConfigValidation_DeprecatedLimits(t *testing.T) { expectedError: "limits.query_store_after (6h0m0s) and the deprecated querier.query_store_after (12h0m0s) are both set: remove querier.query_store_after", }, { - name: "old HA tracker field and a non-default limit set to different values", + name: "old HA tracker YAML field and the new one set to different values", yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s\nlimits:\n ha_tracker_failover_timeout: 1m\n", expectedError: "limits.ha_tracker_failover_timeout (1m0s) and the deprecated distributor.ha_tracker.ha_tracker_failover_timeout (45s) are both set: remove distributor.ha_tracker.ha_tracker_failover_timeout", }, + { + name: "a new flag on the command line wins over the old YAML field, even at its default value", + args: []string{"-distributor.ha-tracker.failover-timeout=30s"}, + yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(30*time.Second), cfg.LimitsConfig.HATrackerFailoverTimeout) + }, + }, + { + name: "a new querier flag on the command line wins over the old YAML field, even at its default value", + args: []string{"-limits.query-store-after=0"}, + yaml: "querier:\n query_store_after: 12h\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(0), cfg.LimitsConfig.QueryStoreAfter) + }, + }, + { + name: "an old flag on the command line wins over the new YAML field", + args: []string{"-querier.query-store-after=12h"}, + yaml: "limits:\n query_ingesters_within: 13h\n query_store_after: 6h\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(12*time.Hour), cfg.LimitsConfig.QueryStoreAfter) + }, + }, + { + name: "old and new flags on the command line set to different values", + args: []string{"-querier.query-store-after=12h", "-limits.query-store-after=6h"}, + expectedError: "-limits.query-store-after (6h0m0s) and the deprecated -querier.query-store-after (12h0m0s) are both set: remove -querier.query-store-after", + }, + { + name: "old HA tracker YAML field and the new one set to the same value", + yaml: "distributor:\n ha_tracker:\n ha_tracker_failover_timeout: 45s\nlimits:\n ha_tracker_failover_timeout: 45s\n", + check: func(t *testing.T, cfg *Config) { + assert.Equal(t, model.Duration(45*time.Second), cfg.LimitsConfig.HATrackerFailoverTimeout) + }, + }, { name: "old fields are validated like the new ones", yaml: "querier:\n query_ingesters_within: 12h\n query_store_after: 12h\n", @@ -363,11 +399,15 @@ func TestConfigValidation_DeprecatedLimits(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { + // Load the config the way cmd/cortex does: defaults, then the YAML file, then the + // command line on top. cfg := &Config{} fs := flag.NewFlagSet("", flag.PanicOnError) cfg.RegisterFlags(fs) - require.NoError(t, fs.Parse(tc.args)) + require.NoError(t, cfg.SetYAMLKeys([]byte(tc.yaml))) require.NoError(t, yaml.UnmarshalStrict([]byte(tc.yaml), cfg)) + require.NoError(t, fs.Parse(tc.args)) + cfg.SetCommandLineFlags(fs) err := cfg.Validate(nil) if tc.expectedError != "" { @@ -380,6 +420,21 @@ func TestConfigValidation_DeprecatedLimits(t *testing.T) { } } +func TestConfigValidation_DeprecatedLimitsWithoutSources(t *testing.T) { + // A config built in code has no recorded sources, so a setting counts as set when it is + // not at its default value. + cfg := newDefaultConfig() + cfg.Querier.DeprecatedQueryStoreAfter = 12 * time.Hour + cfg.LimitsConfig.QueryIngestersWithin = model.Duration(13 * time.Hour) + require.NoError(t, cfg.Validate(nil)) + assert.Equal(t, model.Duration(12*time.Hour), cfg.LimitsConfig.QueryStoreAfter) + + cfg = newDefaultConfig() + cfg.Querier.DeprecatedQueryStoreAfter = 12 * time.Hour + cfg.LimitsConfig.QueryStoreAfter = model.Duration(6 * time.Hour) + require.ErrorContains(t, cfg.Validate(nil), "are both set: remove querier.query_store_after") +} + func TestGrpcAuthMiddleware(t *testing.T) { prepareGlobalMetricsRegistry(t) diff --git a/pkg/ha/ha_tracker.go b/pkg/ha/ha_tracker.go index 60878b2ba80..649b433f8b3 100644 --- a/pkg/ha/ha_tracker.go +++ b/pkg/ha/ha_tracker.go @@ -67,7 +67,7 @@ type HATrackerConfig struct { // in v1.24.0. Kept so that 1.21 configs still start. YAML only: the // -distributor.ha-tracker.failover-timeout flag now sets the limit. A non-zero value is // applied as the default limit by cortex.Config.Validate. - DeprecatedFailoverTimeout time.Duration `yaml:"ha_tracker_failover_timeout" doc:"hidden"` + DeprecatedFailoverTimeout time.Duration `yaml:"ha_tracker_failover_timeout,omitempty" doc:"hidden"` // EnableStartupSync controls whether to fetch all tracked keys from the KV store // on startup to populate the local cache. // This prevents duplicate GET calls for the same key while the cache is cold, diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index 6b2ec5c1ed5..3f24ad3a27b 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -74,9 +74,9 @@ type Config struct { // These moved to the per-tenant limits of the same name, and are scheduled for removal // in v1.24.0. Kept so that 1.21 configs still start. A non-zero value is applied as the // default limit by cortex.Config.Validate. - DeprecatedQueryIngestersWithin time.Duration `yaml:"query_ingesters_within" doc:"hidden"` - DeprecatedQueryStoreAfter time.Duration `yaml:"query_store_after" doc:"hidden"` - DeprecatedShuffleShardingIngestersLookbackPeriod time.Duration `yaml:"shuffle_sharding_ingesters_lookback_period" doc:"hidden"` + DeprecatedQueryIngestersWithin time.Duration `yaml:"query_ingesters_within,omitempty" doc:"hidden"` + DeprecatedQueryStoreAfter time.Duration `yaml:"query_store_after,omitempty" doc:"hidden"` + DeprecatedShuffleShardingIngestersLookbackPeriod time.Duration `yaml:"shuffle_sharding_ingesters_lookback_period,omitempty" doc:"hidden"` // Blocks storage only. StoreGatewayAddresses string `yaml:"store_gateway_addresses"`