From 3ef0ebf042172b0dee2ac73afaee60ee0329e8af Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Thu, 26 Sep 2024 10:37:20 +1000 Subject: [PATCH] MQE: remove obsolete feature flags (#9399) * MQE: remove obsolete feature flags * Add changelog entry --- CHANGELOG.md | 2 +- cmd/mimir/config-descriptor.json | 44 ----------------- cmd/mimir/help-all.txt.tmpl | 8 ---- .../configuration-parameters/index.md | 20 -------- pkg/streamingpromql/config.go | 22 --------- pkg/streamingpromql/engine_test.go | 48 ------------------- pkg/streamingpromql/query.go | 28 ----------- 7 files changed, 1 insertion(+), 171 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d51f69e715..3a31945536b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ * [CHANGE] Querier: Remove deprecated `-querier.max-query-into-future`. The feature was deprecated in Mimir 2.12. #9407 * [FEATURE] Alertmanager: Added `-alertmanager.log-parsing-label-matchers` to control logging when parsing label matchers. This flag is intended to be used with `-alertmanager.utf8-strict-mode-enabled` to validate UTF-8 strict mode is working as intended. The default value is `false`. #9173 * [FEATURE] Alertmanager: Added `-alertmanager.utf8-migration-logging-enabled` to enable logging of tenant configurations that are incompatible with UTF-8 strict mode. The default value is `false`. #9174 -* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #8422 #8430 #8454 #8455 #8360 #8490 #8508 #8577 #8660 #8671 #8677 #8747 #8850 #8872 #8838 #8911 #8909 #8923 #8924 #8925 #8932 #8933 #8934 #8962 #8986 #8993 #8995 #9008 #9017 #9018 #9019 #9120 #9121 #9136 #9139 #9140 #9145 #9191 #9192 #9194 #9196 #9201 #9212 #9225 #9260 #9272 #9277 #9278 #9280 #9281 #9342 #9343 #9367 #9368 #9371 +* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #8422 #8430 #8454 #8455 #8360 #8490 #8508 #8577 #8660 #8671 #8677 #8747 #8850 #8872 #8838 #8911 #8909 #8923 #8924 #8925 #8932 #8933 #8934 #8962 #8986 #8993 #8995 #9008 #9017 #9018 #9019 #9120 #9121 #9136 #9139 #9140 #9145 #9191 #9192 #9194 #9196 #9201 #9212 #9225 #9260 #9272 #9277 #9278 #9280 #9281 #9342 #9343 #9367 #9368 #9371 #9399 * [FEATURE] Experimental Kafka-based ingest storage. #6888 #6894 #6929 #6940 #6951 #6974 #6982 #7029 #7030 #7091 #7142 #7147 #7148 #7153 #7160 #7193 #7349 #7376 #7388 #7391 #7393 #7394 #7402 #7404 #7423 #7424 #7437 #7486 #7503 #7508 #7540 #7621 #7682 #7685 #7694 #7695 #7696 #7697 #7701 #7733 #7734 #7741 #7752 #7838 #7851 #7871 #7877 #7880 #7882 #7887 #7891 #7925 #7955 #7967 #8031 #8063 #8077 #8088 #8135 #8176 #8184 #8194 #8216 #8217 #8222 #8233 #8503 #8542 #8579 #8657 #8686 #8688 #8703 #8706 #8708 #8738 #8750 #8778 #8808 #8809 #8841 #8842 #8845 #8853 #8886 #8988 * What it is: * When the new ingest storage architecture is enabled, distributors write incoming write requests to a Kafka-compatible backend, and the ingesters asynchronously replay ingested data from Kafka. In this architecture, the write and read path are de-coupled through a Kafka-compatible backend. The write path and Kafka load is a function of the incoming write traffic, the read path load is a function of received queries. Whatever the load on the read path, it doesn't affect the write path. diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 8a2d0a6ed6e..f8a3b38c3fd 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -1978,39 +1978,6 @@ "fieldType": "boolean", "fieldCategory": "experimental" }, - { - "kind": "field", - "name": "enable_binary_operations", - "required": false, - "desc": "Enable support for binary operations in Mimir's query engine. Only applies if the Mimir query engine is in use.", - "fieldValue": null, - "fieldDefaultValue": true, - "fieldFlag": "querier.mimir-query-engine.enable-binary-operations", - "fieldType": "boolean", - "fieldCategory": "experimental" - }, - { - "kind": "field", - "name": "enable_offset_modifier", - "required": false, - "desc": "Enable support for offset modifier in Mimir's query engine. Only applies if the Mimir query engine is in use.", - "fieldValue": null, - "fieldDefaultValue": true, - "fieldFlag": "querier.mimir-query-engine.enable-offset-modifier", - "fieldType": "boolean", - "fieldCategory": "experimental" - }, - { - "kind": "field", - "name": "enable_over_time_functions", - "required": false, - "desc": "Enable support for ..._over_time functions in Mimir's query engine. Only applies if the Mimir query engine is in use.", - "fieldValue": null, - "fieldDefaultValue": true, - "fieldFlag": "querier.mimir-query-engine.enable-over-time-functions", - "fieldType": "boolean", - "fieldCategory": "experimental" - }, { "kind": "field", "name": "enable_scalars", @@ -2021,17 +1988,6 @@ "fieldFlag": "querier.mimir-query-engine.enable-scalars", "fieldType": "boolean", "fieldCategory": "experimental" - }, - { - "kind": "field", - "name": "enable_unary_negation", - "required": false, - "desc": "Enable support for unary negation in Mimir's query engine. Only applies if the Mimir query engine is in use.", - "fieldValue": null, - "fieldDefaultValue": true, - "fieldFlag": "querier.mimir-query-engine.enable-unary-negation", - "fieldType": "boolean", - "fieldCategory": "experimental" } ], "fieldValue": null, diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index f58695be7ff..42c819815b1 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -1921,16 +1921,8 @@ Usage of ./cmd/mimir/mimir: Maximum number of samples a single query can load into memory. This config option should be set on query-frontend too when query sharding is enabled. (default 50000000) -querier.mimir-query-engine.enable-aggregation-operations [experimental] Enable support for aggregation operations in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) - -querier.mimir-query-engine.enable-binary-operations - [experimental] Enable support for binary operations in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) - -querier.mimir-query-engine.enable-offset-modifier - [experimental] Enable support for offset modifier in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) - -querier.mimir-query-engine.enable-over-time-functions - [experimental] Enable support for ..._over_time functions in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) -querier.mimir-query-engine.enable-scalars [experimental] Enable support for scalars in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) - -querier.mimir-query-engine.enable-unary-negation - [experimental] Enable support for unary negation in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true) -querier.minimize-ingester-requests If true, when querying ingesters, only the minimum required ingesters required to reach quorum will be queried initially, with other ingesters queried only if needed due to failures from the initial set of ingesters. Enabling this option reduces resource consumption for the happy path at the cost of increased latency for the unhappy path. (default true) -querier.minimize-ingester-requests-hedging-delay duration diff --git a/docs/sources/mimir/configure/configuration-parameters/index.md b/docs/sources/mimir/configure/configuration-parameters/index.md index a74ace45c35..dd5988d7e4e 100644 --- a/docs/sources/mimir/configure/configuration-parameters/index.md +++ b/docs/sources/mimir/configure/configuration-parameters/index.md @@ -1504,30 +1504,10 @@ mimir_query_engine: # CLI flag: -querier.mimir-query-engine.enable-aggregation-operations [enable_aggregation_operations: | default = true] - # (experimental) Enable support for binary operations in Mimir's query engine. - # Only applies if the Mimir query engine is in use. - # CLI flag: -querier.mimir-query-engine.enable-binary-operations - [enable_binary_operations: | default = true] - - # (experimental) Enable support for offset modifier in Mimir's query engine. - # Only applies if the Mimir query engine is in use. - # CLI flag: -querier.mimir-query-engine.enable-offset-modifier - [enable_offset_modifier: | default = true] - - # (experimental) Enable support for ..._over_time functions in Mimir's query - # engine. Only applies if the Mimir query engine is in use. - # CLI flag: -querier.mimir-query-engine.enable-over-time-functions - [enable_over_time_functions: | default = true] - # (experimental) Enable support for scalars in Mimir's query engine. Only # applies if the Mimir query engine is in use. # CLI flag: -querier.mimir-query-engine.enable-scalars [enable_scalars: | default = true] - - # (experimental) Enable support for unary negation in Mimir's query engine. - # Only applies if the Mimir query engine is in use. - # CLI flag: -querier.mimir-query-engine.enable-unary-negation - [enable_unary_negation: | default = true] ``` ### frontend diff --git a/pkg/streamingpromql/config.go b/pkg/streamingpromql/config.go index e6fbc6f5ebb..42bbfb4bd52 100644 --- a/pkg/streamingpromql/config.go +++ b/pkg/streamingpromql/config.go @@ -19,21 +19,7 @@ type EngineOpts struct { type FeatureToggles struct { EnableAggregationOperations bool `yaml:"enable_aggregation_operations" category:"experimental"` - EnableBinaryOperations bool `yaml:"enable_binary_operations" category:"experimental"` - EnableOffsetModifier bool `yaml:"enable_offset_modifier" category:"experimental"` - EnableOverTimeFunctions bool `yaml:"enable_over_time_functions" category:"experimental"` EnableScalars bool `yaml:"enable_scalars" category:"experimental"` - EnableUnaryNegation bool `yaml:"enable_unary_negation" category:"experimental"` -} - -var overTimeFunctionNames = []string{ - "avg_over_time", - "count_over_time", - "last_over_time", - "max_over_time", - "min_over_time", - "present_over_time", - "sum_over_time", } // EnableAllFeatures enables all features supported by MQE, including experimental or incomplete features. @@ -41,17 +27,9 @@ var EnableAllFeatures = FeatureToggles{ // Note that we deliberately use a keyless literal here to force a compilation error if we don't keep this in sync with new fields added to FeatureToggles. true, true, - true, - true, - true, - true, } func (t *FeatureToggles) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&t.EnableAggregationOperations, "querier.mimir-query-engine.enable-aggregation-operations", true, "Enable support for aggregation operations in Mimir's query engine. Only applies if the Mimir query engine is in use.") - f.BoolVar(&t.EnableBinaryOperations, "querier.mimir-query-engine.enable-binary-operations", true, "Enable support for binary operations in Mimir's query engine. Only applies if the Mimir query engine is in use.") - f.BoolVar(&t.EnableOffsetModifier, "querier.mimir-query-engine.enable-offset-modifier", true, "Enable support for offset modifier in Mimir's query engine. Only applies if the Mimir query engine is in use.") - f.BoolVar(&t.EnableOverTimeFunctions, "querier.mimir-query-engine.enable-over-time-functions", true, "Enable support for ..._over_time functions in Mimir's query engine. Only applies if the Mimir query engine is in use.") f.BoolVar(&t.EnableScalars, "querier.mimir-query-engine.enable-scalars", true, "Enable support for scalars in Mimir's query engine. Only applies if the Mimir query engine is in use.") - f.BoolVar(&t.EnableUnaryNegation, "querier.mimir-query-engine.enable-unary-negation", true, "Enable support for unary negation in Mimir's query engine. Only applies if the Mimir query engine is in use.") } diff --git a/pkg/streamingpromql/engine_test.go b/pkg/streamingpromql/engine_test.go index 3ec340ab58e..c0e814100b2 100644 --- a/pkg/streamingpromql/engine_test.go +++ b/pkg/streamingpromql/engine_test.go @@ -82,43 +82,6 @@ func TestUnsupportedPromQLFeaturesWithFeatureToggles(t *testing.T) { requireInstantQueryIsUnsupported(t, featureToggles, "sum by (label) (metric)", "aggregation operations") }) - t.Run("binary expressions", func(t *testing.T) { - featureToggles := EnableAllFeatures - featureToggles.EnableBinaryOperations = false - - requireRangeQueryIsUnsupported(t, featureToggles, "metric{} + other_metric{}", "binary expressions") - requireInstantQueryIsUnsupported(t, featureToggles, "metric{} + other_metric{}", "binary expressions") - - requireRangeQueryIsUnsupported(t, featureToggles, "metric{} + 1", "binary expressions") - requireInstantQueryIsUnsupported(t, featureToggles, "metric{} + 1", "binary expressions") - - requireRangeQueryIsUnsupported(t, featureToggles, "1 + metric{}", "binary expressions") - requireInstantQueryIsUnsupported(t, featureToggles, "1 + metric{}", "binary expressions") - - requireRangeQueryIsUnsupported(t, featureToggles, "2 + 1", "binary expressions") - requireInstantQueryIsUnsupported(t, featureToggles, "2 + 1", "binary expressions") - }) - - t.Run("..._over_time functions", func(t *testing.T) { - featureToggles := EnableAllFeatures - featureToggles.EnableOverTimeFunctions = false - - requireRangeQueryIsUnsupported(t, featureToggles, "count_over_time(metric[1m])", "'count_over_time' function") - requireInstantQueryIsUnsupported(t, featureToggles, "count_over_time(metric[1m])", "'count_over_time' function") - }) - - t.Run("offset modifier", func(t *testing.T) { - featureToggles := EnableAllFeatures - featureToggles.EnableOffsetModifier = false - - requireRangeQueryIsUnsupported(t, featureToggles, "metric offset 1m", "instant vector selector with 'offset'") - requireInstantQueryIsUnsupported(t, featureToggles, "metric offset 1m", "instant vector selector with 'offset'") - requireInstantQueryIsUnsupported(t, featureToggles, "metric[2m] offset 1m", "range vector selector with 'offset'") - - requireRangeQueryIsUnsupported(t, featureToggles, "rate(metric[2m] offset 1m)", "range vector selector with 'offset'") - requireInstantQueryIsUnsupported(t, featureToggles, "rate(metric[2m] offset 1m)", "range vector selector with 'offset'") - }) - t.Run("scalars", func(t *testing.T) { featureToggles := EnableAllFeatures featureToggles.EnableScalars = false @@ -126,17 +89,6 @@ func TestUnsupportedPromQLFeaturesWithFeatureToggles(t *testing.T) { requireRangeQueryIsUnsupported(t, featureToggles, "2", "scalar values") requireInstantQueryIsUnsupported(t, featureToggles, "2", "scalar values") }) - - t.Run("unary negation", func(t *testing.T) { - featureToggles := EnableAllFeatures - featureToggles.EnableUnaryNegation = false - - requireRangeQueryIsUnsupported(t, featureToggles, "-sum(metric{})", "unary negation of instant vectors") - requireInstantQueryIsUnsupported(t, featureToggles, "-sum(metric{})", "unary negation of instant vectors") - - requireRangeQueryIsUnsupported(t, featureToggles, "-(1)", "unary negation of scalars") - requireInstantQueryIsUnsupported(t, featureToggles, "-(1)", "unary negation of scalars") - }) } func requireRangeQueryIsUnsupported(t *testing.T, featureToggles FeatureToggles, expression string, expectedError string) { diff --git a/pkg/streamingpromql/query.go b/pkg/streamingpromql/query.go index e97da911489..0e2394fbdfe 100644 --- a/pkg/streamingpromql/query.go +++ b/pkg/streamingpromql/query.go @@ -125,10 +125,6 @@ func (q *Query) convertToInstantVectorOperator(expr parser.Expr) (types.InstantV lookbackDelta = q.engine.lookbackDelta } - if !q.engine.featureToggles.EnableOffsetModifier && (e.OriginalOffset != 0 || e.Offset != 0) { - return nil, compat.NewNotSupportedError("instant vector selector with 'offset'") - } - return &operators.InstantVectorSelector{ MemoryConsumptionTracker: q.memoryConsumptionTracker, Selector: &operators.Selector{ @@ -169,10 +165,6 @@ func (q *Query) convertToInstantVectorOperator(expr parser.Expr) (types.InstantV case *parser.Call: return q.convertFunctionCallToInstantVectorOperator(e) case *parser.BinaryExpr: - if !q.engine.featureToggles.EnableBinaryOperations { - return nil, compat.NewNotSupportedError("binary expressions") - } - // We only need to handle three combinations of types here: // Scalar on left, vector on right // Vector on left, scalar on right @@ -239,10 +231,6 @@ func (q *Query) convertToInstantVectorOperator(expr parser.Expr) (types.InstantV return nil, compat.NewNotSupportedError(fmt.Sprintf("unary expression with '%s'", e.Op)) } - if !q.engine.featureToggles.EnableUnaryNegation { - return nil, compat.NewNotSupportedError("unary negation of instant vectors") - } - inner, err := q.convertToInstantVectorOperator(e.Expr) if err != nil { return nil, err @@ -261,10 +249,6 @@ func (q *Query) convertToInstantVectorOperator(expr parser.Expr) (types.InstantV } func (q *Query) convertFunctionCallToInstantVectorOperator(e *parser.Call) (types.InstantVectorOperator, error) { - if !q.engine.featureToggles.EnableOverTimeFunctions && slices.Contains(overTimeFunctionNames, e.Func.Name) { - return nil, compat.NewNotSupportedError(fmt.Sprintf("'%s' function", e.Func.Name)) - } - factory, ok := instantVectorFunctionOperatorFactories[e.Func.Name] if !ok { return nil, compat.NewNotSupportedError(fmt.Sprintf("'%s' function", e.Func.Name)) @@ -291,10 +275,6 @@ func (q *Query) convertToRangeVectorOperator(expr parser.Expr) (types.RangeVecto case *parser.MatrixSelector: vectorSelector := e.VectorSelector.(*parser.VectorSelector) - if !q.engine.featureToggles.EnableOffsetModifier && (vectorSelector.OriginalOffset != 0 || vectorSelector.Offset != 0) { - return nil, compat.NewNotSupportedError("range vector selector with 'offset'") - } - return &operators.RangeVectorSelector{ Selector: &operators.Selector{ Queryable: q.queryable, @@ -345,10 +325,6 @@ func (q *Query) convertToScalarOperator(expr parser.Expr) (types.ScalarOperator, return nil, compat.NewNotSupportedError(fmt.Sprintf("unary expression with '%s'", e.Op)) } - if !q.engine.featureToggles.EnableUnaryNegation { - return nil, compat.NewNotSupportedError("unary negation of scalars") - } - inner, err := q.convertToScalarOperator(e.Expr) if err != nil { return nil, err @@ -362,10 +338,6 @@ func (q *Query) convertToScalarOperator(expr parser.Expr) (types.ScalarOperator, case *parser.ParenExpr: return q.convertToScalarOperator(e.Expr) case *parser.BinaryExpr: - if !q.engine.featureToggles.EnableBinaryOperations { - return nil, compat.NewNotSupportedError("binary expressions") - } - lhs, err := q.convertToScalarOperator(e.LHS) if err != nil { return nil, err