Repository navigation
Backport #7919 to release-1.22 - #7918
CharlieTLe merged 1 commit into
Conversation
friedrichg
left a comment
There was a problem hiding this comment.
Thanks for catching!. I left my thoughts.
| * [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). Both the YAML fields and the CLI flags move, with no deprecated alias, so a config that still uses the old names makes Cortex **exit at startup**. Update both before upgrading. #7160 #7323 |
There was a problem hiding this comment.
Thanks for catching it. we should see if we can have old stanzas continue to work, we don't want to introduce breaking changes in these very old features.
There was a problem hiding this comment.
Agreed. #7919 restores the old querier.* YAML fields and the -querier.* flags as deprecated aliases for the new limits, scheduled for removal in v1.24.0. Using one logs a warning and increments deprecated_flags_inuse_total, as #7793 does for max-exemplars. If the old and new names are set to different values, startup fails instead of silently picking one. This entry then becomes a deprecation note rather than a break.
| * [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). Both the YAML fields and the CLI flags move, with no deprecated alias, so a config that still uses the old names makes Cortex **exit at startup**. Update both before upgrading. #7160 #7323 | ||
| - YAML: move the three fields from the `querier` block to `limits`. Leaving them under `querier` fails with `field query_store_after not found in type querier.Config`. | ||
| - CLI: `-querier.query-ingesters-within`, `-querier.query-store-after` and `-querier.shuffle-sharding-ingesters-lookback-period` are renamed to `-limits.query-ingesters-within`, `-limits.query-store-after` and `-limits.shuffle-sharding-ingesters-lookback-period`. Passing an old name fails with `flag provided but not defined`. | ||
| * [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 |
There was a problem hiding this comment.
This changelog line is fine. This is actually a bug fix in correctness so it's ok in my opinion
| * [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, but the YAML field moves from `distributor.ha_tracker.ha_tracker_failover_timeout` to `limits.ha_tracker_failover_timeout`. Leaving it under `distributor.ha_tracker` makes Cortex **exit at startup** with `field ha_tracker_failover_timeout not found in type ha.HATrackerConfig`. #7481 |
There was a problem hiding this comment.
same. we should try to support the old stanzas if possible.
There was a problem hiding this comment.
Done in #7919 too: distributor.ha_tracker.ha_tracker_failover_timeout is accepted again as a deprecated YAML-only alias. The flag name never changed, so the flag already works.
|
The fix for the review is in #7919, against master. Once it merges, I'll turn this PR into its backport to |
…ortexproject#7919) * Keep the 1.21 query range and HA failover timeout settings working cortexproject#7160/cortexproject#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.*. cortexproject#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 cortexproject#7323 rejects equal, and spell out that new startup check in the CHANGELOG. Signed-off-by: Charlie Le <charlie_le@apple.com> * CHANGELOG: reference cortexproject#7919 Signed-off-by: Charlie Le <charlie_le@apple.com> * 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 <charlie_le@apple.com> * 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 <charlie_le@apple.com> --------- Signed-off-by: Charlie Le <charlie_le@apple.com> (cherry picked from commit b6b08e7) Signed-off-by: Charlie Le <charlie_le@apple.com>
a759dbe to
af8c2c2
Compare
Backports #7919 to
release-1.22, so that v1.22.0 starts with 1.21 configs that set the query range or HA failover timeout settings under their old names. It replaces the CHANGELOG-only change this PR first had, as discussed in review.#7919: keep the 1.21 query range and HA failover timeout settings working
querier.{query_ingesters_within, query_store_after, shuffle_sharding_ingesters_lookback_period}and their-querier.*flags, anddistributor.ha_tracker.ha_tracker_failover_timeout, work again as deprecated aliases of the new limits, scheduled for removal in v1.24.0. Using one logs a warning and incrementsdeprecated_flags_inuse_total.-limits.query-ingesters-withinhelp text now says "less than"close-idle-tsdb-timeout, matching the check from making query ingesters config per tenant #7323.query_ingesters_within < close_idle_tsdb_timeoutstartup check.The squash commit cherry-picked cleanly with
-x. Its CHANGELOG changes land in the1.22.0 in progresssection.Verification on this branch
go build -tags "netgo slicelabels" ./cmd/cortex: cleango test ./pkg/cortex/ ./pkg/ha/ ./pkg/querier/ ./pkg/util/validation/ ./cmd/cortex/: okgolangci-lintv2.13.1: 0 issues. The regenerated config reference,blocks-storage/querier.mdand the JSON schema match the committed ones.-print.configfrom the built binary: an old HA YAML field of 45s gives 45s, and 30s with-distributor.ha-tracker.failover-timeout=30s. An oldquerier.query_store_after: 12hgives 12h, and 6h with-querier.query-store-after=6h. A config withquery_ingesters_withinequal toclose_idle_tsdb_timeoutis still rejected.