Skip to content

Commit

Permalink
MQE: add support for and (#9507)
Browse files Browse the repository at this point in the history
* Enable upstream test cases

* Add feature flag

# Conflicts:
#	cmd/mimir/help-all.txt.tmpl
#	pkg/streamingpromql/config.go
#	pkg/streamingpromql/engine_test.go

* Add test cases.

* Add skeleton

* Initial implementation

* Add to concurrency test

* Add to edge cases test

* Enable benchmarks

* Clarify comment

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `vectorMatchingGroupKeyFunc` to its own file.

* Fix test

* Address PR feedback: simplify `NextSeries`

* Fix unpooled metadata slice creation and add linting script to catch similar issues in the future

* Reuse slices in `FilterLeftSeries` if we can

* Add missing license header to script

* Reuse left series metadata slice
  • Loading branch information
charleskorn authored Oct 4, 2024
1 parent 9a03bb5 commit 665bb0f
Show file tree
Hide file tree
Showing 16 changed files with 561 additions and 87 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* `cortex_alertmanager_alerts`
* `cortex_alertmanager_silences`
* [CHANGE] Cache: Deprecate experimental support for Redis as a cache backend. #9453
* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #9367 #9368 #9398 #9399 #9403 #9417 #9418 #9419 #9420 #9482 #9504 #9505
* [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #9367 #9368 #9398 #9399 #9403 #9417 #9418 #9419 #9420 #9482 #9504 #9505 #9507
* [FEATURE] Query-frontend: added experimental configuration options `query-frontend.cache-errors` and `query-frontend.results-cache-ttl-for-errors` to allow non-transient responses to be cached. When set to `true` error responses from hitting limits or bad data are cached for a short TTL. #9028
* [FEATURE] gRPC: Support S2 compression. #9322
* `-alertmanager.alertmanager-client.grpc-compression=s2`
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ lint: ## Run lints to check for style issues.
lint: check-makefiles
misspell -error $(DOC_SOURCES_PATH)

./tools/find-unpooled-slice-creation.sh

# Configured via .golangci.yml.
golangci-lint run

Expand Down
11 changes: 11 additions & 0 deletions cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,17 @@
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "enable_binary_logical_operations",
"required": false,
"desc": "Enable support for binary logical 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-logical-operations",
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "enable_scalars",
Expand Down
2 changes: 2 additions & 0 deletions cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,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-logical-operations
[experimental] Enable support for binary logical operations in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true)
-querier.mimir-query-engine.enable-scalar-scalar-binary-comparison-operations
[experimental] Enable support for binary comparison operations between two scalars in Mimir's query engine. Only applies if the Mimir query engine is in use. (default true)
-querier.mimir-query-engine.enable-scalars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,11 @@ mimir_query_engine:
# CLI flag: -querier.mimir-query-engine.enable-scalar-scalar-binary-comparison-operations
[enable_scalar_scalar_binary_comparison_operations: <boolean> | default = true]
# (experimental) Enable support for binary logical operations in Mimir's query
# engine. Only applies if the Mimir query engine is in use.
# CLI flag: -querier.mimir-query-engine.enable-binary-logical-operations
[enable_binary_logical_operations: <boolean> | 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
Expand Down
12 changes: 6 additions & 6 deletions pkg/streamingpromql/benchmarks/benchmarks.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,18 +167,18 @@ func TestCases(metricSizes []int) []BenchCase {
{
Expr: `a_2000 - b_2000{l="1234"}`,
},
//{
// Expr: "a_X and b_X{l=~'.*[0-4]$'}",
//},
{
Expr: "a_X and b_X{l=~'.*[0-4]$'}",
},
//{
// Expr: "a_X or b_X{l=~'.*[0-4]$'}",
//},
//{
// Expr: "a_X unless b_X{l=~'.*[0-4]$'}",
//},
//{
// Expr: "a_X and b_X{l='notfound'}",
//},
{
Expr: "a_X and b_X{l='notfound'}",
},
//// Simple functions.
//{
// Expr: "abs(a_X)",
Expand Down
3 changes: 3 additions & 0 deletions pkg/streamingpromql/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type FeatureToggles struct {
EnableVectorVectorBinaryComparisonOperations bool `yaml:"enable_vector_vector_binary_comparison_operations" category:"experimental"`
EnableVectorScalarBinaryComparisonOperations bool `yaml:"enable_vector_scalar_binary_comparison_operations" category:"experimental"`
EnableScalarScalarBinaryComparisonOperations bool `yaml:"enable_scalar_scalar_binary_comparison_operations" category:"experimental"`
EnableBinaryLogicalOperations bool `yaml:"enable_binary_logical_operations" category:"experimental"`
EnableScalars bool `yaml:"enable_scalars" category:"experimental"`
}

Expand All @@ -33,12 +34,14 @@ var EnableAllFeatures = FeatureToggles{
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.EnableVectorVectorBinaryComparisonOperations, "querier.mimir-query-engine.enable-vector-vector-binary-comparison-operations", true, "Enable support for binary comparison operations between two vectors in Mimir's query engine. Only applies if the Mimir query engine is in use.")
f.BoolVar(&t.EnableVectorScalarBinaryComparisonOperations, "querier.mimir-query-engine.enable-vector-scalar-binary-comparison-operations", true, "Enable support for binary comparison operations between a vector and a scalar in Mimir's query engine. Only applies if the Mimir query engine is in use.")
f.BoolVar(&t.EnableScalarScalarBinaryComparisonOperations, "querier.mimir-query-engine.enable-scalar-scalar-binary-comparison-operations", true, "Enable support for binary comparison operations between two scalars in Mimir's query engine. Only applies if the Mimir query engine is in use.")
f.BoolVar(&t.EnableBinaryLogicalOperations, "querier.mimir-query-engine.enable-binary-logical-operations", true, "Enable support for binary logical operations 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.")
}
24 changes: 24 additions & 0 deletions pkg/streamingpromql/engine_concurrency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,30 @@ func TestConcurrentQueries(t *testing.T) {
end: startT.Add(10 * time.Minute),
step: time.Minute,
},
{
expr: `float{group="a"} and on (instance) float{group="b"}`,
start: startT,
end: startT.Add(10 * time.Minute),
step: time.Minute,
},
{
expr: `native_histogram{group="a"} and on (instance) float{group="b"}`,
start: startT,
end: startT.Add(10 * time.Minute),
step: time.Minute,
},
{
expr: `native_histogram{group="a"} and on (instance) native_histogram{group="b"}`,
start: startT,
end: startT.Add(10 * time.Minute),
step: time.Minute,
},
{
expr: `{group="a"} and on (instance) float{group="b"}`,
start: startT,
end: startT.Add(10 * time.Minute),
step: time.Minute,
},
}

storage := promqltest.LoadedStorage(t, data)
Expand Down
27 changes: 25 additions & 2 deletions pkg/streamingpromql/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ func TestUnsupportedPromQLFeatures(t *testing.T) {
// The goal of this is not to list every conceivable expression that is unsupported, but to cover all the
// different cases and make sure we produce a reasonable error message when these cases are encountered.
unsupportedExpressions := map[string]string{
"metric{} or other_metric{}": "binary expression with many-to-many matching",
"metric{} or other_metric{}": "binary expression with 'or'",
"metric{} unless other_metric{}": "binary expression with 'unless'",
"metric{} + on() group_left() other_metric{}": "binary expression with many-to-one matching",
"metric{} + on() group_right() other_metric{}": "binary expression with one-to-many matching",
"topk(5, metric{})": "'topk' aggregation with parameter",
Expand Down Expand Up @@ -93,6 +94,7 @@ func TestUnsupportedPromQLFeaturesWithFeatureToggles(t *testing.T) {
requireQueryIsSupported(t, featureToggles, "metric{} + 1")
requireQueryIsSupported(t, featureToggles, "1 + metric{}")
requireQueryIsSupported(t, featureToggles, "2 + 1")
requireQueryIsSupported(t, featureToggles, "metric{} and other_metric{}")
})

t.Run("vector/scalar binary expressions with comparison operation", func(t *testing.T) {
Expand All @@ -109,6 +111,7 @@ func TestUnsupportedPromQLFeaturesWithFeatureToggles(t *testing.T) {
requireQueryIsSupported(t, featureToggles, "metric{} + 1")
requireQueryIsSupported(t, featureToggles, "1 + metric{}")
requireQueryIsSupported(t, featureToggles, "2 + 1")
requireQueryIsSupported(t, featureToggles, "metric{} and other_metric{}")
})

t.Run("scalar/scalar binary expressions with comparison operation", func(t *testing.T) {
Expand All @@ -125,6 +128,26 @@ func TestUnsupportedPromQLFeaturesWithFeatureToggles(t *testing.T) {
requireQueryIsSupported(t, featureToggles, "metric{} + 1")
requireQueryIsSupported(t, featureToggles, "1 + metric{}")
requireQueryIsSupported(t, featureToggles, "2 + 1")
requireQueryIsSupported(t, featureToggles, "metric{} and other_metric{}")
})

t.Run("binary expressions with logical operations", func(t *testing.T) {
featureToggles := EnableAllFeatures
featureToggles.EnableBinaryLogicalOperations = false

requireQueryIsUnsupported(t, featureToggles, "metric{} and other_metric{}", "binary expression with 'and'")
requireQueryIsUnsupported(t, featureToggles, "metric{} or other_metric{}", "binary expression with 'or'")
requireQueryIsUnsupported(t, featureToggles, "metric{} unless other_metric{}", "binary expression with 'unless'")

// Other operations should still be supported.
requireQueryIsSupported(t, featureToggles, "metric{} + other_metric{}")
requireQueryIsSupported(t, featureToggles, "metric{} + 1")
requireQueryIsSupported(t, featureToggles, "1 + metric{}")
requireQueryIsSupported(t, featureToggles, "2 + 1")
requireQueryIsSupported(t, featureToggles, "metric{} > other_metric{}")
requireQueryIsSupported(t, featureToggles, "metric{} > 1")
requireQueryIsSupported(t, featureToggles, "1 > metric{}")
requireQueryIsSupported(t, featureToggles, "2 > bool 1")
})

t.Run("scalars", func(t *testing.T) {
Expand Down Expand Up @@ -1880,7 +1903,7 @@ func TestCompareVariousMixedMetricsBinaryOperations(t *testing.T) {
expressions := []string{}

for _, labels := range labelCombinations {
for _, op := range []string{"+", "-", "*", "/"} {
for _, op := range []string{"+", "-", "*", "/", "and"} {
binaryExpr := fmt.Sprintf(`series{label="%s"}`, labels[0])
for _, label := range labels[1:] {
binaryExpr += fmt.Sprintf(` %s series{label="%s"}`, op, label)
Expand Down
Loading

0 comments on commit 665bb0f

Please sign in to comment.