From edc8851a2a39134230fa08d80180203f2b4dc85c Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Tue, 8 Oct 2019 14:36:52 -0400 Subject: [PATCH 01/97] ast mapping Signed-off-by: Owen Diehl --- pkg/querier/astmapper/astmapper.go | 65 ++++++ pkg/querier/astmapper/astmapper_test.go | 93 ++++++++ pkg/querier/astmapper/embedded.go | 47 ++++ pkg/querier/astmapper/embedded_test.go | 34 +++ pkg/querier/astmapper/parallel.go | 82 +++++++ pkg/querier/astmapper/parallel_test.go | 108 +++++++++ pkg/querier/astmapper/shard_summer.go | 247 +++++++++++++++++++++ pkg/querier/astmapper/shard_summer_test.go | 79 +++++++ 8 files changed, 755 insertions(+) create mode 100644 pkg/querier/astmapper/astmapper.go create mode 100644 pkg/querier/astmapper/astmapper_test.go create mode 100644 pkg/querier/astmapper/embedded.go create mode 100644 pkg/querier/astmapper/embedded_test.go create mode 100644 pkg/querier/astmapper/parallel.go create mode 100644 pkg/querier/astmapper/parallel_test.go create mode 100644 pkg/querier/astmapper/shard_summer.go create mode 100644 pkg/querier/astmapper/shard_summer_test.go diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go new file mode 100644 index 00000000000..3aabe346bf9 --- /dev/null +++ b/pkg/querier/astmapper/astmapper.go @@ -0,0 +1,65 @@ +package astmapper + +import ( + "github.com/pkg/errors" + "github.com/prometheus/prometheus/promql" +) + +// ASTMapper is the exported interface for mapping between multiple AST representations +type ASTMapper interface { + Map(node promql.Node) (promql.Node, error) +} + +type MapperFunc func(node promql.Node) (promql.Node, error) + +func (fn MapperFunc) Map(node promql.Node) (promql.Node, error) { + return fn(node) +} + +type MultiMapper struct { + mappers []ASTMapper +} + +func (m *MultiMapper) Map(node promql.Node) (promql.Node, error) { + var result promql.Node = node + var err error + + if len(m.mappers) == 0 { + return nil, errors.New("MultiMapper: No mappers registered") + } + + for _, x := range m.mappers { + result, err = x.Map(result) + if err != nil { + return nil, err + } + } + return result, nil + +} + +// since registered functions are applied in the order they're registered, it's advised to register them +// in decreasing priority and only operate on nodes that each function cares about, defaulting to CloneNode. +func (m *MultiMapper) Register(xs ...ASTMapper) { + m.mappers = append(m.mappers, xs...) +} + +func NewMultiMapper(xs ...ASTMapper) *MultiMapper { + m := &MultiMapper{} + m.Register(xs...) + return m +} + +// Transform runs a mapper against an AST, producing the new mapped AST +func Transform(m ASTMapper, n promql.Node) (promql.Node, error) { + cloned, err := CloneNode(n) + if err != nil { + return nil, err + } + return m.Map(cloned) +} + +// helper function to clone a node. +func CloneNode(node promql.Node) (promql.Node, error) { + return promql.ParseExpr(node.String()) +} diff --git a/pkg/querier/astmapper/astmapper_test.go b/pkg/querier/astmapper/astmapper_test.go new file mode 100644 index 00000000000..5afa2c3736b --- /dev/null +++ b/pkg/querier/astmapper/astmapper_test.go @@ -0,0 +1,93 @@ +package astmapper + +import ( + "fmt" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestCloneNode(t *testing.T) { + var testExpr = []struct { + input promql.Expr + expected promql.Expr + }{ + // simple unmodified case + { + &promql.BinaryExpr{promql.ItemADD, &promql.NumberLiteral{1}, &promql.NumberLiteral{1}, nil, false}, + &promql.BinaryExpr{promql.ItemADD, &promql.NumberLiteral{1}, &promql.NumberLiteral{1}, nil, false}, + }, + { + &promql.AggregateExpr{ + Op: promql.ItemSum, + Without: true, + Expr: &promql.VectorSelector{ + Name: "some_metric", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), + }, + }, + Grouping: []string{"foo"}, + }, + &promql.AggregateExpr{ + Op: promql.ItemSum, + Without: true, + Expr: &promql.VectorSelector{ + Name: "some_metric", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), + }, + }, + Grouping: []string{"foo"}, + }, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + res, err := CloneNode(c.input) + require.NoError(t, err) + require.Equal(t, c.expected, res) + }) + } +} + +func TestCloneNode_String(t *testing.T) { + var testExpr = []struct { + input string + expected string + }{ + { + `rate(http_requests_total{cluster="us-central1"}[1m])`, + `rate(http_requests_total{cluster="us-central1"}[1m])`, + }, + { + `sum( +sum(rate(http_requests_total{cluster="us-central1"}[1m])) +/ +sum(rate(http_requests_total{cluster="ops-tools1"}[1m])) +)`, + `sum(sum(rate(http_requests_total{cluster="us-central1"}[1m])) / sum(rate(http_requests_total{cluster="ops-tools1"}[1m])))`, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + expr, err := promql.ParseExpr(c.input) + require.Nil(t, err) + res, err := CloneNode(expr) + require.Nil(t, err) + require.Equal(t, c.expected, res.String()) + }) + } +} + +func mustLabelMatcher(mt labels.MatchType, name, val string) *labels.Matcher { + m, err := labels.NewMatcher(mt, name, val) + if err != nil { + panic(err) + } + return m +} diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go new file mode 100644 index 00000000000..6268d349f59 --- /dev/null +++ b/pkg/querier/astmapper/embedded.go @@ -0,0 +1,47 @@ +package astmapper + +import ( + "encoding/hex" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "time" +) + +/* +Design: + +Unfortunately. the prometheus api package enforces a (*promql.Engine argument), making it infeasible to do lazy AST +evaluation and substitution from within this package. +This leaves the (storage.Queryable) interface as the remaining target for conducting application level sharding. + +The main idea is to analyze the AST and determine which subtrees can be parallelized. With those in hand, the queries may +be remapped into vector or matrix selectors utilizing a reserved label containing the original query. + +These may then be parallelized in the storage impl. + +Ideally the promql.Engine could be an interface instead of a concrete type, allowing us to conduct all parallelism from within the AST via the Engine and pass retrieval requests to the storage.Queryable ifc. +*/ + +const ( + QUERY_LABEL = "__cortex_query__" + EMBEDDED_QUERY_FLAG = "__embedded_query__" +) + +// Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. The important part is that return types align. +// TODO(owen): handle inferring return types from different functions/operators +func Squash(node promql.Node) (promql.Expr, error) { + // promql's label charset is not a subset of promql's syntax charset. Therefor we use hex as an intermediary + encoded := hex.EncodeToString([]byte(node.String())) + + embedded_query, err := labels.NewMatcher(labels.MatchEqual, QUERY_LABEL, encoded) + + if err != nil { + return nil, err + } + + return &promql.MatrixSelector{ + Name: EMBEDDED_QUERY_FLAG, + Range: time.Minute, + LabelMatchers: []*labels.Matcher{embedded_query}, + }, nil +} diff --git a/pkg/querier/astmapper/embedded_test.go b/pkg/querier/astmapper/embedded_test.go new file mode 100644 index 00000000000..dc57e0cf228 --- /dev/null +++ b/pkg/querier/astmapper/embedded_test.go @@ -0,0 +1,34 @@ +package astmapper + +import ( + "fmt" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestSquash(t *testing.T) { + var testExpr = []struct { + shards int + input string + expected string + }{ + { + 3, + `sum by(foo) (rate(bar1{baz="blip"}[1m]))`, + `sum by(foo) (__embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22305f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m] or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22315f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m] or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22325f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m])`, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + summer := NewShardSummer(c.shards, Squash) + expr, err := promql.ParseExpr(c.input) + require.Nil(t, err) + res, err := summer.Map(expr) + require.Nil(t, err) + + require.Equal(t, c.expected, res.String()) + }) + } +} diff --git a/pkg/querier/astmapper/parallel.go b/pkg/querier/astmapper/parallel.go new file mode 100644 index 00000000000..b6e2874ca69 --- /dev/null +++ b/pkg/querier/astmapper/parallel.go @@ -0,0 +1,82 @@ +package astmapper + +import ( + "github.com/pkg/errors" + "github.com/prometheus/prometheus/promql" +) + +// matrixselector & vectorselector need to have shard annotations added +/* +Design: +1) annotate subtrees as parallelizable. A subtree is parallelizable if all of it's components are parallelizable +2) deal with splitting/stiching against queriers after mapping into the parallel-compatible AST +*/ + +// function to annotate subtrees as parallelizable. Inefficient, but illustrative. +func CanParallel(node promql.Node) bool { + switch n := node.(type) { + case nil: + // nil handles cases where we check optional fields that are not set + return true + + case promql.Expressions: + for _, e := range n { + if !CanParallel(e) { + return false + } + } + return true + + case *promql.AggregateExpr: + // currently we only support sum for expediency. + if n.Op != promql.ItemSum || !CanParallel(n.Param) || !CanParallel(n.Expr) { + return false + } else { + return true + } + + case *promql.BinaryExpr: + // since binary exprs use each side for mapping, they cannot be parallelized + return false + + case *promql.Call: + if !FuncParallel(n.Func) { + return false + } + + for _, e := range n.Args { + if !CanParallel(e) { + return false + } + } + + return true + + case *promql.SubqueryExpr: + return CanParallel(n.Expr) + + case *promql.ParenExpr: + return CanParallel(n.Expr) + + case *promql.UnaryExpr: + // Since these are only currently supported for Scalars, should be parallel-compatible + return true + + case *promql.EvalStmt: + // cant' find an example for this -- defaulting to using the subexpr + return CanParallel(n.Expr) + + case *promql.MatrixSelector, *promql.NumberLiteral, *promql.StringLiteral, *promql.VectorSelector: + return true + + default: + panic(errors.Errorf("CloneNode: unhandled node type %T", node)) + } + + return false +} + +func FuncParallel(f *promql.Function) bool { + // flagging all functions as parallel for expediency -- this is certainly not correct (i.e. avg). + return true +} diff --git a/pkg/querier/astmapper/parallel_test.go b/pkg/querier/astmapper/parallel_test.go new file mode 100644 index 00000000000..dc499fe1771 --- /dev/null +++ b/pkg/querier/astmapper/parallel_test.go @@ -0,0 +1,108 @@ +package astmapper + +import ( + "fmt" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestCanParallel(t *testing.T) { + var testExpr = []struct { + input promql.Expr + expected bool + }{ + // simple sum + { + &promql.AggregateExpr{ + Op: promql.ItemSum, + Without: true, + Expr: &promql.VectorSelector{ + Name: "some_metric", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), + }, + }, + Grouping: []string{"foo"}, + }, + true, + }, + /* + sum( + sum by (foo) bar1{baz=”blip”}[1m]) + / + sum by (foo) bar2{baz=”blip”}[1m])) + ) + */ + { + &promql.AggregateExpr{ + Op: promql.ItemSum, + Expr: &promql.BinaryExpr{ + Op: promql.ItemDIV, + LHS: &promql.AggregateExpr{ + Op: promql.ItemSum, + Grouping: []string{"foo"}, + Expr: &promql.VectorSelector{ + Name: "idk", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "bar1"), + }}, + }, + RHS: &promql.AggregateExpr{ + Op: promql.ItemSum, + Grouping: []string{"foo"}, + Expr: &promql.VectorSelector{ + Name: "idk", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "bar2"), + }}, + }, + }, + }, + false, + }, + // sum by (foo) bar1{baz=”blip”}[1m]) ---- this is the first leg of the above + { + &promql.AggregateExpr{ + Op: promql.ItemSum, + Grouping: []string{"foo"}, + Expr: &promql.VectorSelector{ + Name: "idk", + LabelMatchers: []*labels.Matcher{ + mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "bar1"), + }}, + }, + true, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + res := CanParallel(c.input) + require.Equal(t, c.expected, res) + }) + } +} + +func TestCanParallel_String(t *testing.T) { + var testExpr = []struct { + input string + expected bool + }{ + { + `sum by (foo) (rate(bar1{baz="blip"}[1m]))`, + true, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + expr, err := promql.ParseExpr(c.input) + require.Nil(t, err) + res := CanParallel(expr) + require.Equal(t, c.expected, res) + }) + } +} diff --git a/pkg/querier/astmapper/shard_summer.go b/pkg/querier/astmapper/shard_summer.go new file mode 100644 index 00000000000..a78a8515001 --- /dev/null +++ b/pkg/querier/astmapper/shard_summer.go @@ -0,0 +1,247 @@ +package astmapper + +import ( + "fmt" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" +) + +const ( + DEFAULT_SHARDS = 12 + SHARD_LABEL = "__cortex_shard__" +) + +type squasher = func(promql.Node) (promql.Expr, error) + +type ShardSummer struct { + shards int + squash squasher +} + +func NewShardSummer(shards int, squasher func(promql.Node) (promql.Expr, error)) *ShardSummer { + if shards == 0 { + shards = DEFAULT_SHARDS + } + + return &ShardSummer{shards, squasher} +} + +// a MapperFunc adapter +func ShardSummerFunc(shards int, squash squasher) MapperFunc { + summer := NewShardSummer(shards, squash) + + return summer.Map +} + +// ShardSummer expands a query AST by sharding and re-summing when possible +func (summer *ShardSummer) Map(node promql.Node) (promql.Node, error) { + return summer.mapWithOpts(MappingOpts{}, node) +} + +type MappingOpts struct { + isParallel bool + // curShard's ptr denotes existence. This helps prevent selectors in non-sum queries from sharding themselves as they'll always register true for isParallel. + curShard *int +} + +// mapWithOpts carries information about whether the node is in a subtree that is a parallelism candidate. +func (summer *ShardSummer) mapWithOpts(opts MappingOpts, node promql.Node) (promql.Node, error) { + // since mapWithOpts is called recursively, the new subtree may be parallelizable even if its parent is not + opts.isParallel = opts.isParallel || CanParallel(node) + + switch n := node.(type) { + case promql.Expressions: + for i, e := range n { + if mapped, err := summer.mapWithOpts(opts, e); err != nil { + return nil, err + } else { + n[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.AggregateExpr: + if opts.isParallel { + return summer.shardSum(n) + } + + if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.BinaryExpr: + if lhs, err := summer.mapWithOpts(opts, n.LHS); err != nil { + return nil, err + } else { + n.LHS = lhs.(promql.Expr) + } + + if rhs, err := summer.mapWithOpts(opts, n.RHS); err != nil { + return nil, err + } else { + n.RHS = rhs.(promql.Expr) + } + return n, nil + + case *promql.Call: + for i, e := range n.Args { + if mapped, err := summer.mapWithOpts(opts, e); err != nil { + return nil, err + } else { + n.Args[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.SubqueryExpr: + if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.ParenExpr: + if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.UnaryExpr: + if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.EvalStmt: + if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.NumberLiteral, *promql.StringLiteral: + return n, nil + + case *promql.VectorSelector: + if opts.isParallel && opts.curShard != nil { + return shardVectorSelector(*opts.curShard, summer.shards, n) + } else { + return n, nil + } + + case *promql.MatrixSelector: + if opts.isParallel && opts.curShard != nil { + return shardMatrixSelector(*opts.curShard, summer.shards, n) + } else { + return n, nil + } + + default: + panic(errors.Errorf("ShardSummer: unhandled node type %T", node)) + } +} + +func (summer *ShardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, error) { + + if summer.shards < 2 { + return expr, nil + } + + subSums := make([]promql.Expr, 0, summer.shards) + + for i := 0; i < summer.shards; i++ { + cloned, err := CloneNode(expr.Expr) + if err != nil { + return nil, err + } + + sharded, err := summer.mapWithOpts(MappingOpts{ + curShard: &i, + }, cloned) + if err != nil { + return nil, err + } + + var subSum promql.Expr = &promql.AggregateExpr{ + Op: expr.Op, + Expr: sharded.(promql.Expr), + Param: expr.Param, + Grouping: expr.Grouping, + Without: expr.Without, + } + + if summer.squash != nil { + subSum, err = Squash(subSum) + } + + if err != nil { + return nil, err + } + + subSums = append(subSums, + subSum, + ) + + } + + var combinedSums promql.Expr = subSums[0] + for i := 1; i < len(subSums); i++ { + combinedSums = &promql.BinaryExpr{ + Op: promql.ItemLOR, + LHS: combinedSums, + RHS: subSums[i], + } + } + + return &promql.AggregateExpr{ + Op: expr.Op, + Expr: combinedSums, + Param: expr.Param, + Grouping: expr.Grouping, + Without: expr.Without, + }, + nil +} + +func shardVectorSelector(curshard, shards int, selector *promql.VectorSelector) (promql.Node, error) { + shardMatcher, err := labels.NewMatcher(labels.MatchEqual, SHARD_LABEL, fmt.Sprintf("%d_of_%d", curshard, shards)) + if err != nil { + return nil, err + } + + return &promql.VectorSelector{ + Name: selector.Name, + Offset: selector.Offset, + LabelMatchers: append( + []*labels.Matcher{shardMatcher}, + selector.LabelMatchers..., + ), + }, nil +} + +func shardMatrixSelector(curshard, shards int, selector *promql.MatrixSelector) (promql.Node, error) { + shardMatcher, err := labels.NewMatcher(labels.MatchEqual, SHARD_LABEL, fmt.Sprintf("%d_of_%d", curshard, shards)) + if err != nil { + return nil, err + } + + return &promql.MatrixSelector{ + Name: selector.Name, + Range: selector.Range, + Offset: selector.Offset, + LabelMatchers: append( + []*labels.Matcher{shardMatcher}, + selector.LabelMatchers..., + ), + }, nil +} diff --git a/pkg/querier/astmapper/shard_summer_test.go b/pkg/querier/astmapper/shard_summer_test.go new file mode 100644 index 00000000000..86c92ba4d30 --- /dev/null +++ b/pkg/querier/astmapper/shard_summer_test.go @@ -0,0 +1,79 @@ +package astmapper + +import ( + "fmt" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestShardSummer(t *testing.T) { + var testExpr = []struct { + shards int + input string + expected string + }{ + { + 3, + `sum by(foo) (rate(bar1{baz="blip"}[1m]))`, + `sum by(foo) ( + sum by(foo) (rate(bar1{__cortex_shard__="0_of_3",baz="blip"}[1m])) or + sum by(foo) (rate(bar1{__cortex_shard__="1_of_3",baz="blip"}[1m])) or + sum by(foo) (rate(bar1{__cortex_shard__="2_of_3",baz="blip"}[1m])) + )`, + }, + { + 2, + `sum( + sum by (foo) (rate(bar1{baz="blip"}[1m])) + / + sum by (foo) (rate(foo{baz="blip"}[1m])) + )`, + `sum( + sum by(foo) ( + sum by(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum by(foo) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) + ) + / + sum by(foo) ( + sum by(foo) (rate(foo{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum by(foo) (rate(foo{__cortex_shard__="1_of_2",baz="blip"}[1m])) + ) + )`, + }, + // This is currently redundant: sums split into sharded versions, including summed sums. + { + 2, + `sum(sum by(foo) (rate(bar1{baz="blip"}[1m])))`, + `sum( + sum( + sum by(foo) ( + sum by(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum by(foo) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) + ) + ) or + sum( + sum by(foo) ( + sum by(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum by(foo) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) + ) + ) + )`, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + summer := NewShardSummer(c.shards, nil) + expr, err := promql.ParseExpr(c.input) + require.Nil(t, err) + res, err := summer.Map(expr) + require.Nil(t, err) + + expected, err := promql.ParseExpr(c.expected) + require.Nil(t, err) + + require.Equal(t, expected.String(), res.String()) + }) + } +} From cecd6ed185230e30556347dbc651db6d3c88f5d5 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Tue, 8 Oct 2019 14:37:38 -0400 Subject: [PATCH 02/97] wrapper types for querier roundtripping and in memory series set Signed-off-by: Owen Diehl --- pkg/querier/querysharding/middleware.go | 3 + pkg/querier/querysharding/queryable.go | 77 ++++++++++++ pkg/querier/querysharding/series.go | 153 ++++++++++++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 pkg/querier/querysharding/middleware.go create mode 100644 pkg/querier/querysharding/queryable.go create mode 100644 pkg/querier/querysharding/series.go diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go new file mode 100644 index 00000000000..7dae2b7af50 --- /dev/null +++ b/pkg/querier/querysharding/middleware.go @@ -0,0 +1,3 @@ +package querysharding + +import () diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go new file mode 100644 index 00000000000..26eb91014d7 --- /dev/null +++ b/pkg/querier/querysharding/queryable.go @@ -0,0 +1,77 @@ +package querysharding + +import ( + "context" + "encoding/hex" + "github.com/cortexproject/cortex/pkg/querier/astmapper" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/storage" +) + +// DownstreamQueryable is a wrapper for and implementor of the Queryable interface. +type DownstreamQueryable struct { + storage.Queryable + req *queryrange.Request + handler queryrange.Handler +} + +func (q *DownstreamQueryable) Querier(ctx context.Context, mint, maxt int64) (storage.Querier, error) { + querier, err := q.Querier(ctx, mint, maxt) + if err != nil { + return nil, err + } + + return &downstreamQuerier{querier, ctx, q.handler, mint, maxt, q.req}, nil +} + +// downstreamQuerier is a wrapper and implementor of the Querier interface +type downstreamQuerier struct { + storage.Querier + ctx context.Context + handler queryrange.Handler + mint, maxt int64 + req *queryrange.Request +} + +// Select returns a set of series that matches the given label matchers. +func (q *downstreamQuerier) Select(sp *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { + for _, matcher := range matchers { + if matcher.Name == astmapper.EMBEDDED_QUERY_FLAG { + // this is an embedded query + return q.handleEmbeddedQuery(matcher.Value) + } + } + + // otherwise pass through to wrapped querier + // TODO: do we want to ship non-embedded selects downstream? + return q.Querier.Select(sp, matchers...) +} + +// handleEmbeddedQuery defers execution of an encoded query to a downstream handler +func (q *downstreamQuerier) handleEmbeddedQuery(encoded string) (storage.SeriesSet, storage.Warnings, error) { + decoded, err := hex.DecodeString(encoded) + if err != nil { + return nil, nil, err + } + + resp, err := q.handler.Do(q.ctx, ReplaceQuery(*q.req, string(decoded))) + if err != nil { + return nil, nil, err + } + + if resp.Error != "" { + return nil, nil, errors.Errorf(resp.Error) + } + + set, err := ResponseToSeries(resp.Data) + return set, nil, err + +} + +// take advantage of pass by value to clone a request with a new query +func ReplaceQuery(req queryrange.Request, query string) *queryrange.Request { + req.Query = query + return &req +} diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go new file mode 100644 index 00000000000..39df6976145 --- /dev/null +++ b/pkg/querier/querysharding/series.go @@ -0,0 +1,153 @@ +package querysharding + +import ( + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/storage" +) + +// needed to map back from api response to the underlying series data +func ResponseToSeries(resp queryrange.Response) (storage.SeriesSet, error) { + switch resp.ResultType { + case promql.ValueTypeVector, promql.ValueTypeMatrix: + return NewSeriesSet(resp.Result), nil + } + + return nil, errors.Errorf( + "Invalid promql.Value type: [%s]. Only %s and %s supported", + resp.ResultType, + promql.ValueTypeVector, + promql.ValueTypeMatrix, + ) +} + +func NewSeriesSet(results []queryrange.SampleStream) *downstreamSeriesSet { + set := make([]*downstreamSeries, 0, len(results)) + for _, srcSeries := range results { + series := &downstreamSeries{ + metric: make([]labels.Label, 0, len(srcSeries.Labels)), + points: make([]promql.Point, 0, len(srcSeries.Samples)), + } + + for _, l := range srcSeries.Labels { + series.metric = append(series.metric, labels.Label(l)) + } + + for _, pt := range srcSeries.Samples { + series.points = append(series.points, promql.Point{ + T: pt.TimestampMs, + V: pt.Value, + }) + } + + set = append(set, series) + } + + return &downstreamSeriesSet{ + set: set, + } +} + +// downstreamSeriesSet is an in-memory series that's mapped from a promql.Value (vector or matrix) +type downstreamSeriesSet struct { + i int + set []*downstreamSeries +} + +// impls storage.SeriesSet +func (set *downstreamSeriesSet) Next() bool { + set.i++ + if set.i >= len(set.set) { + return false + } + + return true +} + +// impls storage.SeriesSet +func (set *downstreamSeriesSet) At() storage.Series { + return set.set[set.i] +} + +// impls storage.SeriesSet +func (set *downstreamSeriesSet) Err() error { + if set.i >= len(set.set) { + return errors.Errorf("downStreamSeriesSet out of bounds: cannot request series %d of %d", set.i, len(set.set)) + } + return nil +} + +type downstreamSeries struct { + metric labels.Labels + i int + points []promql.Point +} + +// impls storage.Series +// Labels returns the complete set of labels identifying the series. +func (series *downstreamSeries) Labels() labels.Labels { + return series.metric +} + +// impls storage.Series +// Iterator returns a new iterator of the data of the series. +func (series *downstreamSeries) Iterator() storage.SeriesIterator { + // TODO(owen): unsure if this method should return a new iterator re-indexed to 0 or if it can + // be a passthrough method. Opting for the former for safety (although it contains the same slice). + return &downstreamSeries{ + metric: series.metric, + points: series.points, + } +} + +// impls storage.SeriesIterator +// Seek advances the iterator forward to the value at or after +// the given timestamp. +func (series *downstreamSeries) Seek(t int64) bool { + inBounds := func(i int) bool { + return i < len(series.points) + } + + // zero length series always returns false + if !inBounds(series.i) { + return false + } + + for i := 0; inBounds(i); i++ { + if series.points[i].T >= t { + series.i = i + return true + } + } + + return false +} + +// impls storage.SeriesIterator +// At returns the current timestamp/value pair. +func (series *downstreamSeries) At() (t int64, v float64) { + pt := series.points[series.i] + return pt.T, pt.V +} + +// impls storage.SeriesIterator +// Next advances the iterator by one. +func (series *downstreamSeries) Next() bool { + series.i++ + if series.i >= len(series.points) { + return false + } + return true +} + +// impls storage.SeriesIterator +// Err returns the current error. +func (series *downstreamSeries) Err() error { + if series.i >= len(series.points) { + return errors.Errorf("downstreamSeries out of bounds: cannot request point %d of %d", series.i, len(series.points)) + } + return nil + +} From 42c164003de47f2d3a6f9a1d3c78797d6a96f8a8 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 9 Oct 2019 10:45:04 -0400 Subject: [PATCH 03/97] query sharding middleware Signed-off-by: Owen Diehl --- pkg/querier/queryrange/query_range.go | 5 +- pkg/querier/queryrange/query_range_test.go | 36 +++++++++++ pkg/querier/querysharding/middleware.go | 74 +++++++++++++++++++++- pkg/querier/querysharding/queryable.go | 55 +++++++++------- pkg/querier/querysharding/value.go | 71 +++++++++++++++++++++ 5 files changed, 216 insertions(+), 25 deletions(-) create mode 100644 pkg/querier/querysharding/value.go diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index e1691c74033..e0ecb705454 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -22,7 +22,10 @@ import ( "github.com/cortexproject/cortex/pkg/ingester/client" ) -const statusSuccess = "success" +const ( + StatusSuccess = "success" + StatusFailure = "failure" +) var ( matrix = model.ValMatrix.String() diff --git a/pkg/querier/queryrange/query_range_test.go b/pkg/querier/queryrange/query_range_test.go index f9649d27907..7f21b94223e 100644 --- a/pkg/querier/queryrange/query_range_test.go +++ b/pkg/querier/queryrange/query_range_test.go @@ -113,9 +113,15 @@ func TestMergeAPIResponses(t *testing.T) { }{ // No responses shouldn't panic. { +<<<<<<< HEAD input: []Response{}, expected: &PrometheusResponse{ Status: statusSuccess, +======= + input: []*APIResponse{}, + expected: &APIResponse{ + Status: StatusSuccess, +>>>>>>> query sharding middleware }, }, @@ -129,9 +135,15 @@ func TestMergeAPIResponses(t *testing.T) { }, }, }, +<<<<<<< HEAD expected: &PrometheusResponse{ Status: statusSuccess, Data: PrometheusData{ +======= + expected: &APIResponse{ + Status: StatusSuccess, + Data: Response{ +>>>>>>> query sharding middleware ResultType: matrix, Result: []SampleStream{}, }, @@ -154,9 +166,15 @@ func TestMergeAPIResponses(t *testing.T) { }, }, }, +<<<<<<< HEAD expected: &PrometheusResponse{ Status: statusSuccess, Data: PrometheusData{ +======= + expected: &APIResponse{ + Status: StatusSuccess, + Data: Response{ +>>>>>>> query sharding middleware ResultType: matrix, Result: []SampleStream{}, }, @@ -195,9 +213,15 @@ func TestMergeAPIResponses(t *testing.T) { }, }, }, +<<<<<<< HEAD expected: &PrometheusResponse{ Status: statusSuccess, Data: PrometheusData{ +======= + expected: &APIResponse{ + Status: StatusSuccess, + Data: Response{ +>>>>>>> query sharding middleware ResultType: matrix, Result: []SampleStream{ { @@ -220,9 +244,15 @@ func TestMergeAPIResponses(t *testing.T) { mustParse(t, `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"a":"b","c":"d"},"values":[[0,"0"],[1,"1"]]}]}}`), mustParse(t, `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"c":"d","a":"b"},"values":[[2,"2"],[3,"3"]]}]}}`), }, +<<<<<<< HEAD expected: &PrometheusResponse{ Status: statusSuccess, Data: PrometheusData{ +======= + expected: &APIResponse{ + Status: StatusSuccess, + Data: Response{ +>>>>>>> query sharding middleware ResultType: matrix, Result: []SampleStream{ { @@ -244,9 +274,15 @@ func TestMergeAPIResponses(t *testing.T) { mustParse(t, `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"a":"b","c":"d"},"values":[[1,"1"],[2,"2"]]}]}}`), mustParse(t, `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"c":"d","a":"b"},"values":[[2,"2"],[3,"3"]]}]}}`), }, +<<<<<<< HEAD expected: &PrometheusResponse{ Status: statusSuccess, Data: PrometheusData{ +======= + expected: &APIResponse{ + Status: StatusSuccess, + Data: Response{ +>>>>>>> query sharding middleware ResultType: matrix, Result: []SampleStream{ { diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 7dae2b7af50..26df03e9114 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -1,3 +1,75 @@ package querysharding -import () +import ( + "context" + "github.com/cortexproject/cortex/pkg/querier/astmapper" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/prometheus/prometheus/promql" + "time" +) + +const ( + downStreamErrType = "downstream error" + parseErrType = "parse error" +) + +func QueryShardMiddleware() queryrange.Middleware { + return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { + return &queryShard{ + next: next, + mapper: astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.Squash), + } + }) +} + +type queryShard struct { + next queryrange.Handler + engine *promql.Engine + mapper astmapper.ASTMapper +} + +func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrange.APIResponse, error) { + queryable := &DownstreamQueryable{r, qs.next} + + qry, err := qs.engine.NewRangeQuery( + queryable, + r.Query, + time.Unix(r.Start, 0), time.Unix(r.End, 0), + time.Duration(r.Step)*time.Second, + ) + + if err != nil { + return &queryrange.APIResponse{ + Status: queryrange.StatusFailure, + ErrorType: parseErrType, + Error: err.Error(), + }, nil + } + + res := qry.Exec(ctx) + + if res.Err != nil { + return &queryrange.APIResponse{ + Status: queryrange.StatusFailure, + ErrorType: downStreamErrType, + Error: res.Err.Error(), + }, nil + } + + if extracted, err := FromValue(res.Value); err != nil { + return &queryrange.APIResponse{ + Status: queryrange.StatusFailure, + ErrorType: downStreamErrType, + Error: err.Error(), + }, nil + + } else { + return &queryrange.APIResponse{ + Status: queryrange.StatusSuccess, + Data: queryrange.Response{ + ResultType: string(res.Value.Type()), + Result: extracted, + }, + }, nil + } +} diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go index 26eb91014d7..8529c2f140e 100644 --- a/pkg/querier/querysharding/queryable.go +++ b/pkg/querier/querysharding/queryable.go @@ -12,31 +12,26 @@ import ( // DownstreamQueryable is a wrapper for and implementor of the Queryable interface. type DownstreamQueryable struct { - storage.Queryable - req *queryrange.Request - handler queryrange.Handler + Req *queryrange.Request + Handler queryrange.Handler } func (q *DownstreamQueryable) Querier(ctx context.Context, mint, maxt int64) (storage.Querier, error) { - querier, err := q.Querier(ctx, mint, maxt) - if err != nil { - return nil, err - } - - return &downstreamQuerier{querier, ctx, q.handler, mint, maxt, q.req}, nil + return &DownstreamQuerier{ctx, q.Req, q.Handler}, nil } -// downstreamQuerier is a wrapper and implementor of the Querier interface -type downstreamQuerier struct { - storage.Querier - ctx context.Context - handler queryrange.Handler - mint, maxt int64 - req *queryrange.Request +// DownstreamQueryable is a an implementor of the Queryable interface. +type DownstreamQuerier struct { + Ctx context.Context + Req *queryrange.Request + Handler queryrange.Handler } // Select returns a set of series that matches the given label matchers. -func (q *downstreamQuerier) Select(sp *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { +func (q *DownstreamQuerier) Select( + sp *storage.SelectParams, + matchers ...*labels.Matcher, +) (storage.SeriesSet, storage.Warnings, error) { for _, matcher := range matchers { if matcher.Name == astmapper.EMBEDDED_QUERY_FLAG { // this is an embedded query @@ -44,19 +39,17 @@ func (q *downstreamQuerier) Select(sp *storage.SelectParams, matchers ...*labels } } - // otherwise pass through to wrapped querier - // TODO: do we want to ship non-embedded selects downstream? - return q.Querier.Select(sp, matchers...) + return nil, nil, errors.Errorf("DownstreamQuerier cannot handle a non-embedded query") } -// handleEmbeddedQuery defers execution of an encoded query to a downstream handler -func (q *downstreamQuerier) handleEmbeddedQuery(encoded string) (storage.SeriesSet, storage.Warnings, error) { +// handleEmbeddedQuery defers execution of an encoded query to a downstream Handler +func (q *DownstreamQuerier) handleEmbeddedQuery(encoded string) (storage.SeriesSet, storage.Warnings, error) { decoded, err := hex.DecodeString(encoded) if err != nil { return nil, nil, err } - resp, err := q.handler.Do(q.ctx, ReplaceQuery(*q.req, string(decoded))) + resp, err := q.Handler.Do(q.Ctx, ReplaceQuery(*q.Req, string(decoded))) if err != nil { return nil, nil, err } @@ -70,6 +63,22 @@ func (q *downstreamQuerier) handleEmbeddedQuery(encoded string) (storage.SeriesS } +// other storage.Querier impls that are not used by engine +// LabelValues returns all potential values for a label name. +func (q *DownstreamQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { + return nil, nil, errors.Errorf("unimplemented") +} + +// LabelNames returns all the unique label names present in the block in sorted order. +func (q *DownstreamQuerier) LabelNames() ([]string, storage.Warnings, error) { + return nil, nil, errors.Errorf("unimplemented") +} + +// Close releases the resources of the Querier. +func (q *DownstreamQuerier) Close() error { + return nil +} + // take advantage of pass by value to clone a request with a new query func ReplaceQuery(req queryrange.Request, query string) *queryrange.Request { req.Query = query diff --git a/pkg/querier/querysharding/value.go b/pkg/querier/querysharding/value.go new file mode 100644 index 00000000000..84a09989f89 --- /dev/null +++ b/pkg/querier/querysharding/value.go @@ -0,0 +1,71 @@ +package querysharding + +import ( + "github.com/cortexproject/cortex/pkg/ingester/client" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" +) + +// FromValue transforms a promql query result into a samplestream +func FromValue(val promql.Value) ([]queryrange.SampleStream, error) { + switch v := val.(type) { + case promql.Scalar: + return []queryrange.SampleStream{ + queryrange.SampleStream{ + Samples: []client.Sample{ + client.Sample{ + Value: v.V, + TimestampMs: v.T, + }, + }, + }, + }, nil + + case promql.Vector: + res := make([]queryrange.SampleStream, 0, len(v)) + for _, sample := range v { + res = append(res, queryrange.SampleStream{ + Labels: mapLabels(sample.Metric), + Samples: mapPoints(sample.Point), + }) + } + return res, nil + + case promql.Matrix: + res := make([]queryrange.SampleStream, 0, len(v)) + for _, series := range v { + res = append(res, queryrange.SampleStream{ + Labels: mapLabels(series.Metric), + Samples: mapPoints(series.Points...), + }) + } + return res, nil + + } + + return nil, errors.Errorf("Unexpected value type: [%s]", val.Type()) +} + +func mapLabels(ls labels.Labels) []client.LabelAdapter { + result := make([]client.LabelAdapter, 0, len(ls)) + for _, l := range ls { + result = append(result, client.LabelAdapter(l)) + } + + return result +} + +func mapPoints(pts ...promql.Point) []client.Sample { + result := make([]client.Sample, 0, len(pts)) + + for _, pt := range pts { + result = append(result, client.Sample{ + Value: pt.V, + TimestampMs: pt.T, + }) + } + + return result +} From f9bdf074fa73f5a1f538602c1d6662c56051877a Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 9 Oct 2019 12:37:31 -0400 Subject: [PATCH 04/97] embeds all selectors for distribution to queriers Signed-off-by: Owen Diehl --- pkg/querier/astmapper/astmapper.go | 9 -- pkg/querier/astmapper/embedded.go | 129 ++++++++++++++++++++++-- pkg/querier/astmapper/embedded_test.go | 37 ++++++- pkg/querier/astmapper/parallel.go | 3 +- pkg/querier/astmapper/shard_summer.go | 2 +- pkg/querier/querysharding/middleware.go | 7 +- 6 files changed, 165 insertions(+), 22 deletions(-) diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go index 3aabe346bf9..38444264c05 100644 --- a/pkg/querier/astmapper/astmapper.go +++ b/pkg/querier/astmapper/astmapper.go @@ -50,15 +50,6 @@ func NewMultiMapper(xs ...ASTMapper) *MultiMapper { return m } -// Transform runs a mapper against an AST, producing the new mapped AST -func Transform(m ASTMapper, n promql.Node) (promql.Node, error) { - cloned, err := CloneNode(n) - if err != nil { - return nil, err - } - return m.Map(cloned) -} - // helper function to clone a node. func CloneNode(node promql.Node) (promql.Node, error) { return promql.ParseExpr(node.String()) diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go index 6268d349f59..956dab48b26 100644 --- a/pkg/querier/astmapper/embedded.go +++ b/pkg/querier/astmapper/embedded.go @@ -2,6 +2,7 @@ package astmapper import ( "encoding/hex" + "github.com/pkg/errors" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "time" @@ -29,7 +30,7 @@ const ( // Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. The important part is that return types align. // TODO(owen): handle inferring return types from different functions/operators -func Squash(node promql.Node) (promql.Expr, error) { +func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { // promql's label charset is not a subset of promql's syntax charset. Therefor we use hex as an intermediary encoded := hex.EncodeToString([]byte(node.String())) @@ -39,9 +40,125 @@ func Squash(node promql.Node) (promql.Expr, error) { return nil, err } - return &promql.MatrixSelector{ - Name: EMBEDDED_QUERY_FLAG, - Range: time.Minute, - LabelMatchers: []*labels.Matcher{embedded_query}, - }, nil + if isMatrix { + return &promql.MatrixSelector{ + Name: EMBEDDED_QUERY_FLAG, + Range: time.Minute, + LabelMatchers: []*labels.Matcher{embedded_query}, + }, nil + } else { + return &promql.VectorSelector{ + Name: EMBEDDED_QUERY_FLAG, + LabelMatchers: []*labels.Matcher{embedded_query}, + }, nil + } +} + +// VectorSquasher always uses a VectorSelector as the substitution node. +// This is important because logical/set binops can only be applied against vectors and not matrices. +func VectorSquasher(node promql.Node) (promql.Expr, error) { + return Squash(node, false) +} + +// ShallowEmbedSelectors encodes selector queries if they do not already have the EMBEDDED_QUERY_FLAG. +// This is primarily useful for deferring query execution. +func ShallowEmbedSelectors(node promql.Node) (promql.Node, error) { + + switch n := node.(type) { + case nil: + // nil handles cases where we check optional fields that are not set + return nil, nil + + case promql.Expressions: + for i, e := range n { + if mapped, err := ShallowEmbedSelectors(e); err != nil { + return nil, err + } else { + n[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.AggregateExpr: + expr, err := ShallowEmbedSelectors(n.Expr) + if err != nil { + return nil, err + } + n.Expr = expr.(promql.Expr) + return n, nil + + case *promql.BinaryExpr: + if lhs, err := ShallowEmbedSelectors(n.LHS); err != nil { + return nil, err + } else { + n.LHS = lhs.(promql.Expr) + } + + if rhs, err := ShallowEmbedSelectors(n.RHS); err != nil { + return nil, err + } else { + n.RHS = rhs.(promql.Expr) + } + return n, nil + + case *promql.Call: + for i, e := range n.Args { + if mapped, err := ShallowEmbedSelectors(e); err != nil { + return nil, err + } else { + n.Args[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.SubqueryExpr: + if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.ParenExpr: + if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.UnaryExpr: + if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.EvalStmt: + if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.NumberLiteral, *promql.StringLiteral: + return n, nil + + case *promql.VectorSelector: + if n.Name == EMBEDDED_QUERY_FLAG { + return n, nil + } + return Squash(n, false) + + case *promql.MatrixSelector: + if n.Name == EMBEDDED_QUERY_FLAG { + return n, nil + } + return Squash(n, true) + + default: + panic(errors.Errorf("ShallowEmbedSelectors: unhandled node type %T", node)) + } } diff --git a/pkg/querier/astmapper/embedded_test.go b/pkg/querier/astmapper/embedded_test.go index dc57e0cf228..04f94083259 100644 --- a/pkg/querier/astmapper/embedded_test.go +++ b/pkg/querier/astmapper/embedded_test.go @@ -16,13 +16,13 @@ func TestSquash(t *testing.T) { { 3, `sum by(foo) (rate(bar1{baz="blip"}[1m]))`, - `sum by(foo) (__embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22305f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m] or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22315f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m] or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22325f6f665f33222c62617a3d22626c6970227d5b316d5d2929"}[1m])`, + `sum by(foo) (__embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22305f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22315f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22325f6f665f33222c62617a3d22626c6970227d5b316d5d2929"})`, }, } for i, c := range testExpr { t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { - summer := NewShardSummer(c.shards, Squash) + summer := NewShardSummer(c.shards, VectorSquasher) expr, err := promql.ParseExpr(c.input) require.Nil(t, err) res, err := summer.Map(expr) @@ -32,3 +32,36 @@ func TestSquash(t *testing.T) { }) } } + +func TestShallowEmbedSelectors(t *testing.T) { + var testExpr = []struct { + input string + expected string + }{ + // already holds embedded query, so noop (don't double encode) + { + `sum by(foo) (__embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22305f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22315f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22325f6f665f33222c62617a3d22626c6970227d5b316d5d2929"})`, + `sum by(foo) (__embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22305f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22315f6f665f33222c62617a3d22626c6970227d5b316d5d2929"} or __embedded_query__{__cortex_query__="73756d20627928666f6f2920287261746528626172317b5f5f636f727465785f73686172645f5f3d22325f6f665f33222c62617a3d22626c6970227d5b316d5d2929"})`, + }, + { + `http_requests_total{cluster="prod"}`, + `__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2270726f64227d"}`, + }, + { + `rate(http_requests_total{cluster="eu-west2"}[5m]) or rate(http_requests_total{cluster="us-central1"}[5m])`, + `rate(__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2265752d7765737432227d5b356d5d"}[1m]) or rate(__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2275732d63656e7472616c31227d5b356d5d"}[1m])`, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + mapper := MapperFunc(ShallowEmbedSelectors) + expr, err := promql.ParseExpr(c.input) + require.Nil(t, err) + res, err := mapper.Map(expr) + require.Nil(t, err) + + require.Equal(t, c.expected, res.String()) + }) + } +} diff --git a/pkg/querier/astmapper/parallel.go b/pkg/querier/astmapper/parallel.go index b6e2874ca69..40d9abb35d3 100644 --- a/pkg/querier/astmapper/parallel.go +++ b/pkg/querier/astmapper/parallel.go @@ -36,7 +36,7 @@ func CanParallel(node promql.Node) bool { } case *promql.BinaryExpr: - // since binary exprs use each side for mapping, they cannot be parallelized + // since binary exprs use each side for merging, they cannot be parallelized return false case *promql.Call: @@ -63,7 +63,6 @@ func CanParallel(node promql.Node) bool { return true case *promql.EvalStmt: - // cant' find an example for this -- defaulting to using the subexpr return CanParallel(n.Expr) case *promql.MatrixSelector, *promql.NumberLiteral, *promql.StringLiteral, *promql.VectorSelector: diff --git a/pkg/querier/astmapper/shard_summer.go b/pkg/querier/astmapper/shard_summer.go index a78a8515001..5db48efc879 100644 --- a/pkg/querier/astmapper/shard_summer.go +++ b/pkg/querier/astmapper/shard_summer.go @@ -181,7 +181,7 @@ func (summer *ShardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, er } if summer.squash != nil { - subSum, err = Squash(subSum) + subSum, err = summer.squash(subSum) } if err != nil { diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 26df03e9114..24c5c4a21ab 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -16,8 +16,11 @@ const ( func QueryShardMiddleware() queryrange.Middleware { return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { return &queryShard{ - next: next, - mapper: astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.Squash), + next: next, + mapper: astmapper.NewMultiMapper( + astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.Squash), + astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), + ), } }) } From 8ac72050a3361ba566acb211668ae5688ccc57bb Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 9 Oct 2019 14:07:05 -0400 Subject: [PATCH 05/97] shardsummer in middleware uses vectorsquasher Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 24c5c4a21ab..ca31096f085 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -18,7 +18,7 @@ func QueryShardMiddleware() queryrange.Middleware { return &queryShard{ next: next, mapper: astmapper.NewMultiMapper( - astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.Squash), + astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.VectorSquasher), astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), ), } From 6eb66c16faf21bf3c9f018b0adcd2b5327d8a03f Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 9 Oct 2019 14:07:17 -0400 Subject: [PATCH 06/97] value conversion unit tests Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/value_test.go | 158 ++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 pkg/querier/querysharding/value_test.go diff --git a/pkg/querier/querysharding/value_test.go b/pkg/querier/querysharding/value_test.go new file mode 100644 index 00000000000..a7ad6c59ea5 --- /dev/null +++ b/pkg/querier/querysharding/value_test.go @@ -0,0 +1,158 @@ +package querysharding + +import ( + "fmt" + "github.com/cortexproject/cortex/pkg/ingester/client" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestFromValue(t *testing.T) { + var testExpr = []struct { + input promql.Value + err bool + expected []queryrange.SampleStream + }{ + // string (errors) + { + input: promql.String{1, "hi"}, + err: true, + }, + // Scalar + { + input: promql.Scalar{1, 1}, + err: false, + expected: []queryrange.SampleStream{ + { + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + }, + }, + }, + }, + // Vector + { + input: promql.Vector{ + promql.Sample{ + promql.Point{1, 1}, + labels.Labels{ + {"a", "a1"}, + {"b", "b1"}, + }, + }, + promql.Sample{ + promql.Point{2, 2}, + labels.Labels{ + {"a", "a2"}, + {"b", "b2"}, + }, + }, + }, + err: false, + expected: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + }, + }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 2, + TimestampMs: 2, + }, + }, + }, + }, + }, + // Matrix + { + input: promql.Matrix{ + { + Metric: labels.Labels{ + {"a", "a1"}, + {"b", "b1"}, + }, + Points: []promql.Point{ + {1, 1}, + {2, 2}, + }, + }, + { + Metric: labels.Labels{ + {"a", "a2"}, + {"b", "b2"}, + }, + Points: []promql.Point{ + {1, 8}, + {2, 9}, + }, + }, + }, + err: false, + expected: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + client.Sample{ + Value: 2, + TimestampMs: 2, + }, + }, + }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 8, + TimestampMs: 1, + }, + client.Sample{ + Value: 9, + TimestampMs: 2, + }, + }, + }, + }, + }, + } + + for i, c := range testExpr { + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + result, err := FromValue(c.input) + if c.err { + require.NotNil(t, err) + } else { + require.Nil(t, err) + require.Equal(t, c.expected, result) + } + }) + } +} From 49caa6a4629963a8a194c9c181b91c5bbeceae41 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 9 Oct 2019 17:38:54 -0400 Subject: [PATCH 07/97] downstreamSeries.Seek method & adds unit tests Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/series.go | 17 +- pkg/querier/querysharding/series_test.go | 350 +++++++++++++++++++++++ 2 files changed, 355 insertions(+), 12 deletions(-) create mode 100644 pkg/querier/querysharding/series_test.go diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go index 39df6976145..4ebe21a1dda 100644 --- a/pkg/querier/querysharding/series.go +++ b/pkg/querier/querysharding/series.go @@ -74,7 +74,7 @@ func (set *downstreamSeriesSet) At() storage.Series { // impls storage.SeriesSet func (set *downstreamSeriesSet) Err() error { if set.i >= len(set.set) { - return errors.Errorf("downStreamSeriesSet out of bounds: cannot request series %d of %d", set.i, len(set.set)) + return errors.Errorf("downStreamSeriesSet out of bounds: cannot request series index %d of length %d", set.i, len(set.set)) } return nil } @@ -106,20 +106,13 @@ func (series *downstreamSeries) Iterator() storage.SeriesIterator { // Seek advances the iterator forward to the value at or after // the given timestamp. func (series *downstreamSeries) Seek(t int64) bool { - inBounds := func(i int) bool { - return i < len(series.points) - } - - // zero length series always returns false - if !inBounds(series.i) { - return false - } - for i := 0; inBounds(i); i++ { - if series.points[i].T >= t { - series.i = i + for series.Err() == nil { + ts, _ := series.At() + if ts >= t { return true } + series.Next() } return false diff --git a/pkg/querier/querysharding/series_test.go b/pkg/querier/querysharding/series_test.go new file mode 100644 index 00000000000..3497885cf5f --- /dev/null +++ b/pkg/querier/querysharding/series_test.go @@ -0,0 +1,350 @@ +package querysharding + +import ( + "github.com/cortexproject/cortex/pkg/ingester/client" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestResponseToSeries(t *testing.T) { + var testExpr = []struct { + name string + input queryrange.Response + err bool + expected *downstreamSeriesSet + }{ + { + name: "vector coercion", + input: queryrange.Response{ + ResultType: promql.ValueTypeVector, + Result: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + }, + }, + }, + }, + err: false, + expected: &downstreamSeriesSet{ + set: []*downstreamSeries{ + { + metric: []labels.Label{ + {"a", "a1"}, + {"b", "b1"}, + }, + points: []promql.Point{ + {1, 1}, + }, + }, + }, + }, + }, + { + name: "matrix coercion", + input: queryrange.Response{ + ResultType: promql.ValueTypeMatrix, + Result: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + { + Value: 1, + TimestampMs: 1, + }, + { + Value: 2, + TimestampMs: 2, + }, + }, + }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + { + Value: 8, + TimestampMs: 1, + }, + { + Value: 9, + TimestampMs: 2, + }, + }, + }, + }, + }, + err: false, + expected: &downstreamSeriesSet{ + set: []*downstreamSeries{ + { + metric: labels.Labels{ + {"a", "a1"}, + {"b", "b1"}, + }, + points: []promql.Point{ + {1, 1}, + {2, 2}, + }, + }, + { + metric: labels.Labels{ + {"a", "a2"}, + {"b", "b2"}, + }, + points: []promql.Point{ + {1, 8}, + {2, 9}, + }, + }, + }, + }}, + { + name: "unknown type -- error converting", + input: queryrange.Response{ + ResultType: "unsupported", + Result: []queryrange.SampleStream{}, + }, + err: true, + expected: nil, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + result, err := ResponseToSeries(c.input) + if c.err { + require.NotNil(t, err) + } else { + require.Nil(t, err) + require.Equal(t, c.expected, result) + } + }) + } +} + +func Test_downstreamSeriesSet_impls_storage_SeriesSet(t *testing.T) { + set := &downstreamSeriesSet{ + // use nil for test expediency -- just checking that indices are maintained + set: []*downstreamSeries{ + &downstreamSeries{ + metric: labels.Labels{{"a", "a"}}, + }, + &downstreamSeries{ + metric: labels.Labels{{"b", "b"}}, + }, + &downstreamSeries{ + metric: labels.Labels{{"c", "c"}}, + }, + }, + } + + for i := 0; i < len(set.set)-1; i++ { + require.Nil(t, set.Err()) + require.Equal(t, set.At(), set.set[i]) + require.Equal(t, set.Next(), true) + } + + require.Equal(t, set.Next(), false) + require.NotNil(t, set.Err()) +} + +func Test_downstreamSeries_impls_storage_Series(t *testing.T) { + series := initSeries() + + // iterator is a passthrough (downstreamSeries impls both Series & SeriesIterator) + require.Equal(t, series, series.Iterator()) + + require.Equal(t, series.Labels(), series.metric) +} + +func Test_downstreamSeries_impls_storage_SeriesIterator_Err(t *testing.T) { + var testExpr = []struct { + name string + input *downstreamSeries + f func(*testing.T, *downstreamSeries) + }{ + { + "empty", + &downstreamSeries{}, + func(t *testing.T, series *downstreamSeries) { + require.NotNil(t, series.Err()) + }, + }, + { + "past bounds", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + //advance internal index to end of data + series.i = len(series.points) + + require.NotNil(t, series.Err()) + }, + }, + { + "past bounds via next", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + for i := 0; i < len(series.points); i++ { + series.Next() + } + + require.NotNil(t, series.Err()) + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + c.f(t, c.input) + }) + } +} + +func Test_downstreamSeries_impls_storage_SeriesIterator_Next(t *testing.T) { + var testExpr = []struct { + name string + input *downstreamSeries + f func(*testing.T, *downstreamSeries) + }{ + { + "next advances series", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + for i := 0; i < len(series.points)-1; i++ { + require.Equal(t, i, series.i) + require.Equal(t, true, series.Next()) + require.Equal(t, i+1, series.i) + } + }, + }, + { + "next returns false at end of series", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + for i := 0; i < len(series.points)-1; i++ { + series.Next() + } + + require.Equal(t, false, series.Next()) + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + c.f(t, c.input) + }) + } +} + +func Test_downstreamSeries_impls_storage_SeriesIterator_At(t *testing.T) { + var testExpr = []struct { + name string + input *downstreamSeries + f func(*testing.T, *downstreamSeries) + }{ + { + "At tracks current index", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + for i := 0; i < len(series.points); i++ { + timestamp, val := series.At() + require.Equal(t, series.points[i].T, timestamp) + require.Equal(t, series.points[i].V, val) + series.Next() + } + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + c.f(t, c.input) + }) + } +} + +func Test_downstreamSeries_impls_storage_SeriesIterator_Seek(t *testing.T) { + var testExpr = []struct { + name string + input *downstreamSeries + f func(*testing.T, *downstreamSeries) + }{ + { + "advances iterator at", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + require.Equal(t, true, series.Seek(2)) + ts, _ := series.At() + require.Equal(t, int64(2), ts) + }, + }, + { + "advances iterator after", + &downstreamSeries{ + metric: labels.Labels{{"a", "a"}}, + points: []promql.Point{ + {1, 1}, + {2, 2}, + {4, 4}, + }, + }, + func(t *testing.T, series *downstreamSeries) { + require.Equal(t, true, series.Seek(3)) + ts, _ := series.At() + require.Equal(t, int64(4), ts) + }, + }, + { + "doesnt unnecessarily advancer iter", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + require.Equal(t, true, series.Seek(2)) + require.Equal(t, true, series.Seek(2)) + ts, _ := series.At() + require.Equal(t, int64(2), ts) + }, + }, + { + "seek past end", + initSeries(), + func(t *testing.T, series *downstreamSeries) { + require.Equal(t, false, series.Seek(100)) + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + c.f(t, c.input) + }) + } +} + +// helper for generating testable series +func initSeries() *downstreamSeries { + return &downstreamSeries{ + metric: labels.Labels{{"a", "a"}}, + points: []promql.Point{ + {1, 1}, + {2, 2}, + {3, 3}, + }, + } +} From 43dbbc0e707c04ccbdbe5396509aa68550770638 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 11:22:08 -0400 Subject: [PATCH 08/97] corrects embedded queryable label matching/dispatching & adds unit tests Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware_test.go | 17 ++ pkg/querier/querysharding/queryable.go | 29 +++- pkg/querier/querysharding/queryable_test.go | 161 +++++++++++++++++++ 3 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 pkg/querier/querysharding/middleware_test.go create mode 100644 pkg/querier/querysharding/queryable_test.go diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go new file mode 100644 index 00000000000..703e5a485b0 --- /dev/null +++ b/pkg/querier/querysharding/middleware_test.go @@ -0,0 +1,17 @@ +package querysharding + +import ( + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "time" +) + +func defaultReq() *queryrange.Request { + return &queryrange.Request{ + Path: "/query_range", + Start: 10, + End: 20, + Step: 5, + Timeout: time.Minute, + Query: `__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2270726f64227d"}`, + } +} diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go index 8529c2f140e..b96ef02d13a 100644 --- a/pkg/querier/querysharding/queryable.go +++ b/pkg/querier/querysharding/queryable.go @@ -10,6 +10,11 @@ import ( "github.com/prometheus/prometheus/storage" ) +const ( + missingEmbeddedQueryMsg = "missing embedded query" + nonEmbeddedErrMsg = "DownstreamQuerier cannot handle a non-embedded query" +) + // DownstreamQueryable is a wrapper for and implementor of the Queryable interface. type DownstreamQueryable struct { Req *queryrange.Request @@ -29,17 +34,31 @@ type DownstreamQuerier struct { // Select returns a set of series that matches the given label matchers. func (q *DownstreamQuerier) Select( - sp *storage.SelectParams, + _ *storage.SelectParams, matchers ...*labels.Matcher, ) (storage.SeriesSet, storage.Warnings, error) { + var embeddedQuery string + var isEmbedded bool for _, matcher := range matchers { - if matcher.Name == astmapper.EMBEDDED_QUERY_FLAG { - // this is an embedded query - return q.handleEmbeddedQuery(matcher.Value) + if matcher.Name == "__name__" && matcher.Value == astmapper.EMBEDDED_QUERY_FLAG { + isEmbedded = true + } + + if matcher.Name == astmapper.QUERY_LABEL { + embeddedQuery = matcher.Value } } - return nil, nil, errors.Errorf("DownstreamQuerier cannot handle a non-embedded query") + if isEmbedded { + if embeddedQuery != "" { + return q.handleEmbeddedQuery(embeddedQuery) + } else { + return nil, nil, errors.Errorf(missingEmbeddedQueryMsg) + } + + } + + return nil, nil, errors.Errorf(nonEmbeddedErrMsg) } // handleEmbeddedQuery defers execution of an encoded query to a downstream Handler diff --git a/pkg/querier/querysharding/queryable_test.go b/pkg/querier/querysharding/queryable_test.go new file mode 100644 index 00000000000..a185de51f7e --- /dev/null +++ b/pkg/querier/querysharding/queryable_test.go @@ -0,0 +1,161 @@ +package querysharding + +import ( + "context" + "encoding/hex" + "github.com/cortexproject/cortex/pkg/ingester/client" + "github.com/cortexproject/cortex/pkg/querier/astmapper" + "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" +) + +func TestSelect(t *testing.T) { + var testExpr = []struct { + name string + querier *DownstreamQuerier + fn func(*testing.T, *DownstreamQuerier) + }{ + { + name: "errors non embedded query", + querier: querier( + nil, + ), + fn: func(t *testing.T, q *DownstreamQuerier) { + set, _, err := q.Select(nil) + require.Nil(t, set) + require.EqualError(t, err, nonEmbeddedErrMsg) + }, + }, + { + name: "replaces query", + querier: querier(mockHandler( + &queryrange.APIResponse{}, + nil, + )), + fn: func(t *testing.T, q *DownstreamQuerier) { + + expected := &queryrange.APIResponse{ + Status: "success", + Data: queryrange.Response{ + ResultType: promql.ValueTypeVector, + }, + } + + // override handler func to assert new query has been substituted + q.Handler = queryrange.HandlerFunc( + func(ctx context.Context, req *queryrange.Request) (*queryrange.APIResponse, error) { + require.Equal(t, `http_requests_total{cluster="prod"}`, req.Query) + return expected, nil + }, + ) + + _, _, err := q.Select( + nil, + exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), + exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + ) + require.Nil(t, err) + }, + }, + { + name: "propagates response error", + querier: querier(mockHandler( + &queryrange.APIResponse{ + Error: "SomeErr", + }, + nil, + )), + fn: func(t *testing.T, q *DownstreamQuerier) { + set, _, err := q.Select( + nil, + exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), + exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + ) + require.Nil(t, set) + require.EqualError(t, err, "SomeErr") + }, + }, + { + name: "returns SeriesSet", + querier: querier(mockHandler( + &queryrange.APIResponse{ + Data: queryrange.Response{ + ResultType: promql.ValueTypeVector, + Result: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + }, + }, + }, + }, + }, + nil, + )), + fn: func(t *testing.T, q *DownstreamQuerier) { + set, _, err := q.Select( + nil, + exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), + exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + ) + require.Nil(t, err) + require.Equal( + t, + &downstreamSeriesSet{ + set: []*downstreamSeries{ + { + metric: []labels.Label{ + {"a", "a1"}, + {"b", "b1"}, + }, + points: []promql.Point{ + {1, 1}, + }, + }, + }, + }, + set, + ) + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + c.fn(t, c.querier) + }) + } +} + +func exactMatch(k, v string) *labels.Matcher { + m, err := labels.NewMatcher(labels.MatchEqual, k, v) + if err != nil { + panic(err) + } + return m + +} + +func querier(handler queryrange.Handler) *DownstreamQuerier { + return &DownstreamQuerier{context.Background(), &queryrange.Request{}, handler} +} + +func hexEncode(str string) string { + return hex.EncodeToString([]byte(str)) +} + +func mockHandler(res *queryrange.APIResponse, err error) queryrange.Handler { + return queryrange.HandlerFunc(func(ctx context.Context, req *queryrange.Request) (*queryrange.APIResponse, error) { + return res, err + }) +} From 0e12f39745e155018a935eea85a09180fc0b5217 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 16:15:08 -0400 Subject: [PATCH 09/97] Series{Iterator,Set} expects to call Next() before At() initially (assumably for 0 ln series) Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/series.go | 32 ++++++----- pkg/querier/querysharding/series_test.go | 67 +++--------------------- 2 files changed, 25 insertions(+), 74 deletions(-) diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go index 4ebe21a1dda..79d7d55efdd 100644 --- a/pkg/querier/querysharding/series.go +++ b/pkg/querier/querysharding/series.go @@ -52,13 +52,19 @@ func NewSeriesSet(results []queryrange.SampleStream) *downstreamSeriesSet { // downstreamSeriesSet is an in-memory series that's mapped from a promql.Value (vector or matrix) type downstreamSeriesSet struct { - i int - set []*downstreamSeries + i int + set []*downstreamSeries + begun bool } // impls storage.SeriesSet func (set *downstreamSeriesSet) Next() bool { - set.i++ + if !set.begun { + set.begun = true + } else { + set.i++ + } + if set.i >= len(set.set) { return false } @@ -73,9 +79,6 @@ func (set *downstreamSeriesSet) At() storage.Series { // impls storage.SeriesSet func (set *downstreamSeriesSet) Err() error { - if set.i >= len(set.set) { - return errors.Errorf("downStreamSeriesSet out of bounds: cannot request series index %d of length %d", set.i, len(set.set)) - } return nil } @@ -83,6 +86,7 @@ type downstreamSeries struct { metric labels.Labels i int points []promql.Point + begun bool } // impls storage.Series @@ -107,12 +111,13 @@ func (series *downstreamSeries) Iterator() storage.SeriesIterator { // the given timestamp. func (series *downstreamSeries) Seek(t int64) bool { - for series.Err() == nil { + // TODO(owen): Is this supposed to automatically advance the iterator or should it return the current + // series if satisfies t? + for series.Next() { ts, _ := series.At() if ts >= t { return true } - series.Next() } return false @@ -128,7 +133,12 @@ func (series *downstreamSeries) At() (t int64, v float64) { // impls storage.SeriesIterator // Next advances the iterator by one. func (series *downstreamSeries) Next() bool { - series.i++ + if !series.begun { + series.begun = true + } else { + series.i++ + } + if series.i >= len(series.points) { return false } @@ -138,9 +148,5 @@ func (series *downstreamSeries) Next() bool { // impls storage.SeriesIterator // Err returns the current error. func (series *downstreamSeries) Err() error { - if series.i >= len(series.points) { - return errors.Errorf("downstreamSeries out of bounds: cannot request point %d of %d", series.i, len(series.points)) - } return nil - } diff --git a/pkg/querier/querysharding/series_test.go b/pkg/querier/querysharding/series_test.go index 3497885cf5f..ed230c6b976 100644 --- a/pkg/querier/querysharding/series_test.go +++ b/pkg/querier/querysharding/series_test.go @@ -154,14 +154,13 @@ func Test_downstreamSeriesSet_impls_storage_SeriesSet(t *testing.T) { }, } - for i := 0; i < len(set.set)-1; i++ { + for i := 0; i < len(set.set); i++ { require.Nil(t, set.Err()) - require.Equal(t, set.At(), set.set[i]) require.Equal(t, set.Next(), true) + require.Equal(t, set.At(), set.set[i]) } require.Equal(t, set.Next(), false) - require.NotNil(t, set.Err()) } func Test_downstreamSeries_impls_storage_Series(t *testing.T) { @@ -173,49 +172,6 @@ func Test_downstreamSeries_impls_storage_Series(t *testing.T) { require.Equal(t, series.Labels(), series.metric) } -func Test_downstreamSeries_impls_storage_SeriesIterator_Err(t *testing.T) { - var testExpr = []struct { - name string - input *downstreamSeries - f func(*testing.T, *downstreamSeries) - }{ - { - "empty", - &downstreamSeries{}, - func(t *testing.T, series *downstreamSeries) { - require.NotNil(t, series.Err()) - }, - }, - { - "past bounds", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - //advance internal index to end of data - series.i = len(series.points) - - require.NotNil(t, series.Err()) - }, - }, - { - "past bounds via next", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points); i++ { - series.Next() - } - - require.NotNil(t, series.Err()) - }, - }, - } - - for _, c := range testExpr { - t.Run(c.name, func(t *testing.T) { - c.f(t, c.input) - }) - } -} - func Test_downstreamSeries_impls_storage_SeriesIterator_Next(t *testing.T) { var testExpr = []struct { name string @@ -226,10 +182,9 @@ func Test_downstreamSeries_impls_storage_SeriesIterator_Next(t *testing.T) { "next advances series", initSeries(), func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points)-1; i++ { - require.Equal(t, i, series.i) + for i := 0; i < len(series.points); i++ { require.Equal(t, true, series.Next()) - require.Equal(t, i+1, series.i) + require.Equal(t, i, series.i) } }, }, @@ -237,7 +192,7 @@ func Test_downstreamSeries_impls_storage_SeriesIterator_Next(t *testing.T) { "next returns false at end of series", initSeries(), func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points)-1; i++ { + for i := 0; i < len(series.points); i++ { series.Next() } @@ -264,10 +219,10 @@ func Test_downstreamSeries_impls_storage_SeriesIterator_At(t *testing.T) { initSeries(), func(t *testing.T, series *downstreamSeries) { for i := 0; i < len(series.points); i++ { + series.Next() timestamp, val := series.At() require.Equal(t, series.points[i].T, timestamp) require.Equal(t, series.points[i].V, val) - series.Next() } }, }, @@ -311,16 +266,6 @@ func Test_downstreamSeries_impls_storage_SeriesIterator_Seek(t *testing.T) { require.Equal(t, int64(4), ts) }, }, - { - "doesnt unnecessarily advancer iter", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - require.Equal(t, true, series.Seek(2)) - require.Equal(t, true, series.Seek(2)) - ts, _ := series.At() - require.Equal(t, int64(2), ts) - }, - }, { "seek past end", initSeries(), From 8b134946589ffa6795025265b6d388417945c8cc Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 16:56:33 -0400 Subject: [PATCH 10/97] middleware embeds engine + unit tests Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 12 +- pkg/querier/querysharding/middleware_test.go | 153 +++++++++++++++++++ pkg/querier/querysharding/queryable_test.go | 6 - 3 files changed, 158 insertions(+), 13 deletions(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index ca31096f085..b4902a2503a 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -13,10 +13,11 @@ const ( parseErrType = "parse error" ) -func QueryShardMiddleware() queryrange.Middleware { +func QueryShardMiddleware(engine *promql.Engine) queryrange.Middleware { return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { return &queryShard{ - next: next, + next: next, + engine: engine, mapper: astmapper.NewMultiMapper( astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.VectorSquasher), astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), @@ -42,15 +43,12 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang ) if err != nil { - return &queryrange.APIResponse{ - Status: queryrange.StatusFailure, - ErrorType: parseErrType, - Error: err.Error(), - }, nil + return nil, err } res := qry.Exec(ctx) + // TODO(owen): Unclear on whether error belongs in APIResponse struct or as 2nd value in return tuple if res.Err != nil { return &queryrange.APIResponse{ Status: queryrange.StatusFailure, diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index 703e5a485b0..9daac294282 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -1,10 +1,163 @@ package querysharding import ( + "context" + "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/cortexproject/cortex/pkg/util" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/promql" + "github.com/stretchr/testify/require" + "testing" "time" ) +func TestMiddleware(t *testing.T) { + expired, _ := context.WithDeadline(context.Background(), time.Now()) + var testExpr = []struct { + name string + next queryrange.Handler + input *queryrange.Request + ctx context.Context + expected *queryrange.APIResponse + err bool + }{ + { + name: "invalid query error", + // if the query parses correctly force it to succeed + next: mockHandler(&queryrange.APIResponse{ + Status: "", + Data: queryrange.Response{ + ResultType: promql.ValueTypeVector, + Result: []queryrange.SampleStream{}, + }, + ErrorType: "", + Error: "", + }, nil), + input: &queryrange.Request{Query: "^GARBAGE"}, + ctx: context.Background(), + expected: nil, + err: true, + }, + { + name: "downstream err", + next: mockHandler(nil, errors.Errorf("some err")), + input: defaultReq(), + ctx: context.Background(), + expected: &queryrange.APIResponse{ + Status: queryrange.StatusFailure, + ErrorType: downStreamErrType, + Error: "some err", + }, + err: false, + }, + { + name: "context expiry", + next: mockHandler(&queryrange.APIResponse{ + Status: "", + Data: queryrange.Response{ + ResultType: promql.ValueTypeVector, + Result: []queryrange.SampleStream{}, + }, + ErrorType: "", + Error: "", + }, nil), + input: defaultReq(), + ctx: expired, + expected: &queryrange.APIResponse{ + Status: queryrange.StatusFailure, + ErrorType: downStreamErrType, + Error: "query timed out in query execution", + }, + err: false, + }, + { + name: "successful trip", + next: mockHandler(sampleMatrixResponse(), nil), + input: defaultReq(), + ctx: context.Background(), + expected: sampleMatrixResponse(), + err: false, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + engine := promql.NewEngine(promql.EngineOpts{ + Logger: util.Logger, + Reg: nil, + MaxConcurrent: 10, + MaxSamples: 1000, + Timeout: time.Minute, + }) + + mware := QueryShardMiddleware(engine).Wrap(c.next) + out, err := mware.Do(c.ctx, c.input) + + if c.err { + require.NotNil(t, err) + } else { + require.Nil(t, err) + require.Equal(t, c.expected, out) + } + + }) + } +} + +func sampleMatrixResponse() *queryrange.APIResponse { + return &queryrange.APIResponse{ + Status: queryrange.StatusSuccess, + Data: queryrange.Response{ + ResultType: promql.ValueTypeMatrix, + Result: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, + }, + client.Sample{ + Value: 2, + TimestampMs: 2, + }, + }, + }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 8, + TimestampMs: 1, + }, + client.Sample{ + Value: 9, + TimestampMs: 2, + }, + }, + }, + }, + }, + } +} + +func mockHandler(resp *queryrange.APIResponse, err error) queryrange.Handler { + return queryrange.HandlerFunc(func(ctx context.Context, req *queryrange.Request) (*queryrange.APIResponse, error) { + if expired := ctx.Err(); expired != nil { + return nil, expired + } + + return resp, err + }) +} + func defaultReq() *queryrange.Request { return &queryrange.Request{ Path: "/query_range", diff --git a/pkg/querier/querysharding/queryable_test.go b/pkg/querier/querysharding/queryable_test.go index a185de51f7e..0e926ce8361 100644 --- a/pkg/querier/querysharding/queryable_test.go +++ b/pkg/querier/querysharding/queryable_test.go @@ -153,9 +153,3 @@ func querier(handler queryrange.Handler) *DownstreamQuerier { func hexEncode(str string) string { return hex.EncodeToString([]byte(str)) } - -func mockHandler(res *queryrange.APIResponse, err error) queryrange.Handler { - return queryrange.HandlerFunc(func(ctx context.Context, req *queryrange.Request) (*queryrange.APIResponse, error) { - return res, err - }) -} From e1d7d97b815c368e7e6666bc85b48b5ebb300131 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 17:07:11 -0400 Subject: [PATCH 11/97] exports ConcreteSeries{,Set} Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/chunk_store_queryable.go | 2 +- pkg/querier/ingester_streaming_queryable.go | 2 +- pkg/querier/matrix.go | 2 +- pkg/querier/series_set.go | 38 ++++++++++----------- pkg/querier/series_set_test.go | 6 ++-- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pkg/querier/chunk_store_queryable.go b/pkg/querier/chunk_store_queryable.go index a28eaf37dfa..7661f4ed5ae 100644 --- a/pkg/querier/chunk_store_queryable.go +++ b/pkg/querier/chunk_store_queryable.go @@ -65,7 +65,7 @@ func (q *chunkStoreQuerier) partitionChunks(chunks []chunk.Chunk) storage.Series }) } - return newConcreteSeriesSet(series) + return NewConcreteSeriesSet(series) } func (q *chunkStoreQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { diff --git a/pkg/querier/ingester_streaming_queryable.go b/pkg/querier/ingester_streaming_queryable.go index f516a39267c..a8482f72ccc 100644 --- a/pkg/querier/ingester_streaming_queryable.go +++ b/pkg/querier/ingester_streaming_queryable.go @@ -109,5 +109,5 @@ func (q *ingesterStreamingQuerier) Select(sp *storage.SelectParams, matchers ... serieses = append(serieses, series) } - return newConcreteSeriesSet(serieses), nil, nil + return NewConcreteSeriesSet(serieses), nil, nil } diff --git a/pkg/querier/matrix.go b/pkg/querier/matrix.go index c6bd9d0ec3f..fdde327b98b 100644 --- a/pkg/querier/matrix.go +++ b/pkg/querier/matrix.go @@ -20,5 +20,5 @@ func mergeChunks(chunks []chunk.Chunk, from, through model.Time) storage.SeriesI } merged := util.MergeNSampleSets(samples...) - return newConcreteSeriesIterator(newConcreteSeries(nil, merged)) + return newConcreteSeriesIterator(NewConcreteSeries(nil, merged)) } diff --git a/pkg/querier/series_set.go b/pkg/querier/series_set.go index bd302cff678..d2879bb6540 100644 --- a/pkg/querier/series_set.go +++ b/pkg/querier/series_set.go @@ -25,61 +25,61 @@ import ( "github.com/prometheus/prometheus/storage" ) -// concreteSeriesSet implements storage.SeriesSet. -type concreteSeriesSet struct { +// ConcreteSeriesSet implements storage.SeriesSet. +type ConcreteSeriesSet struct { cur int series []storage.Series } -func newConcreteSeriesSet(series []storage.Series) storage.SeriesSet { +func NewConcreteSeriesSet(series []storage.Series) storage.SeriesSet { sort.Sort(byLabels(series)) - return &concreteSeriesSet{ + return &ConcreteSeriesSet{ cur: -1, series: series, } } -func (c *concreteSeriesSet) Next() bool { +func (c *ConcreteSeriesSet) Next() bool { c.cur++ return c.cur < len(c.series) } -func (c *concreteSeriesSet) At() storage.Series { +func (c *ConcreteSeriesSet) At() storage.Series { return c.series[c.cur] } -func (c *concreteSeriesSet) Err() error { +func (c *ConcreteSeriesSet) Err() error { return nil } -// concreteSeries implements storage.Series. -type concreteSeries struct { +// ConcreteSeries implements storage.Series. +type ConcreteSeries struct { labels labels.Labels samples []model.SamplePair } -func newConcreteSeries(ls labels.Labels, samples []model.SamplePair) *concreteSeries { - return &concreteSeries{ +func NewConcreteSeries(ls labels.Labels, samples []model.SamplePair) *ConcreteSeries { + return &ConcreteSeries{ labels: ls, samples: samples, } } -func (c *concreteSeries) Labels() labels.Labels { +func (c *ConcreteSeries) Labels() labels.Labels { return c.labels } -func (c *concreteSeries) Iterator() storage.SeriesIterator { +func (c *ConcreteSeries) Iterator() storage.SeriesIterator { return newConcreteSeriesIterator(c) } // concreteSeriesIterator implements storage.SeriesIterator. type concreteSeriesIterator struct { cur int - series *concreteSeries + series *ConcreteSeries } -func newConcreteSeriesIterator(series *concreteSeries) storage.SeriesIterator { +func newConcreteSeriesIterator(series *ConcreteSeries) storage.SeriesIterator { return &concreteSeriesIterator{ cur: -1, series: series, @@ -131,23 +131,23 @@ func (e errIterator) Err() error { func matrixToSeriesSet(m model.Matrix) storage.SeriesSet { series := make([]storage.Series, 0, len(m)) for _, ss := range m { - series = append(series, &concreteSeries{ + series = append(series, &ConcreteSeries{ labels: metricToLabels(ss.Metric), samples: ss.Values, }) } - return newConcreteSeriesSet(series) + return NewConcreteSeriesSet(series) } func metricsToSeriesSet(ms []metric.Metric) storage.SeriesSet { series := make([]storage.Series, 0, len(ms)) for _, m := range ms { - series = append(series, &concreteSeries{ + series = append(series, &ConcreteSeries{ labels: metricToLabels(m.Metric), samples: nil, }) } - return newConcreteSeriesSet(series) + return NewConcreteSeriesSet(series) } func metricToLabels(m model.Metric) labels.Labels { diff --git a/pkg/querier/series_set_test.go b/pkg/querier/series_set_test.go index a1244b57f4d..98989745c7a 100644 --- a/pkg/querier/series_set_test.go +++ b/pkg/querier/series_set_test.go @@ -10,15 +10,15 @@ import ( ) func TestConcreteSeriesSet(t *testing.T) { - series1 := &concreteSeries{ + series1 := &ConcreteSeries{ labels: labels.FromStrings("foo", "bar"), samples: []model.SamplePair{{Value: 1, Timestamp: 2}}, } - series2 := &concreteSeries{ + series2 := &ConcreteSeries{ labels: labels.FromStrings("foo", "baz"), samples: []model.SamplePair{{Value: 3, Timestamp: 4}}, } - c := newConcreteSeriesSet([]storage.Series{series2, series1}) + c := NewConcreteSeriesSet([]storage.Series{series2, series1}) require.True(t, c.Next()) require.Equal(t, series1, c.At()) require.True(t, c.Next()) From c94238229b32fdfef438cb5f6f335f894a855b11 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 18:13:14 -0400 Subject: [PATCH 12/97] removes custom Series{,Set,Iterator} impls in favor of queriers concrete impl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/queryable_test.go | 69 ++++- pkg/querier/querysharding/series.go | 136 ++------- pkg/querier/querysharding/series_test.go | 304 +++----------------- 3 files changed, 110 insertions(+), 399 deletions(-) diff --git a/pkg/querier/querysharding/queryable_test.go b/pkg/querier/querysharding/queryable_test.go index 0e926ce8361..82b410e61a0 100644 --- a/pkg/querier/querysharding/queryable_test.go +++ b/pkg/querier/querysharding/queryable_test.go @@ -20,7 +20,7 @@ func TestSelect(t *testing.T) { }{ { name: "errors non embedded query", - querier: querier( + querier: mkQuerier( nil, ), fn: func(t *testing.T, q *DownstreamQuerier) { @@ -31,7 +31,7 @@ func TestSelect(t *testing.T) { }, { name: "replaces query", - querier: querier(mockHandler( + querier: mkQuerier(mockHandler( &queryrange.APIResponse{}, nil, )), @@ -62,7 +62,7 @@ func TestSelect(t *testing.T) { }, { name: "propagates response error", - querier: querier(mockHandler( + querier: mkQuerier(mockHandler( &queryrange.APIResponse{ Error: "SomeErr", }, @@ -80,7 +80,7 @@ func TestSelect(t *testing.T) { }, { name: "returns SeriesSet", - querier: querier(mockHandler( + querier: mkQuerier(mockHandler( &queryrange.APIResponse{ Data: queryrange.Response{ ResultType: promql.ValueTypeVector, @@ -95,6 +95,26 @@ func TestSelect(t *testing.T) { Value: 1, TimestampMs: 1, }, + client.Sample{ + Value: 2, + TimestampMs: 2, + }, + }, + }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 8, + TimestampMs: 1, + }, + client.Sample{ + Value: 9, + TimestampMs: 2, + }, }, }, }, @@ -111,19 +131,40 @@ func TestSelect(t *testing.T) { require.Nil(t, err) require.Equal( t, - &downstreamSeriesSet{ - set: []*downstreamSeries{ - { - metric: []labels.Label{ - {"a", "a1"}, - {"b", "b1"}, + newSeriesSet([]queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, }, - points: []promql.Point{ - {1, 1}, + client.Sample{ + Value: 2, + TimestampMs: 2, }, }, }, - }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 8, + TimestampMs: 1, + }, + client.Sample{ + Value: 9, + TimestampMs: 2, + }, + }, + }, + }), set, ) }, @@ -146,7 +187,7 @@ func exactMatch(k, v string) *labels.Matcher { } -func querier(handler queryrange.Handler) *DownstreamQuerier { +func mkQuerier(handler queryrange.Handler) *DownstreamQuerier { return &DownstreamQuerier{context.Background(), &queryrange.Request{}, handler} } diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go index 79d7d55efdd..59274cf27c8 100644 --- a/pkg/querier/querysharding/series.go +++ b/pkg/querier/querysharding/series.go @@ -1,8 +1,10 @@ package querysharding import ( + "github.com/cortexproject/cortex/pkg/querier" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/pkg/errors" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/storage" @@ -12,7 +14,7 @@ import ( func ResponseToSeries(resp queryrange.Response) (storage.SeriesSet, error) { switch resp.ResultType { case promql.ValueTypeVector, promql.ValueTypeMatrix: - return NewSeriesSet(resp.Result), nil + return newSeriesSet(resp.Result), nil } return nil, errors.Errorf( @@ -23,130 +25,24 @@ func ResponseToSeries(resp queryrange.Response) (storage.SeriesSet, error) { ) } -func NewSeriesSet(results []queryrange.SampleStream) *downstreamSeriesSet { - set := make([]*downstreamSeries, 0, len(results)) - for _, srcSeries := range results { - series := &downstreamSeries{ - metric: make([]labels.Label, 0, len(srcSeries.Labels)), - points: make([]promql.Point, 0, len(srcSeries.Samples)), - } +func newSeriesSet(results []queryrange.SampleStream) storage.SeriesSet { - for _, l := range srcSeries.Labels { - series.metric = append(series.metric, labels.Label(l)) - } + set := make([]storage.Series, 0, len(results)) - for _, pt := range srcSeries.Samples { - series.points = append(series.points, promql.Point{ - T: pt.TimestampMs, - V: pt.Value, + for _, stream := range results { + samples := make([]model.SamplePair, 0, len(stream.Samples)) + for _, sample := range stream.Samples { + samples = append(samples, model.SamplePair{ + Timestamp: model.Time(sample.TimestampMs), + Value: model.SampleValue(sample.Value), }) } - set = append(set, series) - } - - return &downstreamSeriesSet{ - set: set, - } -} - -// downstreamSeriesSet is an in-memory series that's mapped from a promql.Value (vector or matrix) -type downstreamSeriesSet struct { - i int - set []*downstreamSeries - begun bool -} - -// impls storage.SeriesSet -func (set *downstreamSeriesSet) Next() bool { - if !set.begun { - set.begun = true - } else { - set.i++ - } - - if set.i >= len(set.set) { - return false - } - - return true -} - -// impls storage.SeriesSet -func (set *downstreamSeriesSet) At() storage.Series { - return set.set[set.i] -} - -// impls storage.SeriesSet -func (set *downstreamSeriesSet) Err() error { - return nil -} - -type downstreamSeries struct { - metric labels.Labels - i int - points []promql.Point - begun bool -} - -// impls storage.Series -// Labels returns the complete set of labels identifying the series. -func (series *downstreamSeries) Labels() labels.Labels { - return series.metric -} - -// impls storage.Series -// Iterator returns a new iterator of the data of the series. -func (series *downstreamSeries) Iterator() storage.SeriesIterator { - // TODO(owen): unsure if this method should return a new iterator re-indexed to 0 or if it can - // be a passthrough method. Opting for the former for safety (although it contains the same slice). - return &downstreamSeries{ - metric: series.metric, - points: series.points, - } -} - -// impls storage.SeriesIterator -// Seek advances the iterator forward to the value at or after -// the given timestamp. -func (series *downstreamSeries) Seek(t int64) bool { - - // TODO(owen): Is this supposed to automatically advance the iterator or should it return the current - // series if satisfies t? - for series.Next() { - ts, _ := series.At() - if ts >= t { - return true + ls := make([]labels.Label, 0, len(stream.Labels)) + for _, l := range stream.Labels { + ls = append(ls, labels.Label(l)) } + set = append(set, querier.NewConcreteSeries(ls, samples)) } - - return false -} - -// impls storage.SeriesIterator -// At returns the current timestamp/value pair. -func (series *downstreamSeries) At() (t int64, v float64) { - pt := series.points[series.i] - return pt.T, pt.V -} - -// impls storage.SeriesIterator -// Next advances the iterator by one. -func (series *downstreamSeries) Next() bool { - if !series.begun { - series.begun = true - } else { - series.i++ - } - - if series.i >= len(series.points) { - return false - } - return true -} - -// impls storage.SeriesIterator -// Err returns the current error. -func (series *downstreamSeries) Err() error { - return nil + return querier.NewConcreteSeriesSet(set) } diff --git a/pkg/querier/querysharding/series_test.go b/pkg/querier/querysharding/series_test.go index ed230c6b976..aead8f42033 100644 --- a/pkg/querier/querysharding/series_test.go +++ b/pkg/querier/querysharding/series_test.go @@ -3,293 +3,67 @@ package querysharding import ( "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" - "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" "testing" ) -func TestResponseToSeries(t *testing.T) { - var testExpr = []struct { - name string - input queryrange.Response - err bool - expected *downstreamSeriesSet - }{ - { - name: "vector coercion", - input: queryrange.Response{ - ResultType: promql.ValueTypeVector, - Result: []queryrange.SampleStream{ - { - Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, - }, - Samples: []client.Sample{ - client.Sample{ - Value: 1, - TimestampMs: 1, - }, - }, - }, +func Test_ResponseToSeries(t *testing.T) { + input := queryrange.Response{ + ResultType: promql.ValueTypeMatrix, + Result: []queryrange.SampleStream{ + { + Labels: []client.LabelAdapter{ + {"a", "a1"}, + {"b", "b1"}, }, - }, - err: false, - expected: &downstreamSeriesSet{ - set: []*downstreamSeries{ - { - metric: []labels.Label{ - {"a", "a1"}, - {"b", "b1"}, - }, - points: []promql.Point{ - {1, 1}, - }, + Samples: []client.Sample{ + client.Sample{ + Value: 1, + TimestampMs: 1, }, - }, - }, - }, - { - name: "matrix coercion", - input: queryrange.Response{ - ResultType: promql.ValueTypeMatrix, - Result: []queryrange.SampleStream{ - { - Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, - }, - Samples: []client.Sample{ - { - Value: 1, - TimestampMs: 1, - }, - { - Value: 2, - TimestampMs: 2, - }, - }, - }, - { - Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, - }, - Samples: []client.Sample{ - { - Value: 8, - TimestampMs: 1, - }, - { - Value: 9, - TimestampMs: 2, - }, - }, + client.Sample{ + Value: 2, + TimestampMs: 2, }, }, }, - err: false, - expected: &downstreamSeriesSet{ - set: []*downstreamSeries{ - { - metric: labels.Labels{ - {"a", "a1"}, - {"b", "b1"}, - }, - points: []promql.Point{ - {1, 1}, - {2, 2}, - }, + { + Labels: []client.LabelAdapter{ + {"a", "a2"}, + {"b", "b2"}, + }, + Samples: []client.Sample{ + client.Sample{ + Value: 8, + TimestampMs: 1, }, - { - metric: labels.Labels{ - {"a", "a2"}, - {"b", "b2"}, - }, - points: []promql.Point{ - {1, 8}, - {2, 9}, - }, + client.Sample{ + Value: 9, + TimestampMs: 2, }, }, - }}, - { - name: "unknown type -- error converting", - input: queryrange.Response{ - ResultType: "unsupported", - Result: []queryrange.SampleStream{}, }, - err: true, - expected: nil, }, } - for _, c := range testExpr { - t.Run(c.name, func(t *testing.T) { - result, err := ResponseToSeries(c.input) - if c.err { - require.NotNil(t, err) - } else { - require.Nil(t, err) - require.Equal(t, c.expected, result) - } - }) - } -} + set, err := ResponseToSeries(input) + require.Nil(t, err) -func Test_downstreamSeriesSet_impls_storage_SeriesSet(t *testing.T) { - set := &downstreamSeriesSet{ - // use nil for test expediency -- just checking that indices are maintained - set: []*downstreamSeries{ - &downstreamSeries{ - metric: labels.Labels{{"a", "a"}}, - }, - &downstreamSeries{ - metric: labels.Labels{{"b", "b"}}, - }, - &downstreamSeries{ - metric: labels.Labels{{"c", "c"}}, - }, - }, - } + setCt := 0 - for i := 0; i < len(set.set); i++ { + for set.Next() { + iter := set.At().Iterator() require.Nil(t, set.Err()) - require.Equal(t, set.Next(), true) - require.Equal(t, set.At(), set.set[i]) - } - - require.Equal(t, set.Next(), false) -} - -func Test_downstreamSeries_impls_storage_Series(t *testing.T) { - series := initSeries() - - // iterator is a passthrough (downstreamSeries impls both Series & SeriesIterator) - require.Equal(t, series, series.Iterator()) - - require.Equal(t, series.Labels(), series.metric) -} - -func Test_downstreamSeries_impls_storage_SeriesIterator_Next(t *testing.T) { - var testExpr = []struct { - name string - input *downstreamSeries - f func(*testing.T, *downstreamSeries) - }{ - { - "next advances series", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points); i++ { - require.Equal(t, true, series.Next()) - require.Equal(t, i, series.i) - } - }, - }, - { - "next returns false at end of series", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points); i++ { - series.Next() - } - - require.Equal(t, false, series.Next()) - }, - }, - } - for _, c := range testExpr { - t.Run(c.name, func(t *testing.T) { - c.f(t, c.input) - }) + sampleCt := 0 + for iter.Next() { + sampleCt++ + } + require.Equal(t, len(input.Result[setCt].Samples), sampleCt) + setCt++ } -} -func Test_downstreamSeries_impls_storage_SeriesIterator_At(t *testing.T) { - var testExpr = []struct { - name string - input *downstreamSeries - f func(*testing.T, *downstreamSeries) - }{ - { - "At tracks current index", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - for i := 0; i < len(series.points); i++ { - series.Next() - timestamp, val := series.At() - require.Equal(t, series.points[i].T, timestamp) - require.Equal(t, series.points[i].V, val) - } - }, - }, - } + require.Equal(t, len(input.Result), setCt) - for _, c := range testExpr { - t.Run(c.name, func(t *testing.T) { - c.f(t, c.input) - }) - } -} - -func Test_downstreamSeries_impls_storage_SeriesIterator_Seek(t *testing.T) { - var testExpr = []struct { - name string - input *downstreamSeries - f func(*testing.T, *downstreamSeries) - }{ - { - "advances iterator at", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - require.Equal(t, true, series.Seek(2)) - ts, _ := series.At() - require.Equal(t, int64(2), ts) - }, - }, - { - "advances iterator after", - &downstreamSeries{ - metric: labels.Labels{{"a", "a"}}, - points: []promql.Point{ - {1, 1}, - {2, 2}, - {4, 4}, - }, - }, - func(t *testing.T, series *downstreamSeries) { - require.Equal(t, true, series.Seek(3)) - ts, _ := series.At() - require.Equal(t, int64(4), ts) - }, - }, - { - "seek past end", - initSeries(), - func(t *testing.T, series *downstreamSeries) { - require.Equal(t, false, series.Seek(100)) - }, - }, - } - - for _, c := range testExpr { - t.Run(c.name, func(t *testing.T) { - c.f(t, c.input) - }) - } -} - -// helper for generating testable series -func initSeries() *downstreamSeries { - return &downstreamSeries{ - metric: labels.Labels{{"a", "a"}}, - points: []promql.Point{ - {1, 1}, - {2, 2}, - {3, 3}, - }, - } } From ff1a38547433b8be6ffbaaa0dff8598c61bb7878 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 10 Oct 2019 19:50:50 -0400 Subject: [PATCH 13/97] concreteSeries moved to its own pkg Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/chunk_store_queryable.go | 3 +- pkg/querier/distributor_queryable.go | 3 +- pkg/querier/ingester_streaming_queryable.go | 3 +- pkg/querier/matrix.go | 5 +- pkg/querier/querier.go | 3 +- pkg/querier/querysharding/middleware_test.go | 66 ++++++++++---------- pkg/querier/querysharding/series.go | 6 +- pkg/querier/querysharding/series_test.go | 3 + pkg/querier/remote_read_test.go | 3 +- pkg/querier/{ => series}/series_set.go | 14 +++-- pkg/querier/{ => series}/series_set_test.go | 4 +- 11 files changed, 63 insertions(+), 50 deletions(-) rename pkg/querier/{ => series}/series_set.go (92%) rename pkg/querier/{ => series}/series_set_test.go (96%) diff --git a/pkg/querier/chunk_store_queryable.go b/pkg/querier/chunk_store_queryable.go index 7661f4ed5ae..8c02d238e62 100644 --- a/pkg/querier/chunk_store_queryable.go +++ b/pkg/querier/chunk_store_queryable.go @@ -11,6 +11,7 @@ import ( "github.com/weaveworks/common/user" "github.com/cortexproject/cortex/pkg/chunk" + seriesset "github.com/cortexproject/cortex/pkg/querier/series" ) type chunkIteratorFunc func(chunks []chunk.Chunk, from, through model.Time) storage.SeriesIterator @@ -65,7 +66,7 @@ func (q *chunkStoreQuerier) partitionChunks(chunks []chunk.Chunk) storage.Series }) } - return NewConcreteSeriesSet(series) + return seriesset.NewConcreteSeriesSet(series) } func (q *chunkStoreQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { diff --git a/pkg/querier/distributor_queryable.go b/pkg/querier/distributor_queryable.go index c52d67b05be..b941cf5eb4f 100644 --- a/pkg/querier/distributor_queryable.go +++ b/pkg/querier/distributor_queryable.go @@ -10,6 +10,7 @@ import ( "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/prom1/storage/metric" + "github.com/cortexproject/cortex/pkg/querier/series" ) // Distributor is the read interface to the distributor, made an interface here @@ -51,7 +52,7 @@ func (q *distributorQuerier) Select(sp *storage.SelectParams, matchers ...*label return nil, nil, promql.ErrStorage{Err: err} } - return matrixToSeriesSet(matrix), nil, nil + return series.MatrixToSeriesSet(matrix), nil, nil } func (q *distributorQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { diff --git a/pkg/querier/ingester_streaming_queryable.go b/pkg/querier/ingester_streaming_queryable.go index a8482f72ccc..8fbe9b90617 100644 --- a/pkg/querier/ingester_streaming_queryable.go +++ b/pkg/querier/ingester_streaming_queryable.go @@ -11,6 +11,7 @@ import ( "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/ingester/client" + seriesset "github.com/cortexproject/cortex/pkg/querier/series" "github.com/cortexproject/cortex/pkg/util/chunkcompat" "github.com/weaveworks/common/user" ) @@ -109,5 +110,5 @@ func (q *ingesterStreamingQuerier) Select(sp *storage.SelectParams, matchers ... serieses = append(serieses, series) } - return NewConcreteSeriesSet(serieses), nil, nil + return seriesset.NewConcreteSeriesSet(serieses), nil, nil } diff --git a/pkg/querier/matrix.go b/pkg/querier/matrix.go index fdde327b98b..8ac963ad5ac 100644 --- a/pkg/querier/matrix.go +++ b/pkg/querier/matrix.go @@ -5,6 +5,7 @@ import ( "github.com/prometheus/prometheus/storage" "github.com/cortexproject/cortex/pkg/chunk" + "github.com/cortexproject/cortex/pkg/querier/series" "github.com/cortexproject/cortex/pkg/util" ) @@ -13,12 +14,12 @@ func mergeChunks(chunks []chunk.Chunk, from, through model.Time) storage.SeriesI for _, c := range chunks { ss, err := c.Samples(from, through) if err != nil { - return errIterator{err} + return series.NewErrIterator(err) } samples = append(samples, ss) } merged := util.MergeNSampleSets(samples...) - return newConcreteSeriesIterator(NewConcreteSeries(nil, merged)) + return series.NewConcreteSeriesIterator(series.NewConcreteSeries(nil, merged)) } diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index d91937d74ff..5e6b30707fe 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -14,6 +14,7 @@ import ( "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/batch" "github.com/cortexproject/cortex/pkg/querier/iterators" + seriesset "github.com/cortexproject/cortex/pkg/querier/series" "github.com/cortexproject/cortex/pkg/util" ) @@ -183,7 +184,7 @@ func (q querier) metadataQuery(matchers ...*labels.Matcher) (storage.SeriesSet, if err != nil { return nil, nil, err } - return metricsToSeriesSet(ms), nil, nil + return seriesset.MetricsToSeriesSet(ms), nil, nil } func (querier) Close() error { diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index 9daac294282..3c558dc85b2 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -13,7 +13,6 @@ import ( ) func TestMiddleware(t *testing.T) { - expired, _ := context.WithDeadline(context.Background(), time.Now()) var testExpr = []struct { name string next queryrange.Handler @@ -21,6 +20,7 @@ func TestMiddleware(t *testing.T) { ctx context.Context expected *queryrange.APIResponse err bool + override func(*testing.T, queryrange.Handler) }{ { name: "invalid query error", @@ -52,32 +52,24 @@ func TestMiddleware(t *testing.T) { err: false, }, { - name: "context expiry", - next: mockHandler(&queryrange.APIResponse{ - Status: "", - Data: queryrange.Response{ - ResultType: promql.ValueTypeVector, - Result: []queryrange.SampleStream{}, - }, - ErrorType: "", - Error: "", - }, nil), - input: defaultReq(), - ctx: expired, - expected: &queryrange.APIResponse{ - Status: queryrange.StatusFailure, - ErrorType: downStreamErrType, - Error: "query timed out in query execution", + name: "expiration", + next: mockHandler(sampleMatrixResponse(), nil), + override: func(t *testing.T, handler queryrange.Handler) { + expired, _ := context.WithDeadline(context.Background(), time.Unix(0, 0)) + res, err := handler.Do(expired, defaultReq()) + require.Nil(t, err) + require.NotEqual(t, "", res.Error) }, - err: false, }, { - name: "successful trip", - next: mockHandler(sampleMatrixResponse(), nil), - input: defaultReq(), - ctx: context.Background(), - expected: sampleMatrixResponse(), - err: false, + name: "successful trip", + next: mockHandler(sampleMatrixResponse(), nil), + override: func(t *testing.T, handler queryrange.Handler) { + out, err := handler.Do(context.Background(), defaultReq()) + require.Nil(t, err) + require.Equal(t, promql.ValueTypeMatrix, out.Data.ResultType) + require.Equal(t, sampleMatrixResponse(), out) + }, }, } @@ -91,8 +83,15 @@ func TestMiddleware(t *testing.T) { Timeout: time.Minute, }) - mware := QueryShardMiddleware(engine).Wrap(c.next) - out, err := mware.Do(c.ctx, c.input) + handler := QueryShardMiddleware(engine).Wrap(c.next) + + // escape hatch for custom tests + if c.override != nil { + c.override(t, handler) + return + } + + out, err := handler.Do(c.ctx, c.input) if c.err { require.NotNil(t, err) @@ -118,12 +117,12 @@ func sampleMatrixResponse() *queryrange.APIResponse { }, Samples: []client.Sample{ client.Sample{ + TimestampMs: 5000, Value: 1, - TimestampMs: 1, }, client.Sample{ + TimestampMs: 10000, Value: 2, - TimestampMs: 2, }, }, }, @@ -134,12 +133,12 @@ func sampleMatrixResponse() *queryrange.APIResponse { }, Samples: []client.Sample{ client.Sample{ + TimestampMs: 5000, Value: 8, - TimestampMs: 1, }, client.Sample{ + TimestampMs: 10000, Value: 9, - TimestampMs: 2, }, }, }, @@ -161,10 +160,11 @@ func mockHandler(resp *queryrange.APIResponse, err error) queryrange.Handler { func defaultReq() *queryrange.Request { return &queryrange.Request{ Path: "/query_range", - Start: 10, - End: 20, + Start: 00, + End: 10, Step: 5, Timeout: time.Minute, - Query: `__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2270726f64227d"}`, + // encoding of: `http_requests_total{cluster="prod"}` + Query: `__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2270726f64227d"}`, } } diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go index 59274cf27c8..1e300b9c16f 100644 --- a/pkg/querier/querysharding/series.go +++ b/pkg/querier/querysharding/series.go @@ -1,8 +1,8 @@ package querysharding import ( - "github.com/cortexproject/cortex/pkg/querier" "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/cortexproject/cortex/pkg/querier/series" "github.com/pkg/errors" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" @@ -42,7 +42,7 @@ func newSeriesSet(results []queryrange.SampleStream) storage.SeriesSet { for _, l := range stream.Labels { ls = append(ls, labels.Label(l)) } - set = append(set, querier.NewConcreteSeries(ls, samples)) + set = append(set, series.NewConcreteSeries(ls, samples)) } - return querier.NewConcreteSeriesSet(set) + return series.NewConcreteSeriesSet(set) } diff --git a/pkg/querier/querysharding/series_test.go b/pkg/querier/querysharding/series_test.go index aead8f42033..b255a1da97b 100644 --- a/pkg/querier/querysharding/series_test.go +++ b/pkg/querier/querysharding/series_test.go @@ -58,6 +58,9 @@ func Test_ResponseToSeries(t *testing.T) { sampleCt := 0 for iter.Next() { + ts, v := iter.At() + require.Equal(t, input.Result[setCt].Samples[sampleCt].TimestampMs, ts) + require.Equal(t, input.Result[setCt].Samples[sampleCt].Value, v) sampleCt++ } require.Equal(t, len(input.Result[setCt].Samples), sampleCt) diff --git a/pkg/querier/remote_read_test.go b/pkg/querier/remote_read_test.go index 049e0f56a7e..c7641eaabaa 100644 --- a/pkg/querier/remote_read_test.go +++ b/pkg/querier/remote_read_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/cortexproject/cortex/pkg/ingester/client" + "github.com/cortexproject/cortex/pkg/querier/series" "github.com/gogo/protobuf/proto" "github.com/golang/snappy" "github.com/prometheus/common/model" @@ -89,7 +90,7 @@ func (m mockQuerier) Select(sp *storage.SelectParams, matchers ...*labels.Matche if sp == nil { panic(fmt.Errorf("select params must be set")) } - return matrixToSeriesSet(m.matrix), nil, nil + return series.MatrixToSeriesSet(m.matrix), nil, nil } func (m mockQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { diff --git a/pkg/querier/series_set.go b/pkg/querier/series/series_set.go similarity index 92% rename from pkg/querier/series_set.go rename to pkg/querier/series/series_set.go index d2879bb6540..a181d9c2797 100644 --- a/pkg/querier/series_set.go +++ b/pkg/querier/series/series_set.go @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package querier +package series import ( "sort" @@ -70,7 +70,7 @@ func (c *ConcreteSeries) Labels() labels.Labels { } func (c *ConcreteSeries) Iterator() storage.SeriesIterator { - return newConcreteSeriesIterator(c) + return NewConcreteSeriesIterator(c) } // concreteSeriesIterator implements storage.SeriesIterator. @@ -79,7 +79,7 @@ type concreteSeriesIterator struct { series *ConcreteSeries } -func newConcreteSeriesIterator(series *ConcreteSeries) storage.SeriesIterator { +func NewConcreteSeriesIterator(series *ConcreteSeries) storage.SeriesIterator { return &concreteSeriesIterator{ cur: -1, series: series, @@ -107,6 +107,10 @@ func (c *concreteSeriesIterator) Err() error { return nil } +func NewErrIterator(err error) errIterator { + return errIterator{err} +} + // errIterator implements storage.SeriesIterator, just returning an error. type errIterator struct { err error @@ -128,7 +132,7 @@ func (e errIterator) Err() error { return e.err } -func matrixToSeriesSet(m model.Matrix) storage.SeriesSet { +func MatrixToSeriesSet(m model.Matrix) storage.SeriesSet { series := make([]storage.Series, 0, len(m)) for _, ss := range m { series = append(series, &ConcreteSeries{ @@ -139,7 +143,7 @@ func matrixToSeriesSet(m model.Matrix) storage.SeriesSet { return NewConcreteSeriesSet(series) } -func metricsToSeriesSet(ms []metric.Metric) storage.SeriesSet { +func MetricsToSeriesSet(ms []metric.Metric) storage.SeriesSet { series := make([]storage.Series, 0, len(ms)) for _, m := range ms { series = append(series, &ConcreteSeries{ diff --git a/pkg/querier/series_set_test.go b/pkg/querier/series/series_set_test.go similarity index 96% rename from pkg/querier/series_set_test.go rename to pkg/querier/series/series_set_test.go index 98989745c7a..a8ca1ae4efc 100644 --- a/pkg/querier/series_set_test.go +++ b/pkg/querier/series/series_set_test.go @@ -1,4 +1,4 @@ -package querier +package series import ( "testing" @@ -39,7 +39,7 @@ func TestMatrixToSeriesSetSortsMetricLabels(t *testing.T) { Values: []model.SamplePair{{Timestamp: 0, Value: 0}}, }, } - ss := matrixToSeriesSet(matrix) + ss := MatrixToSeriesSet(matrix) require.True(t, ss.Next()) require.NoError(t, ss.Err()) From a509b57a87e2336ab26936995ab39bf7567be54e Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 16 Oct 2019 04:49:01 -0400 Subject: [PATCH 14/97] wiring for multiple sharding configs Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 72 ++++++++++-- pkg/querier/querysharding/middleware_test.go | 111 ++++++++++++++++++- 2 files changed, 174 insertions(+), 9 deletions(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index b4902a2503a..cc51592388f 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -2,42 +2,98 @@ package querysharding import ( "context" + "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/astmapper" "github.com/cortexproject/cortex/pkg/querier/queryrange" + "github.com/pkg/errors" "github.com/prometheus/prometheus/promql" "time" ) const ( downStreamErrType = "downstream error" - parseErrType = "parse error" ) -func QueryShardMiddleware(engine *promql.Engine) queryrange.Middleware { +var ( + invalidShardingRange = errors.New("Query does not fit in a single sharding configuration") +) + +// Sharding configuration. Will eventually support ranges like chunk.PeriodConfig for meshing togethe multiple queryShard(s). +// This will enable compatibility for deployments which change their shard factors over time. +type ShardingConfig struct { + From chunk.DayTime `yaml:"from"` + Shards int `yaml:"shards"` +} + +type ShardingConfigs []ShardingConfig + +func (confs ShardingConfigs) ValidRange(start, end int64) (ShardingConfig, error) { + for i, conf := range confs { + if start < int64(conf.From.Time) { + // the query starts before this config's range + return ShardingConfig{}, invalidShardingRange + } else if i == len(confs)-1 { + // the last configuration has no upper bound + return conf, nil + } else if end < int64(confs[i+1].From.Time) { + // The request is entirely scoped into this shard config + return conf, nil + } else { + continue + } + } + + return ShardingConfig{}, invalidShardingRange + +} + +func mapQuery(mapper astmapper.ASTMapper, query string) (promql.Node, error) { + expr, err := promql.ParseExpr(query) + if err != nil { + return nil, err + } + return mapper.Map(expr) +} + +func QueryShardMiddleware(engine *promql.Engine, confs ShardingConfigs) queryrange.Middleware { return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { return &queryShard{ + confs: confs, next: next, engine: engine, - mapper: astmapper.NewMultiMapper( - astmapper.NewShardSummer(astmapper.DEFAULT_SHARDS, astmapper.VectorSquasher), - astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), - ), } }) } type queryShard struct { + confs ShardingConfigs next queryrange.Handler engine *promql.Engine - mapper astmapper.ASTMapper } func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrange.APIResponse, error) { queryable := &DownstreamQueryable{r, qs.next} + conf, err := qs.confs.ValidRange(r.Start, r.End) + if err != nil { + return nil, err + } + + mappedQuery, err := mapQuery( + astmapper.NewMultiMapper( + astmapper.NewShardSummer(conf.Shards, astmapper.VectorSquasher), + astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), + ), + r.Query, + ) + + if err != nil { + return nil, err + } + qry, err := qs.engine.NewRangeQuery( queryable, - r.Query, + mappedQuery.String(), time.Unix(r.Start, 0), time.Unix(r.End, 0), time.Duration(r.Step)*time.Second, ) diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index 3c558dc85b2..e8a8ae4725e 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -2,10 +2,12 @@ package querysharding import ( "context" + "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/cortexproject/cortex/pkg/util" "github.com/pkg/errors" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" "testing" @@ -83,7 +85,14 @@ func TestMiddleware(t *testing.T) { Timeout: time.Minute, }) - handler := QueryShardMiddleware(engine).Wrap(c.next) + handler := QueryShardMiddleware( + engine, + ShardingConfigs{ + { + Shards: 3, + }, + }, + ).Wrap(c.next) // escape hatch for custom tests if c.override != nil { @@ -168,3 +177,103 @@ func defaultReq() *queryrange.Request { Query: `__embedded_query__{__cortex_query__="687474705f72657175657374735f746f74616c7b636c75737465723d2270726f64227d"}`, } } + +func TestShardingConfigs_ValidRange(t *testing.T) { + reqWith := func(start, end string) *queryrange.Request { + r := defaultReq() + + if start != "" { + r.Start = int64(parseDate(start)) + } + + if end != "" { + r.End = int64(parseDate(end)) + } + + return r + } + + var testExpr = []struct { + name string + confs ShardingConfigs + req *queryrange.Request + expected ShardingConfig + err error + }{ + { + name: "0 ln configs fail", + confs: ShardingConfigs{}, + req: defaultReq(), + err: invalidShardingRange, + }, + { + name: "request starts before beginning config", + confs: ShardingConfigs{ + { + From: chunk.DayTime{parseDate("2019-10-16")}, + Shards: 1, + }, + }, + req: reqWith("2019-10-15", ""), + err: invalidShardingRange, + }, + { + name: "request spans multiple configs", + confs: ShardingConfigs{ + { + From: chunk.DayTime{parseDate("2019-10-16")}, + Shards: 1, + }, + { + From: chunk.DayTime{parseDate("2019-11-16")}, + Shards: 2, + }, + }, + req: reqWith("2019-10-15", "2019-11-17"), + err: invalidShardingRange, + }, + { + name: "selects correct config ", + confs: ShardingConfigs{ + { + From: chunk.DayTime{parseDate("2019-10-16")}, + Shards: 1, + }, + { + From: chunk.DayTime{parseDate("2019-11-16")}, + Shards: 2, + }, + { + From: chunk.DayTime{parseDate("2019-12-16")}, + Shards: 3, + }, + }, + req: reqWith("2019-11-20", "2019-11-25"), + expected: ShardingConfig{ + From: chunk.DayTime{parseDate("2019-11-16")}, + Shards: 2, + }, + }, + } + + for _, c := range testExpr { + t.Run(c.name, func(t *testing.T) { + out, err := c.confs.ValidRange(c.req.Start, c.req.End) + + if c.err != nil { + require.EqualError(t, err, c.err.Error()) + } else { + require.Nil(t, err) + require.Equal(t, c.expected, out) + } + }) + } +} + +func parseDate(in string) model.Time { + t, err := time.Parse("2006-01-02", in) + if err != nil { + panic(err) + } + return model.Time(t.UnixNano()) +} From 1ec82a00702e43f6ddfb37949fa7a91ed431c4c5 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Sat, 19 Oct 2019 08:59:45 -0400 Subject: [PATCH 15/97] NodeMapper ASTMapper adapter which requires less verbose impls Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/astmapper.go | 101 ++++++++++++++++++++++++ pkg/querier/astmapper/embedded.go | 88 +-------------------- pkg/querier/astmapper/embedded_test.go | 3 +- pkg/querier/querysharding/middleware.go | 2 +- 4 files changed, 107 insertions(+), 87 deletions(-) diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go index 38444264c05..a41288c6f0d 100644 --- a/pkg/querier/astmapper/astmapper.go +++ b/pkg/querier/astmapper/astmapper.go @@ -54,3 +54,104 @@ func NewMultiMapper(xs ...ASTMapper) *MultiMapper { func CloneNode(node promql.Node) (promql.Node, error) { return promql.ParseExpr(node.String()) } + +// NodeMapper is an ASTMapper adapter which recursively evaluates mapped subtrees. +// This is helpful because it allows mappers to only implement logic for node types they want to change. +// It makes some mappers trivially easy to implement (see Squash from embedded.go) +type NodeMapper struct { + mapper ASTMapper +} + +func (nm *NodeMapper) Map(node promql.Node) (promql.Node, error) { + node, err := nm.mapper.Map(node) + + if err != nil { + return nil, err + } + + switch n := node.(type) { + case nil: + // nil handles cases where we check optional fields that are not set + return nil, nil + + case promql.Expressions: + for i, e := range n { + if mapped, err := nm.Map(e); err != nil { + return nil, err + } else { + n[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.AggregateExpr: + expr, err := nm.Map(n.Expr) + if err != nil { + return nil, err + } + n.Expr = expr.(promql.Expr) + return n, nil + + case *promql.BinaryExpr: + if lhs, err := nm.Map(n.LHS); err != nil { + return nil, err + } else { + n.LHS = lhs.(promql.Expr) + } + + if rhs, err := nm.Map(n.RHS); err != nil { + return nil, err + } else { + n.RHS = rhs.(promql.Expr) + } + return n, nil + + case *promql.Call: + for i, e := range n.Args { + if mapped, err := nm.Map(e); err != nil { + return nil, err + } else { + n.Args[i] = mapped.(promql.Expr) + } + } + return n, nil + + case *promql.SubqueryExpr: + if mapped, err := nm.Map(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.ParenExpr: + if mapped, err := nm.Map(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.UnaryExpr: + if mapped, err := nm.Map(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.EvalStmt: + if mapped, err := nm.Map(n.Expr); err != nil { + return nil, err + } else { + n.Expr = mapped.(promql.Expr) + } + return n, nil + + case *promql.NumberLiteral, *promql.StringLiteral, *promql.VectorSelector, *promql.MatrixSelector: + return n, nil + + default: + panic(errors.Errorf("NodeMapper: unhandled node type %T", node)) + } +} diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go index 956dab48b26..f3a35e7f0bb 100644 --- a/pkg/querier/astmapper/embedded.go +++ b/pkg/querier/astmapper/embedded.go @@ -2,7 +2,6 @@ package astmapper import ( "encoding/hex" - "github.com/pkg/errors" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "time" @@ -62,90 +61,11 @@ func VectorSquasher(node promql.Node) (promql.Expr, error) { // ShallowEmbedSelectors encodes selector queries if they do not already have the EMBEDDED_QUERY_FLAG. // This is primarily useful for deferring query execution. -func ShallowEmbedSelectors(node promql.Node) (promql.Node, error) { +var ShallowEmbedSelectors = &NodeMapper{MapperFunc(shallowEmbedSelectors)} - switch n := node.(type) { - case nil: - // nil handles cases where we check optional fields that are not set - return nil, nil - - case promql.Expressions: - for i, e := range n { - if mapped, err := ShallowEmbedSelectors(e); err != nil { - return nil, err - } else { - n[i] = mapped.(promql.Expr) - } - } - return n, nil - - case *promql.AggregateExpr: - expr, err := ShallowEmbedSelectors(n.Expr) - if err != nil { - return nil, err - } - n.Expr = expr.(promql.Expr) - return n, nil - - case *promql.BinaryExpr: - if lhs, err := ShallowEmbedSelectors(n.LHS); err != nil { - return nil, err - } else { - n.LHS = lhs.(promql.Expr) - } - - if rhs, err := ShallowEmbedSelectors(n.RHS); err != nil { - return nil, err - } else { - n.RHS = rhs.(promql.Expr) - } - return n, nil - - case *promql.Call: - for i, e := range n.Args { - if mapped, err := ShallowEmbedSelectors(e); err != nil { - return nil, err - } else { - n.Args[i] = mapped.(promql.Expr) - } - } - return n, nil - - case *promql.SubqueryExpr: - if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.ParenExpr: - if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.UnaryExpr: - if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.EvalStmt: - if mapped, err := ShallowEmbedSelectors(n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.NumberLiteral, *promql.StringLiteral: - return n, nil +func shallowEmbedSelectors(node promql.Node) (promql.Node, error) { + switch n := node.(type) { case *promql.VectorSelector: if n.Name == EMBEDDED_QUERY_FLAG { return n, nil @@ -159,6 +79,6 @@ func ShallowEmbedSelectors(node promql.Node) (promql.Node, error) { return Squash(n, true) default: - panic(errors.Errorf("ShallowEmbedSelectors: unhandled node type %T", node)) + return n, nil } } diff --git a/pkg/querier/astmapper/embedded_test.go b/pkg/querier/astmapper/embedded_test.go index 04f94083259..95fd1c365bd 100644 --- a/pkg/querier/astmapper/embedded_test.go +++ b/pkg/querier/astmapper/embedded_test.go @@ -55,10 +55,9 @@ func TestShallowEmbedSelectors(t *testing.T) { for i, c := range testExpr { t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { - mapper := MapperFunc(ShallowEmbedSelectors) expr, err := promql.ParseExpr(c.input) require.Nil(t, err) - res, err := mapper.Map(expr) + res, err := ShallowEmbedSelectors.Map(expr) require.Nil(t, err) require.Equal(t, c.expected, res.String()) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index cc51592388f..cf11205d32d 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -82,7 +82,7 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang mappedQuery, err := mapQuery( astmapper.NewMultiMapper( astmapper.NewShardSummer(conf.Shards, astmapper.VectorSquasher), - astmapper.MapperFunc(astmapper.ShallowEmbedSelectors), + astmapper.ShallowEmbedSelectors, ), r.Query, ) From cd35d76511140f91ae8264db70a1d06245110484 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Sat, 19 Oct 2019 14:21:51 -0400 Subject: [PATCH 16/97] adds NodeMapper ifc to simplify ASTMapper impls Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/astmapper.go | 34 ++++- pkg/querier/astmapper/embedded.go | 20 +-- pkg/querier/astmapper/embedded_test.go | 3 +- pkg/querier/astmapper/shard_summer.go | 148 +++++---------------- pkg/querier/astmapper/shard_summer_test.go | 2 +- 5 files changed, 74 insertions(+), 133 deletions(-) diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go index a41288c6f0d..00979193314 100644 --- a/pkg/querier/astmapper/astmapper.go +++ b/pkg/querier/astmapper/astmapper.go @@ -55,20 +55,40 @@ func CloneNode(node promql.Node) (promql.Node, error) { return promql.ParseExpr(node.String()) } -// NodeMapper is an ASTMapper adapter which recursively evaluates mapped subtrees. +// a NodeMapper either maps a single AST node or returns the unaltered node. +// It also returns a bool to signal that no further recursion is necessary. // This is helpful because it allows mappers to only implement logic for node types they want to change. -// It makes some mappers trivially easy to implement (see Squash from embedded.go) -type NodeMapper struct { - mapper ASTMapper +// It makes some mappers trivially easy to implement (see ShallowEmbedSelectors from embedded.go) +type NodeMapper interface { + MapNode(node promql.Node) (mapped promql.Node, err error, finished bool) } -func (nm *NodeMapper) Map(node promql.Node) (promql.Node, error) { - node, err := nm.mapper.Map(node) +type NodeMapperFunc func(node promql.Node) (promql.Node, error, bool) + +func (f NodeMapperFunc) MapNode(node promql.Node) (promql.Node, error, bool) { + return f(node) +} + +func NewNodeMapper(mapper NodeMapper) *nodeMapper { + return &nodeMapper{mapper} +} + +// nodeMapper is an ASTMapper adapter which uses a NodeMapper internally. +type nodeMapper struct { + NodeMapper +} + +func (nm *nodeMapper) Map(node promql.Node) (promql.Node, error) { + node, err, fin := nm.MapNode(node) if err != nil { return nil, err } + if fin { + return node, nil + } + switch n := node.(type) { case nil: // nil handles cases where we check optional fields that are not set @@ -152,6 +172,6 @@ func (nm *NodeMapper) Map(node promql.Node) (promql.Node, error) { return n, nil default: - panic(errors.Errorf("NodeMapper: unhandled node type %T", node)) + panic(errors.Errorf("nodeMapper: unhandled node type %T", node)) } } diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go index f3a35e7f0bb..21f9db87a60 100644 --- a/pkg/querier/astmapper/embedded.go +++ b/pkg/querier/astmapper/embedded.go @@ -27,8 +27,7 @@ const ( EMBEDDED_QUERY_FLAG = "__embedded_query__" ) -// Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. The important part is that return types align. -// TODO(owen): handle inferring return types from different functions/operators +// Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { // promql's label charset is not a subset of promql's syntax charset. Therefor we use hex as an intermediary encoded := hex.EncodeToString([]byte(node.String())) @@ -61,24 +60,25 @@ func VectorSquasher(node promql.Node) (promql.Expr, error) { // ShallowEmbedSelectors encodes selector queries if they do not already have the EMBEDDED_QUERY_FLAG. // This is primarily useful for deferring query execution. -var ShallowEmbedSelectors = &NodeMapper{MapperFunc(shallowEmbedSelectors)} - -func shallowEmbedSelectors(node promql.Node) (promql.Node, error) { +var ShallowEmbedSelectors = NewNodeMapper(NodeMapperFunc(shallowEmbedSelectors)) +func shallowEmbedSelectors(node promql.Node) (promql.Node, error, bool) { switch n := node.(type) { case *promql.VectorSelector: if n.Name == EMBEDDED_QUERY_FLAG { - return n, nil + return n, nil, true } - return Squash(n, false) + squashed, err := Squash(n, false) + return squashed, err, true case *promql.MatrixSelector: if n.Name == EMBEDDED_QUERY_FLAG { - return n, nil + return n, nil, true } - return Squash(n, true) + squashed, err := Squash(n, true) + return squashed, err, true default: - return n, nil + return n, nil, false } } diff --git a/pkg/querier/astmapper/embedded_test.go b/pkg/querier/astmapper/embedded_test.go index 95fd1c365bd..fa43d191992 100644 --- a/pkg/querier/astmapper/embedded_test.go +++ b/pkg/querier/astmapper/embedded_test.go @@ -55,9 +55,10 @@ func TestShallowEmbedSelectors(t *testing.T) { for i, c := range testExpr { t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + mapper := ShallowEmbedSelectors expr, err := promql.ParseExpr(c.input) require.Nil(t, err) - res, err := ShallowEmbedSelectors.Map(expr) + res, err := mapper.Map(expr) require.Nil(t, err) require.Equal(t, c.expected, res.String()) diff --git a/pkg/querier/astmapper/shard_summer.go b/pkg/querier/astmapper/shard_summer.go index 5db48efc879..3cb33d40c4a 100644 --- a/pkg/querier/astmapper/shard_summer.go +++ b/pkg/querier/astmapper/shard_summer.go @@ -2,7 +2,6 @@ package astmapper import ( "fmt" - "github.com/pkg/errors" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" ) @@ -14,144 +13,66 @@ const ( type squasher = func(promql.Node) (promql.Expr, error) -type ShardSummer struct { - shards int - squash squasher +type shardSummer struct { + shards int + curshard *int + squash squasher } -func NewShardSummer(shards int, squasher func(promql.Node) (promql.Expr, error)) *ShardSummer { +func NewShardSummer(shards int, squasher squasher) ASTMapper { if shards == 0 { shards = DEFAULT_SHARDS } - return &ShardSummer{shards, squasher} + return NewNodeMapper(&shardSummer{ + shards: shards, + squash: squasher, + curshard: nil, + }) } -// a MapperFunc adapter -func ShardSummerFunc(shards int, squash squasher) MapperFunc { - summer := NewShardSummer(shards, squash) - - return summer.Map -} - -// ShardSummer expands a query AST by sharding and re-summing when possible -func (summer *ShardSummer) Map(node promql.Node) (promql.Node, error) { - return summer.mapWithOpts(MappingOpts{}, node) -} - -type MappingOpts struct { - isParallel bool - // curShard's ptr denotes existence. This helps prevent selectors in non-sum queries from sharding themselves as they'll always register true for isParallel. - curShard *int +// CopyWithCurshard clones a shardSummer with a new current shard. This facilitates recursive sharding. +func (summer *shardSummer) CopyWithCurshard(curshard int) *shardSummer { + s := *summer + s.curshard = &curshard + return &s } -// mapWithOpts carries information about whether the node is in a subtree that is a parallelism candidate. -func (summer *ShardSummer) mapWithOpts(opts MappingOpts, node promql.Node) (promql.Node, error) { - // since mapWithOpts is called recursively, the new subtree may be parallelizable even if its parent is not - opts.isParallel = opts.isParallel || CanParallel(node) +// shardSummer expands a query AST by sharding and re-summing when possible +func (summer *shardSummer) MapNode(node promql.Node) (promql.Node, error, bool) { switch n := node.(type) { - case promql.Expressions: - for i, e := range n { - if mapped, err := summer.mapWithOpts(opts, e); err != nil { - return nil, err - } else { - n[i] = mapped.(promql.Expr) - } - } - return n, nil - case *promql.AggregateExpr: - if opts.isParallel { - return summer.shardSum(n) - } - - if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.BinaryExpr: - if lhs, err := summer.mapWithOpts(opts, n.LHS); err != nil { - return nil, err - } else { - n.LHS = lhs.(promql.Expr) - } - - if rhs, err := summer.mapWithOpts(opts, n.RHS); err != nil { - return nil, err - } else { - n.RHS = rhs.(promql.Expr) - } - return n, nil - - case *promql.Call: - for i, e := range n.Args { - if mapped, err := summer.mapWithOpts(opts, e); err != nil { - return nil, err - } else { - n.Args[i] = mapped.(promql.Expr) - } - } - return n, nil - - case *promql.SubqueryExpr: - if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.ParenExpr: - if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.UnaryExpr: - if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) - } - return n, nil - - case *promql.EvalStmt: - if mapped, err := summer.mapWithOpts(opts, n.Expr); err != nil { - return nil, err - } else { - n.Expr = mapped.(promql.Expr) + if CanParallel(n) { + result, err := summer.shardSum(n) + return result, err, true } - return n, nil - case *promql.NumberLiteral, *promql.StringLiteral: - return n, nil + return n, nil, false case *promql.VectorSelector: - if opts.isParallel && opts.curShard != nil { - return shardVectorSelector(*opts.curShard, summer.shards, n) + if summer.curshard != nil { + mapped, err := shardVectorSelector(*summer.curshard, summer.shards, n) + return mapped, err, true } else { - return n, nil + return n, nil, true } case *promql.MatrixSelector: - if opts.isParallel && opts.curShard != nil { - return shardMatrixSelector(*opts.curShard, summer.shards, n) + if summer.curshard != nil { + mapped, err := shardMatrixSelector(*summer.curshard, summer.shards, n) + return mapped, err, true } else { - return n, nil + return n, nil, true } default: - panic(errors.Errorf("ShardSummer: unhandled node type %T", node)) + return n, nil, false } } -func (summer *ShardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, error) { +// shardSum contains the logic for how we split/stitch legs of a parallelized query +func (summer *shardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, error) { if summer.shards < 2 { return expr, nil @@ -165,9 +86,8 @@ func (summer *ShardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, er return nil, err } - sharded, err := summer.mapWithOpts(MappingOpts{ - curShard: &i, - }, cloned) + subSummer := NewNodeMapper(summer.CopyWithCurshard(i)) + sharded, err := subSummer.Map(cloned) if err != nil { return nil, err } diff --git a/pkg/querier/astmapper/shard_summer_test.go b/pkg/querier/astmapper/shard_summer_test.go index 86c92ba4d30..30e4d3750e8 100644 --- a/pkg/querier/astmapper/shard_summer_test.go +++ b/pkg/querier/astmapper/shard_summer_test.go @@ -41,7 +41,7 @@ func TestShardSummer(t *testing.T) { ) )`, }, - // This is currently redundant: sums split into sharded versions, including summed sums. + // This is currently redundant but still equivalent: sums split into sharded versions, including summed sums. { 2, `sum(sum by(foo) (rate(bar1{baz="blip"}[1m])))`, From c27c58dee9c43373b0d1fbe0c9766a9ba34cfbe8 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 10:13:52 -0400 Subject: [PATCH 17/97] queries assume milliseconds Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 16 +++++++++-- pkg/querier/querysharding/middleware_test.go | 30 ++++++++++++++++---- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index cf11205d32d..9e9da8d00b8 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -2,12 +2,13 @@ package querysharding import ( "context" + "time" + "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/astmapper" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/pkg/errors" "github.com/prometheus/prometheus/promql" - "time" ) const ( @@ -15,6 +16,8 @@ const ( ) var ( + nanosecondsInMillisecond = int64(time.Millisecond / time.Nanosecond) + invalidShardingRange = errors.New("Query does not fit in a single sharding configuration") ) @@ -94,8 +97,9 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang qry, err := qs.engine.NewRangeQuery( queryable, mappedQuery.String(), - time.Unix(r.Start, 0), time.Unix(r.End, 0), - time.Duration(r.Step)*time.Second, + TimeFromMillis(r.Start), + TimeFromMillis(r.End), + time.Duration(r.Step)*time.Millisecond, ) if err != nil { @@ -130,3 +134,9 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang }, nil } } + +func TimeFromMillis(ms int64) time.Time { + secs := ms / 1000 + rem := ms - (secs * 1000) + return time.Unix(secs, rem*nanosecondsInMillisecond) +} diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index e8a8ae4725e..d152ade6fe7 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -2,6 +2,9 @@ package querysharding import ( "context" + "testing" + "time" + "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" @@ -10,8 +13,6 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" - "time" ) func TestMiddleware(t *testing.T) { @@ -126,11 +127,11 @@ func sampleMatrixResponse() *queryrange.APIResponse { }, Samples: []client.Sample{ client.Sample{ - TimestampMs: 5000, + TimestampMs: 5, Value: 1, }, client.Sample{ - TimestampMs: 10000, + TimestampMs: 10, Value: 2, }, }, @@ -142,11 +143,11 @@ func sampleMatrixResponse() *queryrange.APIResponse { }, Samples: []client.Sample{ client.Sample{ - TimestampMs: 5000, + TimestampMs: 5, Value: 8, }, client.Sample{ - TimestampMs: 10000, + TimestampMs: 10, Value: 9, }, }, @@ -270,6 +271,23 @@ func TestShardingConfigs_ValidRange(t *testing.T) { } } +func TestTimeFromMillis(t *testing.T) { + var testExpr = []struct { + input int64 + expected time.Time + }{ + {1000, time.Unix(1, 0)}, + {1500, time.Unix(1, 500*nanosecondsInMillisecond)}, + } + + for i, c := range testExpr { + t.Run(string(i), func(t *testing.T) { + res := TimeFromMillis(c.input) + require.Equal(t, c.expected, res) + }) + } +} + func parseDate(in string) model.Time { t, err := time.Parse("2006-01-02", in) if err != nil { From 5e9c0d4043f3ab2a333e644e9564c4c2ce7479cd Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 14:40:04 -0400 Subject: [PATCH 18/97] dont shard requests across multiple shard configs Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 9e9da8d00b8..bf37bbc5249 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -78,8 +78,9 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang queryable := &DownstreamQueryable{r, qs.next} conf, err := qs.confs.ValidRange(r.Start, r.End) + // query exists across multiple sharding configs, so don't try to do AST mapping. if err != nil { - return nil, err + return qs.next.Do(ctx, r) } mappedQuery, err := mapQuery( From a344f4877e8b22c1591290ab6db9c1c65b766a6d Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 15:16:03 -0400 Subject: [PATCH 19/97] lazy queryable and chunkstore to their own packages Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/chunk_store_queryable.go | 5 ++- pkg/querier/chunk_tar_test.go | 5 ++- pkg/querier/chunks_handler.go | 3 +- pkg/querier/chunkstore/chunkstore.go | 15 +++++++ .../lazyquery.go} | 40 ++++++++++++++----- pkg/querier/querier.go | 13 ++---- pkg/querier/unified_querier.go | 9 +++-- 7 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 pkg/querier/chunkstore/chunkstore.go rename pkg/querier/{lazy_querier.go => lazyquery/lazyquery.go} (65%) diff --git a/pkg/querier/chunk_store_queryable.go b/pkg/querier/chunk_store_queryable.go index 8c02d238e62..1f16b4e310f 100644 --- a/pkg/querier/chunk_store_queryable.go +++ b/pkg/querier/chunk_store_queryable.go @@ -11,12 +11,13 @@ import ( "github.com/weaveworks/common/user" "github.com/cortexproject/cortex/pkg/chunk" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" seriesset "github.com/cortexproject/cortex/pkg/querier/series" ) type chunkIteratorFunc func(chunks []chunk.Chunk, from, through model.Time) storage.SeriesIterator -func newChunkStoreQueryable(store ChunkStore, chunkIteratorFunc chunkIteratorFunc) storage.Queryable { +func newChunkStoreQueryable(store chunkstore.ChunkStore, chunkIteratorFunc chunkIteratorFunc) storage.Queryable { return storage.QueryableFunc(func(ctx context.Context, mint, maxt int64) (storage.Querier, error) { return &chunkStoreQuerier{ store: store, @@ -29,7 +30,7 @@ func newChunkStoreQueryable(store ChunkStore, chunkIteratorFunc chunkIteratorFun } type chunkStoreQuerier struct { - store ChunkStore + store chunkstore.ChunkStore chunkIteratorFunc chunkIteratorFunc ctx context.Context mint, maxt int64 diff --git a/pkg/querier/chunk_tar_test.go b/pkg/querier/chunk_tar_test.go index fab91013af5..608c2297545 100644 --- a/pkg/querier/chunk_tar_test.go +++ b/pkg/querier/chunk_tar_test.go @@ -13,6 +13,7 @@ import ( "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/batch" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" "github.com/cortexproject/cortex/pkg/util" "github.com/pkg/errors" "github.com/prometheus/prometheus/promql" @@ -20,7 +21,7 @@ import ( "github.com/weaveworks/common/user" ) -func getTarDataFromEnv(t testing.TB) (query string, from, through time.Time, step time.Duration, store ChunkStore) { +func getTarDataFromEnv(t testing.TB) (query string, from, through time.Time, step time.Duration, store chunkstore.ChunkStore) { var ( err error chunksFilename = os.Getenv("CHUNKS") @@ -47,7 +48,7 @@ func getTarDataFromEnv(t testing.TB) (query string, from, through time.Time, ste return query, from, through, step, &mockChunkStore{chunks} } -func runRangeQuery(t testing.TB, query string, from, through time.Time, step time.Duration, store ChunkStore) { +func runRangeQuery(t testing.TB, query string, from, through time.Time, step time.Duration, store chunkstore.ChunkStore) { if len(query) == 0 || store == nil { return } diff --git a/pkg/querier/chunks_handler.go b/pkg/querier/chunks_handler.go index d91ed8eb58c..ea694ae12b0 100644 --- a/pkg/querier/chunks_handler.go +++ b/pkg/querier/chunks_handler.go @@ -10,6 +10,7 @@ import ( "github.com/prometheus/prometheus/storage" "github.com/weaveworks/common/user" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" "github.com/cortexproject/cortex/pkg/querier/queryrange" ) @@ -48,7 +49,7 @@ func ChunksHandler(queryable storage.Queryable) http.Handler { return } - store, ok := querier.(ChunkStore) + store, ok := querier.(chunkstore.ChunkStore) if !ok { http.Error(w, "not supported", http.StatusServiceUnavailable) return diff --git a/pkg/querier/chunkstore/chunkstore.go b/pkg/querier/chunkstore/chunkstore.go new file mode 100644 index 00000000000..3b1045ef0d6 --- /dev/null +++ b/pkg/querier/chunkstore/chunkstore.go @@ -0,0 +1,15 @@ +package chunkstore + +import ( + "context" + + "github.com/cortexproject/cortex/pkg/chunk" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/pkg/labels" +) + +// ChunkStore is the read-interface to the Chunk Store. Made an interface here +// to reduce package coupling. +type ChunkStore interface { + Get(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) ([]chunk.Chunk, error) +} diff --git a/pkg/querier/lazy_querier.go b/pkg/querier/lazyquery/lazyquery.go similarity index 65% rename from pkg/querier/lazy_querier.go rename to pkg/querier/lazyquery/lazyquery.go index 6b0e2107d95..e5f35e78d3d 100644 --- a/pkg/querier/lazy_querier.go +++ b/pkg/querier/lazyquery/lazyquery.go @@ -1,26 +1,44 @@ -package querier +package lazyquery import ( "context" "fmt" "github.com/cortexproject/cortex/pkg/chunk" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage" ) -type lazyQuerier struct { +type LazyQueryable struct { + q storage.Queryable +} + +func (lq LazyQueryable) Querier(ctx context.Context, mint, maxt int64) (storage.Querier, error) { + q, err := lq.q.Querier(ctx, mint, maxt) + if err != nil { + return nil, err + } + + return NewLazyQuerier(q), nil +} + +func NewLazyQueryable(q storage.Queryable) storage.Queryable { + return LazyQueryable{q} +} + +type LazyQuerier struct { next storage.Querier } -// newLazyQuerier wraps a storage.Querier, does the Select in the background. +// NewLazyQuerier wraps a storage.Querier, does the Select in the background. // Return value cannot be used from more than one goroutine simultaneously. -func newLazyQuerier(next storage.Querier) storage.Querier { - return lazyQuerier{next} +func NewLazyQuerier(next storage.Querier) storage.Querier { + return LazyQuerier{next} } -func (l lazyQuerier) Select(params *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { +func (l LazyQuerier) Select(params *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { future := make(chan storage.SeriesSet) go func() { set, _, err := l.next.Select(params, matchers...) @@ -35,21 +53,21 @@ func (l lazyQuerier) Select(params *storage.SelectParams, matchers ...*labels.Ma }, nil, nil } -func (l lazyQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { +func (l LazyQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { return l.next.LabelValues(name) } -func (l lazyQuerier) LabelNames() ([]string, storage.Warnings, error) { +func (l LazyQuerier) LabelNames() ([]string, storage.Warnings, error) { return l.next.LabelNames() } -func (l lazyQuerier) Close() error { +func (l LazyQuerier) Close() error { return l.next.Close() } // Get implements ChunkStore for the chunk tar HTTP handler. -func (l lazyQuerier) Get(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) ([]chunk.Chunk, error) { - store, ok := l.next.(ChunkStore) +func (l LazyQuerier) Get(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) ([]chunk.Chunk, error) { + store, ok := l.next.(chunkstore.ChunkStore) if !ok { return nil, fmt.Errorf("not supported") } diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index 5e6b30707fe..49b6818b820 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -11,9 +11,10 @@ import ( "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/storage" - "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/batch" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" "github.com/cortexproject/cortex/pkg/querier/iterators" + "github.com/cortexproject/cortex/pkg/querier/lazyquery" seriesset "github.com/cortexproject/cortex/pkg/querier/series" "github.com/cortexproject/cortex/pkg/util" ) @@ -53,14 +54,8 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { cfg.metricsRegisterer = prometheus.DefaultRegisterer } -// ChunkStore is the read-interface to the Chunk Store. Made an interface here -// to reduce package coupling. -type ChunkStore interface { - Get(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) ([]chunk.Chunk, error) -} - // New builds a queryable and promql engine. -func New(cfg Config, distributor Distributor, chunkStore ChunkStore) (storage.Queryable, *promql.Engine) { +func New(cfg Config, distributor Distributor, chunkStore chunkstore.ChunkStore) (storage.Queryable, *promql.Engine) { iteratorFunc := mergeChunks if cfg.BatchIterators { iteratorFunc = batch.NewChunkMergeIterator @@ -83,7 +78,7 @@ func New(cfg Config, distributor Distributor, chunkStore ChunkStore) (storage.Qu if err != nil { return nil, err } - return newLazyQuerier(querier), nil + return lazyquery.NewLazyQuerier(querier), nil }) promql.SetDefaultEvaluationInterval(cfg.DefaultEvaluationInterval) diff --git a/pkg/querier/unified_querier.go b/pkg/querier/unified_querier.go index 16b12b0d338..1173e630305 100644 --- a/pkg/querier/unified_querier.go +++ b/pkg/querier/unified_querier.go @@ -10,12 +10,13 @@ import ( "github.com/weaveworks/common/user" "github.com/cortexproject/cortex/pkg/chunk" + "github.com/cortexproject/cortex/pkg/querier/chunkstore" ) -func newUnifiedChunkQueryable(ds, cs ChunkStore, distributor Distributor, chunkIteratorFunc chunkIteratorFunc, ingesterMaxQueryLookback time.Duration) storage.Queryable { +func newUnifiedChunkQueryable(ds, cs chunkstore.ChunkStore, distributor Distributor, chunkIteratorFunc chunkIteratorFunc, ingesterMaxQueryLookback time.Duration) storage.Queryable { return storage.QueryableFunc(func(ctx context.Context, mint, maxt int64) (storage.Querier, error) { ucq := &unifiedChunkQuerier{ - stores: []ChunkStore{cs}, + stores: []chunkstore.ChunkStore{cs}, querier: querier{ ctx: ctx, mint: mint, @@ -40,7 +41,7 @@ func newUnifiedChunkQueryable(ds, cs ChunkStore, distributor Distributor, chunkI } type unifiedChunkQuerier struct { - stores []ChunkStore + stores []chunkstore.ChunkStore // We reuse metadataQuery, LabelValues and Close from querier. querier @@ -53,7 +54,7 @@ func (q *unifiedChunkQuerier) Get(ctx context.Context, userID string, from, thro css := make(chan []chunk.Chunk, len(q.stores)) errs := make(chan error, len(q.stores)) for _, store := range q.stores { - go func(store ChunkStore) { + go func(store chunkstore.ChunkStore) { cs, err := store.Get(ctx, userID, from, through, matchers...) if err != nil { errs <- err From 039eefc06dcbbe09a238ecbe75d9894aeaa4e172 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 15:17:47 -0400 Subject: [PATCH 20/97] querysharding uses a lazy querier Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/middleware.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index bf37bbc5249..7583800a2df 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -6,6 +6,7 @@ import ( "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/querier/astmapper" + "github.com/cortexproject/cortex/pkg/querier/lazyquery" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/pkg/errors" "github.com/prometheus/prometheus/promql" @@ -75,7 +76,7 @@ type queryShard struct { } func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrange.APIResponse, error) { - queryable := &DownstreamQueryable{r, qs.next} + queryable := lazyquery.NewLazyQueryable(&DownstreamQueryable{r, qs.next}) conf, err := qs.confs.ValidRange(r.Start, r.End) // query exists across multiple sharding configs, so don't try to do AST mapping. From 4f00f9e3b6d3c5639af69cacc138891e0a71b62e Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 16:23:51 -0400 Subject: [PATCH 21/97] linter fixes Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/astmapper.go | 77 +++++++++++--------- pkg/querier/astmapper/embedded.go | 39 +++++----- pkg/querier/astmapper/parallel.go | 17 ++--- pkg/querier/astmapper/shard_summer.go | 30 ++++---- pkg/querier/queryrange/query_range.go | 4 +- pkg/querier/querysharding/middleware.go | 11 ++- pkg/querier/querysharding/middleware_test.go | 6 +- pkg/querier/querysharding/queryable.go | 15 ++-- pkg/querier/querysharding/queryable_test.go | 15 ++-- pkg/querier/querysharding/series.go | 2 +- pkg/querier/series/series_set.go | 11 +++ 11 files changed, 122 insertions(+), 105 deletions(-) diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go index 00979193314..b2b44734260 100644 --- a/pkg/querier/astmapper/astmapper.go +++ b/pkg/querier/astmapper/astmapper.go @@ -10,12 +10,15 @@ type ASTMapper interface { Map(node promql.Node) (promql.Node, error) } +// MapperFunc is a function adapter for ASTMapper type MapperFunc func(node promql.Node) (promql.Node, error) +// Map applies a mapperfunc as an ASTMapper func (fn MapperFunc) Map(node promql.Node) (promql.Node, error) { return fn(node) } +// MultiMapper can compose multiple ASTMappers type MultiMapper struct { mappers []ASTMapper } @@ -38,39 +41,44 @@ func (m *MultiMapper) Map(node promql.Node) (promql.Node, error) { } -// since registered functions are applied in the order they're registered, it's advised to register them +// Register adds ASTMappers into a multimapper. +// Since registered functions are applied in the order they're registered, it's advised to register them // in decreasing priority and only operate on nodes that each function cares about, defaulting to CloneNode. func (m *MultiMapper) Register(xs ...ASTMapper) { m.mappers = append(m.mappers, xs...) } +// NewMultiMapper instaniates an ASTMapper from multiple ASTMappers func NewMultiMapper(xs ...ASTMapper) *MultiMapper { m := &MultiMapper{} m.Register(xs...) return m } -// helper function to clone a node. +// CloneNode is a helper function to clone a node. func CloneNode(node promql.Node) (promql.Node, error) { return promql.ParseExpr(node.String()) } -// a NodeMapper either maps a single AST node or returns the unaltered node. +// NodeMapper either maps a single AST node or returns the unaltered node. // It also returns a bool to signal that no further recursion is necessary. // This is helpful because it allows mappers to only implement logic for node types they want to change. // It makes some mappers trivially easy to implement (see ShallowEmbedSelectors from embedded.go) type NodeMapper interface { - MapNode(node promql.Node) (mapped promql.Node, err error, finished bool) + MapNode(node promql.Node) (mapped promql.Node, finished bool, err error) } -type NodeMapperFunc func(node promql.Node) (promql.Node, error, bool) +// NodeMapperFunc is an adapter for NodeMapper +type NodeMapperFunc func(node promql.Node) (promql.Node, bool, error) -func (f NodeMapperFunc) MapNode(node promql.Node) (promql.Node, error, bool) { +// MapNode applies a NodeMapperFunc as a NodeMapper +func (f NodeMapperFunc) MapNode(node promql.Node) (promql.Node, bool, error) { return f(node) } -func NewNodeMapper(mapper NodeMapper) *nodeMapper { - return &nodeMapper{mapper} +// NewNodeMapper creates an ASTMapper from a NodeMapper +func NewNodeMapper(mapper NodeMapper) nodeMapper { + return nodeMapper{mapper} } // nodeMapper is an ASTMapper adapter which uses a NodeMapper internally. @@ -78,8 +86,9 @@ type nodeMapper struct { NodeMapper } -func (nm *nodeMapper) Map(node promql.Node) (promql.Node, error) { - node, err, fin := nm.MapNode(node) +// Map impls ASTMapper from a NodeMapper +func (nm nodeMapper) Map(node promql.Node) (promql.Node, error) { + node, fin, err := nm.MapNode(node) if err != nil { return nil, err @@ -96,11 +105,11 @@ func (nm *nodeMapper) Map(node promql.Node) (promql.Node, error) { case promql.Expressions: for i, e := range n { - if mapped, err := nm.Map(e); err != nil { + mapped, err := nm.Map(e) + if err != nil { return nil, err - } else { - n[i] = mapped.(promql.Expr) } + n[i] = mapped.(promql.Expr) } return n, nil @@ -113,59 +122,59 @@ func (nm *nodeMapper) Map(node promql.Node) (promql.Node, error) { return n, nil case *promql.BinaryExpr: - if lhs, err := nm.Map(n.LHS); err != nil { + lhs, err := nm.Map(n.LHS) + if err != nil { return nil, err - } else { - n.LHS = lhs.(promql.Expr) } + n.LHS = lhs.(promql.Expr) - if rhs, err := nm.Map(n.RHS); err != nil { + rhs, err := nm.Map(n.RHS) + if err != nil { return nil, err - } else { - n.RHS = rhs.(promql.Expr) } + n.RHS = rhs.(promql.Expr) return n, nil case *promql.Call: for i, e := range n.Args { - if mapped, err := nm.Map(e); err != nil { + mapped, err := nm.Map(e) + if err != nil { return nil, err - } else { - n.Args[i] = mapped.(promql.Expr) } + n.Args[i] = mapped.(promql.Expr) } return n, nil case *promql.SubqueryExpr: - if mapped, err := nm.Map(n.Expr); err != nil { + mapped, err := nm.Map(n.Expr) + if err != nil { return nil, err - } else { - n.Expr = mapped.(promql.Expr) } + n.Expr = mapped.(promql.Expr) return n, nil case *promql.ParenExpr: - if mapped, err := nm.Map(n.Expr); err != nil { + mapped, err := nm.Map(n.Expr) + if err != nil { return nil, err - } else { - n.Expr = mapped.(promql.Expr) } + n.Expr = mapped.(promql.Expr) return n, nil case *promql.UnaryExpr: - if mapped, err := nm.Map(n.Expr); err != nil { + mapped, err := nm.Map(n.Expr) + if err != nil { return nil, err - } else { - n.Expr = mapped.(promql.Expr) } + n.Expr = mapped.(promql.Expr) return n, nil case *promql.EvalStmt: - if mapped, err := nm.Map(n.Expr); err != nil { + mapped, err := nm.Map(n.Expr) + if err != nil { return nil, err - } else { - n.Expr = mapped.(promql.Expr) } + n.Expr = mapped.(promql.Expr) return n, nil case *promql.NumberLiteral, *promql.StringLiteral, *promql.VectorSelector, *promql.MatrixSelector: diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go index 21f9db87a60..737b0d77808 100644 --- a/pkg/querier/astmapper/embedded.go +++ b/pkg/querier/astmapper/embedded.go @@ -2,9 +2,10 @@ package astmapper import ( "encoding/hex" + "time" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" - "time" ) /* @@ -23,8 +24,8 @@ Ideally the promql.Engine could be an interface instead of a concrete type, allo */ const ( - QUERY_LABEL = "__cortex_query__" - EMBEDDED_QUERY_FLAG = "__embedded_query__" + QueryLabel = "__cortex_query__" // reserved label containing an embedded query + EmbeddedQueryFlag = "__embedded_query__" // reserved label (metric name) denoting an embedded query ) // Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. @@ -32,7 +33,7 @@ func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { // promql's label charset is not a subset of promql's syntax charset. Therefor we use hex as an intermediary encoded := hex.EncodeToString([]byte(node.String())) - embedded_query, err := labels.NewMatcher(labels.MatchEqual, QUERY_LABEL, encoded) + embedded_query, err := labels.NewMatcher(labels.MatchEqual, QueryLabel, encoded) if err != nil { return nil, err @@ -40,16 +41,16 @@ func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { if isMatrix { return &promql.MatrixSelector{ - Name: EMBEDDED_QUERY_FLAG, + Name: EmbeddedQueryFlag, Range: time.Minute, LabelMatchers: []*labels.Matcher{embedded_query}, }, nil - } else { - return &promql.VectorSelector{ - Name: EMBEDDED_QUERY_FLAG, - LabelMatchers: []*labels.Matcher{embedded_query}, - }, nil } + + return &promql.VectorSelector{ + Name: EmbeddedQueryFlag, + LabelMatchers: []*labels.Matcher{embedded_query}, + }, nil } // VectorSquasher always uses a VectorSelector as the substitution node. @@ -58,27 +59,27 @@ func VectorSquasher(node promql.Node) (promql.Expr, error) { return Squash(node, false) } -// ShallowEmbedSelectors encodes selector queries if they do not already have the EMBEDDED_QUERY_FLAG. +// ShallowEmbedSelectors encodes selector queries if they do not already have the EmbeddedQueryFlag. // This is primarily useful for deferring query execution. var ShallowEmbedSelectors = NewNodeMapper(NodeMapperFunc(shallowEmbedSelectors)) -func shallowEmbedSelectors(node promql.Node) (promql.Node, error, bool) { +func shallowEmbedSelectors(node promql.Node) (promql.Node, bool, error) { switch n := node.(type) { case *promql.VectorSelector: - if n.Name == EMBEDDED_QUERY_FLAG { - return n, nil, true + if n.Name == EmbeddedQueryFlag { + return n, true, nil } squashed, err := Squash(n, false) - return squashed, err, true + return squashed, true, err case *promql.MatrixSelector: - if n.Name == EMBEDDED_QUERY_FLAG { - return n, nil, true + if n.Name == EmbeddedQueryFlag { + return n, true, nil } squashed, err := Squash(n, true) - return squashed, err, true + return squashed, true, err default: - return n, nil, false + return n, false, nil } } diff --git a/pkg/querier/astmapper/parallel.go b/pkg/querier/astmapper/parallel.go index 40d9abb35d3..76e5697b630 100644 --- a/pkg/querier/astmapper/parallel.go +++ b/pkg/querier/astmapper/parallel.go @@ -5,14 +5,8 @@ import ( "github.com/prometheus/prometheus/promql" ) -// matrixselector & vectorselector need to have shard annotations added -/* -Design: -1) annotate subtrees as parallelizable. A subtree is parallelizable if all of it's components are parallelizable -2) deal with splitting/stiching against queriers after mapping into the parallel-compatible AST -*/ - -// function to annotate subtrees as parallelizable. Inefficient, but illustrative. +// CanParallel annotates subtrees as parallelizable. +// A subtree is parallelizable if all of it's components are parallelizable. func CanParallel(node promql.Node) bool { switch n := node.(type) { case nil: @@ -31,16 +25,15 @@ func CanParallel(node promql.Node) bool { // currently we only support sum for expediency. if n.Op != promql.ItemSum || !CanParallel(n.Param) || !CanParallel(n.Expr) { return false - } else { - return true } + return true case *promql.BinaryExpr: // since binary exprs use each side for merging, they cannot be parallelized return false case *promql.Call: - if !FuncParallel(n.Func) { + if !funcParallel(n.Func) { return false } @@ -75,7 +68,7 @@ func CanParallel(node promql.Node) bool { return false } -func FuncParallel(f *promql.Function) bool { +func funcParallel(f *promql.Function) bool { // flagging all functions as parallel for expediency -- this is certainly not correct (i.e. avg). return true } diff --git a/pkg/querier/astmapper/shard_summer.go b/pkg/querier/astmapper/shard_summer.go index 3cb33d40c4a..827ba111032 100644 --- a/pkg/querier/astmapper/shard_summer.go +++ b/pkg/querier/astmapper/shard_summer.go @@ -2,13 +2,14 @@ package astmapper import ( "fmt" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" ) const ( - DEFAULT_SHARDS = 12 - SHARD_LABEL = "__cortex_shard__" + DefaultShards = 12 // default shard factor to assume + ShardLabel = "__cortex_shard__" // reserved label referencing a cortex shard ) type squasher = func(promql.Node) (promql.Expr, error) @@ -19,9 +20,10 @@ type shardSummer struct { squash squasher } +// NewShardSummer instantiates an ASTMapper which will fan out sums queries by shard func NewShardSummer(shards int, squasher squasher) ASTMapper { if shards == 0 { - shards = DEFAULT_SHARDS + shards = DefaultShards } return NewNodeMapper(&shardSummer{ @@ -39,35 +41,33 @@ func (summer *shardSummer) CopyWithCurshard(curshard int) *shardSummer { } // shardSummer expands a query AST by sharding and re-summing when possible -func (summer *shardSummer) MapNode(node promql.Node) (promql.Node, error, bool) { +func (summer *shardSummer) MapNode(node promql.Node) (promql.Node, bool, error) { switch n := node.(type) { case *promql.AggregateExpr: if CanParallel(n) { result, err := summer.shardSum(n) - return result, err, true + return result, true, err } - return n, nil, false + return n, false, nil case *promql.VectorSelector: if summer.curshard != nil { mapped, err := shardVectorSelector(*summer.curshard, summer.shards, n) - return mapped, err, true - } else { - return n, nil, true + return mapped, true, err } + return n, true, nil case *promql.MatrixSelector: if summer.curshard != nil { mapped, err := shardMatrixSelector(*summer.curshard, summer.shards, n) - return mapped, err, true - } else { - return n, nil, true + return mapped, true, err } + return n, true, nil default: - return n, nil, false + return n, false, nil } } @@ -134,7 +134,7 @@ func (summer *shardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, er } func shardVectorSelector(curshard, shards int, selector *promql.VectorSelector) (promql.Node, error) { - shardMatcher, err := labels.NewMatcher(labels.MatchEqual, SHARD_LABEL, fmt.Sprintf("%d_of_%d", curshard, shards)) + shardMatcher, err := labels.NewMatcher(labels.MatchEqual, ShardLabel, fmt.Sprintf("%d_of_%d", curshard, shards)) if err != nil { return nil, err } @@ -150,7 +150,7 @@ func shardVectorSelector(curshard, shards int, selector *promql.VectorSelector) } func shardMatrixSelector(curshard, shards int, selector *promql.MatrixSelector) (promql.Node, error) { - shardMatcher, err := labels.NewMatcher(labels.MatchEqual, SHARD_LABEL, fmt.Sprintf("%d_of_%d", curshard, shards)) + shardMatcher, err := labels.NewMatcher(labels.MatchEqual, ShardLabel, fmt.Sprintf("%d_of_%d", curshard, shards)) if err != nil { return nil, err } diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index e0ecb705454..94a98c392ba 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -23,8 +23,8 @@ import ( ) const ( - StatusSuccess = "success" - StatusFailure = "failure" + StatusSuccess = "success" // Successful status + StatusFailure = "failure" // Failure status ) var ( diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 7583800a2df..163eab470e5 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -19,23 +19,25 @@ const ( var ( nanosecondsInMillisecond = int64(time.Millisecond / time.Nanosecond) - invalidShardingRange = errors.New("Query does not fit in a single sharding configuration") + errInvalidShardingRange = errors.New("Query does not fit in a single sharding configuration") ) -// Sharding configuration. Will eventually support ranges like chunk.PeriodConfig for meshing togethe multiple queryShard(s). +// ShardingConfig will eventually support ranges like chunk.PeriodConfig for meshing togethe multiple queryShard(s). // This will enable compatibility for deployments which change their shard factors over time. type ShardingConfig struct { From chunk.DayTime `yaml:"from"` Shards int `yaml:"shards"` } +// ShardingConfigs is a slice of shards type ShardingConfigs []ShardingConfig +// ValidRange extracts a non-overlapping sharding configuration from a list of configs and a time range. func (confs ShardingConfigs) ValidRange(start, end int64) (ShardingConfig, error) { for i, conf := range confs { if start < int64(conf.From.Time) { // the query starts before this config's range - return ShardingConfig{}, invalidShardingRange + return ShardingConfig{}, errInvalidShardingRange } else if i == len(confs)-1 { // the last configuration has no upper bound return conf, nil @@ -47,7 +49,7 @@ func (confs ShardingConfigs) ValidRange(start, end int64) (ShardingConfig, error } } - return ShardingConfig{}, invalidShardingRange + return ShardingConfig{}, errInvalidShardingRange } @@ -59,6 +61,7 @@ func mapQuery(mapper astmapper.ASTMapper, query string) (promql.Node, error) { return mapper.Map(expr) } +// QueryShardMiddleware creates a middleware which downstreams queries after AST mapping and query encoding. func QueryShardMiddleware(engine *promql.Engine, confs ShardingConfigs) queryrange.Middleware { return queryrange.MiddlewareFunc(func(next queryrange.Handler) queryrange.Handler { return &queryShard{ diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index d152ade6fe7..52c5519b1b2 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -205,7 +205,7 @@ func TestShardingConfigs_ValidRange(t *testing.T) { name: "0 ln configs fail", confs: ShardingConfigs{}, req: defaultReq(), - err: invalidShardingRange, + err: errInvalidShardingRange, }, { name: "request starts before beginning config", @@ -216,7 +216,7 @@ func TestShardingConfigs_ValidRange(t *testing.T) { }, }, req: reqWith("2019-10-15", ""), - err: invalidShardingRange, + err: errInvalidShardingRange, }, { name: "request spans multiple configs", @@ -231,7 +231,7 @@ func TestShardingConfigs_ValidRange(t *testing.T) { }, }, req: reqWith("2019-10-15", "2019-11-17"), - err: invalidShardingRange, + err: errInvalidShardingRange, }, { name: "selects correct config ", diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go index b96ef02d13a..1fe4ca7789d 100644 --- a/pkg/querier/querysharding/queryable.go +++ b/pkg/querier/querysharding/queryable.go @@ -3,6 +3,7 @@ package querysharding import ( "context" "encoding/hex" + "github.com/cortexproject/cortex/pkg/querier/astmapper" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/pkg/errors" @@ -15,7 +16,7 @@ const ( nonEmbeddedErrMsg = "DownstreamQuerier cannot handle a non-embedded query" ) -// DownstreamQueryable is a wrapper for and implementor of the Queryable interface. +// DownstreamQueryable is an implementor of the Queryable interface. type DownstreamQueryable struct { Req *queryrange.Request Handler queryrange.Handler @@ -25,7 +26,7 @@ func (q *DownstreamQueryable) Querier(ctx context.Context, mint, maxt int64) (st return &DownstreamQuerier{ctx, q.Req, q.Handler}, nil } -// DownstreamQueryable is a an implementor of the Queryable interface. +// DownstreamQuerier is a an implementor of the Querier interface. type DownstreamQuerier struct { Ctx context.Context Req *queryrange.Request @@ -40,11 +41,11 @@ func (q *DownstreamQuerier) Select( var embeddedQuery string var isEmbedded bool for _, matcher := range matchers { - if matcher.Name == "__name__" && matcher.Value == astmapper.EMBEDDED_QUERY_FLAG { + if matcher.Name == "__name__" && matcher.Value == astmapper.EmbeddedQueryFlag { isEmbedded = true } - if matcher.Name == astmapper.QUERY_LABEL { + if matcher.Name == astmapper.QueryLabel { embeddedQuery = matcher.Value } } @@ -52,9 +53,8 @@ func (q *DownstreamQuerier) Select( if isEmbedded { if embeddedQuery != "" { return q.handleEmbeddedQuery(embeddedQuery) - } else { - return nil, nil, errors.Errorf(missingEmbeddedQueryMsg) } + return nil, nil, errors.Errorf(missingEmbeddedQueryMsg) } @@ -82,7 +82,6 @@ func (q *DownstreamQuerier) handleEmbeddedQuery(encoded string) (storage.SeriesS } -// other storage.Querier impls that are not used by engine // LabelValues returns all potential values for a label name. func (q *DownstreamQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { return nil, nil, errors.Errorf("unimplemented") @@ -98,7 +97,7 @@ func (q *DownstreamQuerier) Close() error { return nil } -// take advantage of pass by value to clone a request with a new query +// ReplaceQuery clones a request with a new query func ReplaceQuery(req queryrange.Request, query string) *queryrange.Request { req.Query = query return &req diff --git a/pkg/querier/querysharding/queryable_test.go b/pkg/querier/querysharding/queryable_test.go index 82b410e61a0..f34f4a46228 100644 --- a/pkg/querier/querysharding/queryable_test.go +++ b/pkg/querier/querysharding/queryable_test.go @@ -3,13 +3,14 @@ package querysharding import ( "context" "encoding/hex" + "testing" + "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/astmapper" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" ) func TestSelect(t *testing.T) { @@ -54,8 +55,8 @@ func TestSelect(t *testing.T) { _, _, err := q.Select( nil, - exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), - exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + exactMatch("__name__", astmapper.EmbeddedQueryFlag), + exactMatch(astmapper.QueryLabel, hexEncode(`http_requests_total{cluster="prod"}`)), ) require.Nil(t, err) }, @@ -71,8 +72,8 @@ func TestSelect(t *testing.T) { fn: func(t *testing.T, q *DownstreamQuerier) { set, _, err := q.Select( nil, - exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), - exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + exactMatch("__name__", astmapper.EmbeddedQueryFlag), + exactMatch(astmapper.QueryLabel, hexEncode(`http_requests_total{cluster="prod"}`)), ) require.Nil(t, set) require.EqualError(t, err, "SomeErr") @@ -125,8 +126,8 @@ func TestSelect(t *testing.T) { fn: func(t *testing.T, q *DownstreamQuerier) { set, _, err := q.Select( nil, - exactMatch("__name__", astmapper.EMBEDDED_QUERY_FLAG), - exactMatch(astmapper.QUERY_LABEL, hexEncode(`http_requests_total{cluster="prod"}`)), + exactMatch("__name__", astmapper.EmbeddedQueryFlag), + exactMatch(astmapper.QueryLabel, hexEncode(`http_requests_total{cluster="prod"}`)), ) require.Nil(t, err) require.Equal( diff --git a/pkg/querier/querysharding/series.go b/pkg/querier/querysharding/series.go index 1e300b9c16f..fdf181005ea 100644 --- a/pkg/querier/querysharding/series.go +++ b/pkg/querier/querysharding/series.go @@ -10,7 +10,7 @@ import ( "github.com/prometheus/prometheus/storage" ) -// needed to map back from api response to the underlying series data +// ResponseToSeries is needed to map back from api response to the underlying series data func ResponseToSeries(resp queryrange.Response) (storage.SeriesSet, error) { switch resp.ResultType { case promql.ValueTypeVector, promql.ValueTypeMatrix: diff --git a/pkg/querier/series/series_set.go b/pkg/querier/series/series_set.go index a181d9c2797..c819e19d330 100644 --- a/pkg/querier/series/series_set.go +++ b/pkg/querier/series/series_set.go @@ -31,6 +31,7 @@ type ConcreteSeriesSet struct { series []storage.Series } +// NewConcreteSeriesSet instantiates an in-memory series set from a series func NewConcreteSeriesSet(series []storage.Series) storage.SeriesSet { sort.Sort(byLabels(series)) return &ConcreteSeriesSet{ @@ -39,15 +40,18 @@ func NewConcreteSeriesSet(series []storage.Series) storage.SeriesSet { } } +// Next iterates through a series set and impls storage.SeriesSet func (c *ConcreteSeriesSet) Next() bool { c.cur++ return c.cur < len(c.series) } +// At returns the current series and impls storage.SeriesSet func (c *ConcreteSeriesSet) At() storage.Series { return c.series[c.cur] } +// Err impls storage.SeriesSet func (c *ConcreteSeriesSet) Err() error { return nil } @@ -58,6 +62,7 @@ type ConcreteSeries struct { samples []model.SamplePair } +// NewConcreteSeries instantiates an in memory series from a list of samples & labels func NewConcreteSeries(ls labels.Labels, samples []model.SamplePair) *ConcreteSeries { return &ConcreteSeries{ labels: ls, @@ -65,10 +70,12 @@ func NewConcreteSeries(ls labels.Labels, samples []model.SamplePair) *ConcreteSe } } +// Labels impls storage.Series func (c *ConcreteSeries) Labels() labels.Labels { return c.labels } +// Iterator impls storage.Series func (c *ConcreteSeries) Iterator() storage.SeriesIterator { return NewConcreteSeriesIterator(c) } @@ -79,6 +86,7 @@ type concreteSeriesIterator struct { series *ConcreteSeries } +// NewConcreteSeries instaniates an in memory storage.SeriesIterator func NewConcreteSeriesIterator(series *ConcreteSeries) storage.SeriesIterator { return &concreteSeriesIterator{ cur: -1, @@ -107,6 +115,7 @@ func (c *concreteSeriesIterator) Err() error { return nil } +// NewErrIterator instantiates an errIterator func NewErrIterator(err error) errIterator { return errIterator{err} } @@ -132,6 +141,7 @@ func (e errIterator) Err() error { return e.err } +// MatrixToSeriesSet creates a storage.SeriesSet from a model.Matrix func MatrixToSeriesSet(m model.Matrix) storage.SeriesSet { series := make([]storage.Series, 0, len(m)) for _, ss := range m { @@ -143,6 +153,7 @@ func MatrixToSeriesSet(m model.Matrix) storage.SeriesSet { return NewConcreteSeriesSet(series) } +// MetricsToSeriesSet creates a storage.SeriesSet from a []metric.Metric func MetricsToSeriesSet(ms []metric.Metric) storage.SeriesSet { series := make([]storage.Series, 0, len(ms)) for _, m := range ms { From bc5077912bd7fbb5430a2ac803aedde32b099d76 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 15:18:39 -0400 Subject: [PATCH 22/97] __name__ label via constant Co-Authored-By: Cyril Tovena Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/querysharding/queryable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go index 1fe4ca7789d..80208bbff66 100644 --- a/pkg/querier/querysharding/queryable.go +++ b/pkg/querier/querysharding/queryable.go @@ -41,7 +41,7 @@ func (q *DownstreamQuerier) Select( var embeddedQuery string var isEmbedded bool for _, matcher := range matchers { - if matcher.Name == "__name__" && matcher.Value == astmapper.EmbeddedQueryFlag { + if matcher.Name == labels.MetricName && matcher.Value == astmapper.EmbeddedQueryFlag { isEmbedded = true } From 8879c6a77ee0ad02d36a171e2ae2a5c0c8a83f14 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 15:19:08 -0400 Subject: [PATCH 23/97] corrects error source Co-Authored-By: Cyril Tovena Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/parallel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/querier/astmapper/parallel.go b/pkg/querier/astmapper/parallel.go index 76e5697b630..68fab76e1d2 100644 --- a/pkg/querier/astmapper/parallel.go +++ b/pkg/querier/astmapper/parallel.go @@ -62,7 +62,7 @@ func CanParallel(node promql.Node) bool { return true default: - panic(errors.Errorf("CloneNode: unhandled node type %T", node)) + panic(errors.Errorf("CanParallel: unhandled node type %T", node)) } return false From a936b6bec21a45595eb0ccf48a6e5cff52c63964 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Mon, 21 Oct 2019 17:56:10 -0400 Subject: [PATCH 24/97] linting Signed-off-by: Owen Diehl Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/astmapper.go | 13 +++++++------ pkg/querier/astmapper/embedded.go | 14 ++++++++------ pkg/querier/astmapper/shard_summer.go | 10 ++++++---- pkg/querier/lazyquery/lazyquery.go | 8 ++++++++ pkg/querier/queryrange/query_range.go | 5 +++-- pkg/querier/querysharding/middleware.go | 21 +++++++++++---------- pkg/querier/querysharding/queryable.go | 1 + pkg/querier/series/series_set.go | 4 ++-- 8 files changed, 46 insertions(+), 30 deletions(-) diff --git a/pkg/querier/astmapper/astmapper.go b/pkg/querier/astmapper/astmapper.go index b2b44734260..461d92c58fa 100644 --- a/pkg/querier/astmapper/astmapper.go +++ b/pkg/querier/astmapper/astmapper.go @@ -23,6 +23,7 @@ type MultiMapper struct { mappers []ASTMapper } +// Map impls ASTMapper func (m *MultiMapper) Map(node promql.Node) (promql.Node, error) { var result promql.Node = node var err error @@ -76,18 +77,18 @@ func (f NodeMapperFunc) MapNode(node promql.Node) (promql.Node, bool, error) { return f(node) } -// NewNodeMapper creates an ASTMapper from a NodeMapper -func NewNodeMapper(mapper NodeMapper) nodeMapper { - return nodeMapper{mapper} +// NewASTNodeMapper creates an ASTMapper from a NodeMapper +func NewASTNodeMapper(mapper NodeMapper) ASTNodeMapper { + return ASTNodeMapper{mapper} } -// nodeMapper is an ASTMapper adapter which uses a NodeMapper internally. -type nodeMapper struct { +// ASTNodeMapper is an ASTMapper adapter which uses a NodeMapper internally. +type ASTNodeMapper struct { NodeMapper } // Map impls ASTMapper from a NodeMapper -func (nm nodeMapper) Map(node promql.Node) (promql.Node, error) { +func (nm ASTNodeMapper) Map(node promql.Node) (promql.Node, error) { node, fin, err := nm.MapNode(node) if err != nil { diff --git a/pkg/querier/astmapper/embedded.go b/pkg/querier/astmapper/embedded.go index 737b0d77808..54b1333f163 100644 --- a/pkg/querier/astmapper/embedded.go +++ b/pkg/querier/astmapper/embedded.go @@ -24,8 +24,10 @@ Ideally the promql.Engine could be an interface instead of a concrete type, allo */ const ( - QueryLabel = "__cortex_query__" // reserved label containing an embedded query - EmbeddedQueryFlag = "__embedded_query__" // reserved label (metric name) denoting an embedded query + // QueryLabel is a reserved label containing an embedded query + QueryLabel = "__cortex_query__" + // EmbeddedQueryFlag is a reserved label (metric name) denoting an embedded query + EmbeddedQueryFlag = "__embedded_query__" ) // Squash reduces an AST into a single vector or matrix query which can be hijacked by a Queryable impl. @@ -33,7 +35,7 @@ func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { // promql's label charset is not a subset of promql's syntax charset. Therefor we use hex as an intermediary encoded := hex.EncodeToString([]byte(node.String())) - embedded_query, err := labels.NewMatcher(labels.MatchEqual, QueryLabel, encoded) + embeddedQuery, err := labels.NewMatcher(labels.MatchEqual, QueryLabel, encoded) if err != nil { return nil, err @@ -43,13 +45,13 @@ func Squash(node promql.Node, isMatrix bool) (promql.Expr, error) { return &promql.MatrixSelector{ Name: EmbeddedQueryFlag, Range: time.Minute, - LabelMatchers: []*labels.Matcher{embedded_query}, + LabelMatchers: []*labels.Matcher{embeddedQuery}, }, nil } return &promql.VectorSelector{ Name: EmbeddedQueryFlag, - LabelMatchers: []*labels.Matcher{embedded_query}, + LabelMatchers: []*labels.Matcher{embeddedQuery}, }, nil } @@ -61,7 +63,7 @@ func VectorSquasher(node promql.Node) (promql.Expr, error) { // ShallowEmbedSelectors encodes selector queries if they do not already have the EmbeddedQueryFlag. // This is primarily useful for deferring query execution. -var ShallowEmbedSelectors = NewNodeMapper(NodeMapperFunc(shallowEmbedSelectors)) +var ShallowEmbedSelectors = NewASTNodeMapper(NodeMapperFunc(shallowEmbedSelectors)) func shallowEmbedSelectors(node promql.Node) (promql.Node, bool, error) { switch n := node.(type) { diff --git a/pkg/querier/astmapper/shard_summer.go b/pkg/querier/astmapper/shard_summer.go index 827ba111032..4c6531379b9 100644 --- a/pkg/querier/astmapper/shard_summer.go +++ b/pkg/querier/astmapper/shard_summer.go @@ -8,8 +8,10 @@ import ( ) const ( - DefaultShards = 12 // default shard factor to assume - ShardLabel = "__cortex_shard__" // reserved label referencing a cortex shard + // DefaultShards factor to assume + DefaultShards = 12 + // ShardLabel is a reserved label referencing a cortex shard + ShardLabel = "__cortex_shard__" ) type squasher = func(promql.Node) (promql.Expr, error) @@ -26,7 +28,7 @@ func NewShardSummer(shards int, squasher squasher) ASTMapper { shards = DefaultShards } - return NewNodeMapper(&shardSummer{ + return NewASTNodeMapper(&shardSummer{ shards: shards, squash: squasher, curshard: nil, @@ -86,7 +88,7 @@ func (summer *shardSummer) shardSum(expr *promql.AggregateExpr) (promql.Node, er return nil, err } - subSummer := NewNodeMapper(summer.CopyWithCurshard(i)) + subSummer := NewASTNodeMapper(summer.CopyWithCurshard(i)) sharded, err := subSummer.Map(cloned) if err != nil { return nil, err diff --git a/pkg/querier/lazyquery/lazyquery.go b/pkg/querier/lazyquery/lazyquery.go index e5f35e78d3d..60954be341b 100644 --- a/pkg/querier/lazyquery/lazyquery.go +++ b/pkg/querier/lazyquery/lazyquery.go @@ -11,10 +11,12 @@ import ( "github.com/prometheus/prometheus/storage" ) +// LazyQueryable wraps a storage.Queryable type LazyQueryable struct { q storage.Queryable } +// Querier impls storage.Queryable func (lq LazyQueryable) Querier(ctx context.Context, mint, maxt int64) (storage.Querier, error) { q, err := lq.q.Querier(ctx, mint, maxt) if err != nil { @@ -24,10 +26,12 @@ func (lq LazyQueryable) Querier(ctx context.Context, mint, maxt int64) (storage. return NewLazyQuerier(q), nil } +// NewLazyQueryable returns a lazily wrapped queryable func NewLazyQueryable(q storage.Queryable) storage.Queryable { return LazyQueryable{q} } +// LazyQuerier is a lazy-loaded adapter for a storage.Querier type LazyQuerier struct { next storage.Querier } @@ -38,6 +42,7 @@ func NewLazyQuerier(next storage.Querier) storage.Querier { return LazyQuerier{next} } +// Select impls Storage.Querier func (l LazyQuerier) Select(params *storage.SelectParams, matchers ...*labels.Matcher) (storage.SeriesSet, storage.Warnings, error) { future := make(chan storage.SeriesSet) go func() { @@ -53,14 +58,17 @@ func (l LazyQuerier) Select(params *storage.SelectParams, matchers ...*labels.Ma }, nil, nil } +// LabelValues impls Storage.Querier func (l LazyQuerier) LabelValues(name string) ([]string, storage.Warnings, error) { return l.next.LabelValues(name) } +// LabelNames impls Storage.Querier func (l LazyQuerier) LabelNames() ([]string, storage.Warnings, error) { return l.next.LabelNames() } +// Close impls Storage.Querier func (l LazyQuerier) Close() error { return l.next.Close() } diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index 94a98c392ba..c258b784fde 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -22,9 +22,10 @@ import ( "github.com/cortexproject/cortex/pkg/ingester/client" ) +// Exported statuses for downstream queries const ( - StatusSuccess = "success" // Successful status - StatusFailure = "failure" // Failure status + StatusSuccess = "success" + StatusFailure = "failure" ) var ( diff --git a/pkg/querier/querysharding/middleware.go b/pkg/querier/querysharding/middleware.go index 163eab470e5..142ea7ed97e 100644 --- a/pkg/querier/querysharding/middleware.go +++ b/pkg/querier/querysharding/middleware.go @@ -113,7 +113,7 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang res := qry.Exec(ctx) - // TODO(owen): Unclear on whether error belongs in APIResponse struct or as 2nd value in return tuple + // TODO(owen-d): Unclear on whether error belongs in APIResponse struct or as 2nd value in return tuple if res.Err != nil { return &queryrange.APIResponse{ Status: queryrange.StatusFailure, @@ -122,24 +122,25 @@ func (qs *queryShard) Do(ctx context.Context, r *queryrange.Request) (*queryrang }, nil } - if extracted, err := FromValue(res.Value); err != nil { + extracted, err := FromValue(res.Value) + if err != nil { return &queryrange.APIResponse{ Status: queryrange.StatusFailure, ErrorType: downStreamErrType, Error: err.Error(), }, nil - } else { - return &queryrange.APIResponse{ - Status: queryrange.StatusSuccess, - Data: queryrange.Response{ - ResultType: string(res.Value.Type()), - Result: extracted, - }, - }, nil } + return &queryrange.APIResponse{ + Status: queryrange.StatusSuccess, + Data: queryrange.Response{ + ResultType: string(res.Value.Type()), + Result: extracted, + }, + }, nil } +// TimeFromMillis is a helper to turn milliseconds -> time.Time func TimeFromMillis(ms int64) time.Time { secs := ms / 1000 rem := ms - (secs * 1000) diff --git a/pkg/querier/querysharding/queryable.go b/pkg/querier/querysharding/queryable.go index 80208bbff66..61d9e2b31ad 100644 --- a/pkg/querier/querysharding/queryable.go +++ b/pkg/querier/querysharding/queryable.go @@ -22,6 +22,7 @@ type DownstreamQueryable struct { Handler queryrange.Handler } +// Querier impls Queryable func (q *DownstreamQueryable) Querier(ctx context.Context, mint, maxt int64) (storage.Querier, error) { return &DownstreamQuerier{ctx, q.Req, q.Handler}, nil } diff --git a/pkg/querier/series/series_set.go b/pkg/querier/series/series_set.go index c819e19d330..d079f57ec76 100644 --- a/pkg/querier/series/series_set.go +++ b/pkg/querier/series/series_set.go @@ -86,7 +86,7 @@ type concreteSeriesIterator struct { series *ConcreteSeries } -// NewConcreteSeries instaniates an in memory storage.SeriesIterator +// NewConcreteSeriesIterator instaniates an in memory storage.SeriesIterator func NewConcreteSeriesIterator(series *ConcreteSeries) storage.SeriesIterator { return &concreteSeriesIterator{ cur: -1, @@ -116,7 +116,7 @@ func (c *concreteSeriesIterator) Err() error { } // NewErrIterator instantiates an errIterator -func NewErrIterator(err error) errIterator { +func NewErrIterator(err error) storage.SeriesIterator { return errIterator{err} } From 197d902015bd1e9bc36e755e80569b6d2983609c Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Tue, 22 Oct 2019 09:31:02 -0400 Subject: [PATCH 25/97] ast unit test cases, removes use of unkeyed fields Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/querier/astmapper/astmapper_test.go | 23 +++++-- pkg/querier/astmapper/parallel.go | 1 - pkg/querier/astmapper/shard_summer_test.go | 39 ++++++++--- pkg/querier/querysharding/middleware_test.go | 37 ++++++----- pkg/querier/querysharding/queryable_test.go | 32 ++++----- pkg/querier/querysharding/series_test.go | 19 +++--- pkg/querier/querysharding/value.go | 4 +- pkg/querier/querysharding/value_test.go | 69 ++++++++++---------- 8 files changed, 127 insertions(+), 97 deletions(-) diff --git a/pkg/querier/astmapper/astmapper_test.go b/pkg/querier/astmapper/astmapper_test.go index 5afa2c3736b..c3f0c0b9f1f 100644 --- a/pkg/querier/astmapper/astmapper_test.go +++ b/pkg/querier/astmapper/astmapper_test.go @@ -2,11 +2,12 @@ package astmapper import ( "fmt" + "testing" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" ) func TestCloneNode(t *testing.T) { @@ -16,8 +17,16 @@ func TestCloneNode(t *testing.T) { }{ // simple unmodified case { - &promql.BinaryExpr{promql.ItemADD, &promql.NumberLiteral{1}, &promql.NumberLiteral{1}, nil, false}, - &promql.BinaryExpr{promql.ItemADD, &promql.NumberLiteral{1}, &promql.NumberLiteral{1}, nil, false}, + &promql.BinaryExpr{ + Op: promql.ItemADD, + LHS: &promql.NumberLiteral{Val: 1}, + RHS: &promql.NumberLiteral{Val: 1}, + }, + &promql.BinaryExpr{ + Op: promql.ItemADD, + LHS: &promql.NumberLiteral{Val: 1}, + RHS: &promql.NumberLiteral{Val: 1}, + }, }, { &promql.AggregateExpr{ @@ -60,16 +69,16 @@ func TestCloneNode_String(t *testing.T) { expected string }{ { - `rate(http_requests_total{cluster="us-central1"}[1m])`, - `rate(http_requests_total{cluster="us-central1"}[1m])`, + input: `rate(http_requests_total{cluster="us-central1"}[1m])`, + expected: `rate(http_requests_total{cluster="us-central1"}[1m])`, }, { - `sum( + input: `sum( sum(rate(http_requests_total{cluster="us-central1"}[1m])) / sum(rate(http_requests_total{cluster="ops-tools1"}[1m])) )`, - `sum(sum(rate(http_requests_total{cluster="us-central1"}[1m])) / sum(rate(http_requests_total{cluster="ops-tools1"}[1m])))`, + expected: `sum(sum(rate(http_requests_total{cluster="us-central1"}[1m])) / sum(rate(http_requests_total{cluster="ops-tools1"}[1m])))`, }, } diff --git a/pkg/querier/astmapper/parallel.go b/pkg/querier/astmapper/parallel.go index 68fab76e1d2..b1038e7cd72 100644 --- a/pkg/querier/astmapper/parallel.go +++ b/pkg/querier/astmapper/parallel.go @@ -65,7 +65,6 @@ func CanParallel(node promql.Node) bool { panic(errors.Errorf("CanParallel: unhandled node type %T", node)) } - return false } func funcParallel(f *promql.Function) bool { diff --git a/pkg/querier/astmapper/shard_summer_test.go b/pkg/querier/astmapper/shard_summer_test.go index 30e4d3750e8..86df72ab3cc 100644 --- a/pkg/querier/astmapper/shard_summer_test.go +++ b/pkg/querier/astmapper/shard_summer_test.go @@ -2,9 +2,10 @@ package astmapper import ( "fmt" + "testing" + "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" ) func TestShardSummer(t *testing.T) { @@ -14,22 +15,22 @@ func TestShardSummer(t *testing.T) { expected string }{ { - 3, - `sum by(foo) (rate(bar1{baz="blip"}[1m]))`, - `sum by(foo) ( + shards: 3, + input: `sum by(foo) (rate(bar1{baz="blip"}[1m]))`, + expected: `sum by(foo) ( sum by(foo) (rate(bar1{__cortex_shard__="0_of_3",baz="blip"}[1m])) or sum by(foo) (rate(bar1{__cortex_shard__="1_of_3",baz="blip"}[1m])) or sum by(foo) (rate(bar1{__cortex_shard__="2_of_3",baz="blip"}[1m])) )`, }, { - 2, - `sum( + shards: 2, + input: `sum( sum by (foo) (rate(bar1{baz="blip"}[1m])) / sum by (foo) (rate(foo{baz="blip"}[1m])) )`, - `sum( + expected: `sum( sum by(foo) ( sum by(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or sum by(foo) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) @@ -43,9 +44,9 @@ func TestShardSummer(t *testing.T) { }, // This is currently redundant but still equivalent: sums split into sharded versions, including summed sums. { - 2, - `sum(sum by(foo) (rate(bar1{baz="blip"}[1m])))`, - `sum( + shards: 2, + input: `sum(sum by(foo) (rate(bar1{baz="blip"}[1m])))`, + expected: `sum( sum( sum by(foo) ( sum by(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or @@ -60,6 +61,24 @@ func TestShardSummer(t *testing.T) { ) )`, }, + // without + { + shards: 2, + input: `sum without(foo) (rate(bar1{baz="blip"}[1m]))`, + expected: `sum without(foo) ( + sum without(foo) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum without(foo) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) + )`, + }, + // multiple dimensions + { + shards: 2, + input: `sum by(foo, bom) (rate(bar1{baz="blip"}[1m]))`, + expected: `sum by(foo, bom) ( + sum by(foo, bom) (rate(bar1{__cortex_shard__="0_of_2",baz="blip"}[1m])) or + sum by(foo, bom) (rate(bar1{__cortex_shard__="1_of_2",baz="blip"}[1m])) + )`, + }, } for i, c := range testExpr { diff --git a/pkg/querier/querysharding/middleware_test.go b/pkg/querier/querysharding/middleware_test.go index 52c5519b1b2..99273b020ca 100644 --- a/pkg/querier/querysharding/middleware_test.go +++ b/pkg/querier/querysharding/middleware_test.go @@ -58,7 +58,8 @@ func TestMiddleware(t *testing.T) { name: "expiration", next: mockHandler(sampleMatrixResponse(), nil), override: func(t *testing.T, handler queryrange.Handler) { - expired, _ := context.WithDeadline(context.Background(), time.Unix(0, 0)) + expired, cancel := context.WithDeadline(context.Background(), time.Unix(0, 0)) + defer cancel() res, err := handler.Do(expired, defaultReq()) require.Nil(t, err) require.NotEqual(t, "", res.Error) @@ -122,15 +123,15 @@ func sampleMatrixResponse() *queryrange.APIResponse { Result: []queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { TimestampMs: 5, Value: 1, }, - client.Sample{ + { TimestampMs: 10, Value: 2, }, @@ -138,15 +139,15 @@ func sampleMatrixResponse() *queryrange.APIResponse { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { TimestampMs: 5, Value: 8, }, - client.Sample{ + { TimestampMs: 10, Value: 9, }, @@ -211,7 +212,7 @@ func TestShardingConfigs_ValidRange(t *testing.T) { name: "request starts before beginning config", confs: ShardingConfigs{ { - From: chunk.DayTime{parseDate("2019-10-16")}, + From: chunk.DayTime{Time: parseDate("2019-10-16")}, Shards: 1, }, }, @@ -222,11 +223,11 @@ func TestShardingConfigs_ValidRange(t *testing.T) { name: "request spans multiple configs", confs: ShardingConfigs{ { - From: chunk.DayTime{parseDate("2019-10-16")}, + From: chunk.DayTime{Time: parseDate("2019-10-16")}, Shards: 1, }, { - From: chunk.DayTime{parseDate("2019-11-16")}, + From: chunk.DayTime{Time: parseDate("2019-11-16")}, Shards: 2, }, }, @@ -237,21 +238,21 @@ func TestShardingConfigs_ValidRange(t *testing.T) { name: "selects correct config ", confs: ShardingConfigs{ { - From: chunk.DayTime{parseDate("2019-10-16")}, + From: chunk.DayTime{Time: parseDate("2019-10-16")}, Shards: 1, }, { - From: chunk.DayTime{parseDate("2019-11-16")}, + From: chunk.DayTime{Time: parseDate("2019-11-16")}, Shards: 2, }, { - From: chunk.DayTime{parseDate("2019-12-16")}, + From: chunk.DayTime{Time: parseDate("2019-12-16")}, Shards: 3, }, }, req: reqWith("2019-11-20", "2019-11-25"), expected: ShardingConfig{ - From: chunk.DayTime{parseDate("2019-11-16")}, + From: chunk.DayTime{Time: parseDate("2019-11-16")}, Shards: 2, }, }, @@ -276,8 +277,8 @@ func TestTimeFromMillis(t *testing.T) { input int64 expected time.Time }{ - {1000, time.Unix(1, 0)}, - {1500, time.Unix(1, 500*nanosecondsInMillisecond)}, + {input: 1000, expected: time.Unix(1, 0)}, + {input: 1500, expected: time.Unix(1, 500*nanosecondsInMillisecond)}, } for i, c := range testExpr { diff --git a/pkg/querier/querysharding/queryable_test.go b/pkg/querier/querysharding/queryable_test.go index f34f4a46228..038b11405de 100644 --- a/pkg/querier/querysharding/queryable_test.go +++ b/pkg/querier/querysharding/queryable_test.go @@ -88,15 +88,15 @@ func TestSelect(t *testing.T) { Result: []queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, - client.Sample{ + { Value: 2, TimestampMs: 2, }, @@ -104,15 +104,15 @@ func TestSelect(t *testing.T) { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 8, TimestampMs: 1, }, - client.Sample{ + { Value: 9, TimestampMs: 2, }, @@ -135,15 +135,15 @@ func TestSelect(t *testing.T) { newSeriesSet([]queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, - client.Sample{ + { Value: 2, TimestampMs: 2, }, @@ -151,15 +151,15 @@ func TestSelect(t *testing.T) { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 8, TimestampMs: 1, }, - client.Sample{ + { Value: 9, TimestampMs: 2, }, diff --git a/pkg/querier/querysharding/series_test.go b/pkg/querier/querysharding/series_test.go index b255a1da97b..08c67b58a1f 100644 --- a/pkg/querier/querysharding/series_test.go +++ b/pkg/querier/querysharding/series_test.go @@ -1,11 +1,12 @@ package querysharding import ( + "testing" + "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" ) func Test_ResponseToSeries(t *testing.T) { @@ -14,15 +15,15 @@ func Test_ResponseToSeries(t *testing.T) { Result: []queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, - client.Sample{ + { Value: 2, TimestampMs: 2, }, @@ -30,15 +31,15 @@ func Test_ResponseToSeries(t *testing.T) { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 8, TimestampMs: 1, }, - client.Sample{ + { Value: 9, TimestampMs: 2, }, diff --git a/pkg/querier/querysharding/value.go b/pkg/querier/querysharding/value.go index 84a09989f89..d406ac11d20 100644 --- a/pkg/querier/querysharding/value.go +++ b/pkg/querier/querysharding/value.go @@ -13,9 +13,9 @@ func FromValue(val promql.Value) ([]queryrange.SampleStream, error) { switch v := val.(type) { case promql.Scalar: return []queryrange.SampleStream{ - queryrange.SampleStream{ + { Samples: []client.Sample{ - client.Sample{ + { Value: v.V, TimestampMs: v.T, }, diff --git a/pkg/querier/querysharding/value_test.go b/pkg/querier/querysharding/value_test.go index a7ad6c59ea5..71e5a959052 100644 --- a/pkg/querier/querysharding/value_test.go +++ b/pkg/querier/querysharding/value_test.go @@ -2,12 +2,13 @@ package querysharding import ( "fmt" + "testing" + "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/querier/queryrange" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/stretchr/testify/require" - "testing" ) func TestFromValue(t *testing.T) { @@ -18,17 +19,17 @@ func TestFromValue(t *testing.T) { }{ // string (errors) { - input: promql.String{1, "hi"}, + input: promql.String{T: 1, V: "hi"}, err: true, }, // Scalar { - input: promql.Scalar{1, 1}, + input: promql.Scalar{T: 1, V: 1}, err: false, expected: []queryrange.SampleStream{ { Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, @@ -40,17 +41,17 @@ func TestFromValue(t *testing.T) { { input: promql.Vector{ promql.Sample{ - promql.Point{1, 1}, - labels.Labels{ - {"a", "a1"}, - {"b", "b1"}, + Point: promql.Point{T: 1, V: 1}, + Metric: labels.Labels{ + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, }, promql.Sample{ - promql.Point{2, 2}, - labels.Labels{ - {"a", "a2"}, - {"b", "b2"}, + Point: promql.Point{T: 2, V: 2}, + Metric: labels.Labels{ + {Name: "a", Value: "a2"}, + {Name: "b", Value: "b2"}, }, }, }, @@ -58,11 +59,11 @@ func TestFromValue(t *testing.T) { expected: []queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, @@ -70,11 +71,11 @@ func TestFromValue(t *testing.T) { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a2"}, + {Name: "b", Value: "b2"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 2, TimestampMs: 2, }, @@ -87,22 +88,22 @@ func TestFromValue(t *testing.T) { input: promql.Matrix{ { Metric: labels.Labels{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Points: []promql.Point{ - {1, 1}, - {2, 2}, + {T: 1, V: 1}, + {T: 2, V: 2}, }, }, { Metric: labels.Labels{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a2"}, + {Name: "b", Value: "b2"}, }, Points: []promql.Point{ - {1, 8}, - {2, 9}, + {T: 1, V: 8}, + {T: 2, V: 9}, }, }, }, @@ -110,15 +111,15 @@ func TestFromValue(t *testing.T) { expected: []queryrange.SampleStream{ { Labels: []client.LabelAdapter{ - {"a", "a1"}, - {"b", "b1"}, + {Name: "a", Value: "a1"}, + {Name: "b", Value: "b1"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 1, TimestampMs: 1, }, - client.Sample{ + { Value: 2, TimestampMs: 2, }, @@ -126,15 +127,15 @@ func TestFromValue(t *testing.T) { }, { Labels: []client.LabelAdapter{ - {"a", "a2"}, - {"b", "b2"}, + {Name: "a", Value: "a2"}, + {Name: "b", Value: "b2"}, }, Samples: []client.Sample{ - client.Sample{ + { Value: 8, TimestampMs: 1, }, - client.Sample{ + { Value: 9, TimestampMs: 2, }, From 455cdefde628501899c5003d34debeae5e02b37a Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Tue, 22 Oct 2019 17:13:18 -0400 Subject: [PATCH 26/97] shard aware storage filtering Signed-off-by: Owen Diehl Signed-off-by: Cyril Tovena --- pkg/chunk/schema.go | 54 ++++++++++++++++-- pkg/chunk/schema_test.go | 64 ++++++++++++++++++++++ pkg/chunk/series_store.go | 33 ++++++++++- pkg/querier/astmapper/shard_summer.go | 40 +++++++++++++- pkg/querier/astmapper/shard_summer_test.go | 40 ++++++++++++++ 5 files changed, 222 insertions(+), 9 deletions(-) diff --git a/pkg/chunk/schema.go b/pkg/chunk/schema.go index ce69abf8ed7..89646d596d5 100644 --- a/pkg/chunk/schema.go +++ b/pkg/chunk/schema.go @@ -7,7 +7,10 @@ import ( "fmt" "strings" + "strconv" + jsoniter "github.com/json-iterator/go" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" ) @@ -46,6 +49,7 @@ type Schema interface { GetReadQueriesForMetric(from, through model.Time, userID string, metricName string) ([]IndexQuery, error) GetReadQueriesForMetricLabel(from, through model.Time, userID string, metricName string, labelName string) ([]IndexQuery, error) GetReadQueriesForMetricLabelValue(from, through model.Time, userID string, metricName string, labelName string, labelValue string) ([]IndexQuery, error) + FilterIndexQueries(queries []IndexQuery, shard *int) []IndexQuery // If the query resulted in series IDs, use this method to find chunks. GetChunksForSeries(from, through model.Time, userID string, seriesID []byte) ([]IndexQuery, error) @@ -216,6 +220,10 @@ func (s schema) GetLabelNamesForSeries(from, through model.Time, userID string, return result, nil } +func (s schema) FilterIndexQueries(queries []IndexQuery, shard *int) []IndexQuery { + return s.entries.FilterIndexQueries(queries, shard) +} + type entries interface { GetWriteEntries(bucket Bucket, metricName string, labels labels.Labels, chunkID string) ([]IndexEntry, error) GetLabelWriteEntries(bucket Bucket, metricName string, labels labels.Labels, chunkID string) ([]IndexEntry, error) @@ -226,13 +234,23 @@ type entries interface { GetReadMetricLabelValueQueries(bucket Bucket, metricName string, labelName string, labelValue string) ([]IndexQuery, error) GetChunksForSeries(bucket Bucket, seriesID []byte) ([]IndexQuery, error) GetLabelNamesForSeries(bucket Bucket, seriesID []byte) ([]IndexQuery, error) + FilterIndexQueries(queries []IndexQuery, shard *int) []IndexQuery +} + +// noops is a placeholder which can be embedded to provide default implementations +type noops struct{} + +func (n noops) FilterIndexQueries(queries []IndexQuery, shard *int) []IndexQuery { + return queries } // original entries: // - hash key: :: // - range key: