-
Notifications
You must be signed in to change notification settings - Fork 867
[DQE-1] Core Planning #7688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[DQE-1] Core Planning #7688
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package distributed_execution | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/prometheus/prometheus/promql/parser" | ||
| "github.com/prometheus/prometheus/storage" | ||
| "github.com/thanos-io/promql-engine/execution" | ||
| "github.com/thanos-io/promql-engine/execution/exchange" | ||
| "github.com/thanos-io/promql-engine/execution/model" | ||
| "github.com/thanos-io/promql-engine/logicalplan" | ||
| "github.com/thanos-io/promql-engine/query" | ||
| ) | ||
|
|
||
| const ( | ||
| ShardedRemoteExecutionNode logicalplan.NodeType = "ShardedRemoteExecutionNode" | ||
| ) | ||
|
|
||
| var _ logicalplan.UserDefinedExpr = (*ShardedRemoteExecutions)(nil) | ||
|
|
||
| // ShardedRemoteExecutions is a custom logical-plan node that fans a single | ||
| // aggregation out into one sub-expression per shard. At execution time each | ||
| // sub-expression is built into its own operator and the partial results are | ||
| // coalesced together, so a parent aggregation (e.g. sum) can combine them. | ||
| // | ||
| // NOTE: This is experimental. It is reconciled to compile against the current | ||
| // Thanos engine API, but it is not yet wired end-to-end (see caveats in the | ||
| // port notes): the node has no JSON (un)marshaling for codec round-tripping, | ||
| // and the shard matcher injected by the optimizer does not match the merged | ||
| // metric-name-hash storage sharding mechanism. | ||
| type ShardedRemoteExecutions struct { | ||
| Expressions []logicalplan.Node `json:"-"` | ||
| } | ||
|
|
||
| // MakeExecutionOperator builds one operator per shard sub-expression and | ||
| // coalesces them into a single operator stream for the parent aggregation. | ||
| func (r *ShardedRemoteExecutions) MakeExecutionOperator( | ||
| ctx context.Context, | ||
| opts *query.Options, | ||
| hints storage.SelectHints, | ||
| ) (model.VectorOperator, error) { | ||
| operators := make([]model.VectorOperator, len(r.Expressions)) | ||
| var err error | ||
| for i := range operators { | ||
| operators[i], err = execution.New(ctx, r.Expressions[i], nil, opts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| coalesce := exchange.NewCoalesce(opts, 0, operators...) | ||
| return exchange.NewConcurrent(coalesce, 2, opts), nil | ||
| } | ||
|
|
||
| func (r *ShardedRemoteExecutions) Clone() logicalplan.Node { | ||
| clone := &ShardedRemoteExecutions{Expressions: make([]logicalplan.Node, len(r.Expressions))} | ||
| for i, e := range r.Expressions { | ||
| clone.Expressions[i] = e.Clone() | ||
| } | ||
| return clone | ||
| } | ||
|
|
||
| func (r *ShardedRemoteExecutions) Children() []*logicalplan.Node { | ||
| children := make([]*logicalplan.Node, len(r.Expressions)) | ||
| for i := range r.Expressions { | ||
| children[i] = &r.Expressions[i] | ||
| } | ||
| return children | ||
| } | ||
|
|
||
| func (r *ShardedRemoteExecutions) String() string { | ||
| return fmt.Sprintf("shard(%d)", len(r.Expressions)) | ||
| } | ||
|
|
||
| func (r *ShardedRemoteExecutions) ReturnType() parser.ValueType { return parser.ValueTypeVector } | ||
|
|
||
| func (r *ShardedRemoteExecutions) Type() logicalplan.NodeType { return ShardedRemoteExecutionNode } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,37 +1,84 @@ | ||
| package distributed_execution | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/prometheus/prometheus/model/labels" | ||
| "github.com/prometheus/prometheus/promql/parser" | ||
| "github.com/prometheus/prometheus/util/annotations" | ||
| "github.com/thanos-io/promql-engine/logicalplan" | ||
| "github.com/thanos-io/promql-engine/query" | ||
| ) | ||
|
|
||
| // This is a simplified implementation that only handles binary aggregation cases | ||
| // Future versions of the distributed optimizer are expected to: | ||
| // - Support more complex query patterns | ||
| // - Incorporate diverse optimization strategies | ||
| // - Extend support to node types beyond binary operations | ||
|
|
||
| type DistributedOptimizer struct{} | ||
| // This optimizer inserts Remote nodes so portions of the plan can be executed | ||
| // remotely. It supports two strategies: | ||
| // - Binary-aggregation splitting: each operand of a binary expression that | ||
| // contains an aggregation is offloaded to a Remote fragment. | ||
| // - Sharded-aggregation splitting (experimental): a sum/count aggregation is | ||
| // rewritten into sum(ShardedRemoteExecutions{per-shard sub-aggregations}), | ||
| // so a single aggregation can be evaluated across shards and merged. | ||
| // | ||
| // NOTE: The sharded-aggregation path is experimental and not yet wired | ||
| // end-to-end (no codec round-tripping for ShardedRemoteExecutions, and the | ||
| // injected shard matcher does not match the merged metric-name-hash storage | ||
| // sharding). It is reconciled here only to compile and to allow local | ||
| // iteration. | ||
| type DistributedOptimizer struct { | ||
| // ShardCount is the number of shards the sharded-aggregation strategy fans | ||
| // an aggregation out into. It should match the compactor's per-tenant | ||
| // metric-name-shard-size so per-shard subqueries route to disjoint blocks. | ||
| ShardCount int | ||
| } | ||
|
|
||
| func (d *DistributedOptimizer) Optimize(root logicalplan.Node, opts *query.Options) (logicalplan.Node, annotations.Annotations) { | ||
| warns := annotations.New() | ||
|
|
||
| // insert remote nodes | ||
| logicalplan.TraverseBottomUp(nil, &root, func(parent, current *logicalplan.Node) bool { | ||
|
|
||
| if (*current).Type() == logicalplan.BinaryNode && d.hasAggregation(current) { | ||
| ch := (*current).Children() | ||
| if root == nil { | ||
| return root, *warns | ||
| } | ||
|
|
||
| for _, child := range ch { | ||
| temp := (*child).Clone() | ||
| *child = NewRemoteNode(temp) | ||
| *(*child).Children()[0] = temp | ||
| // Strategy 1 (experimental): shard aggregations (sum/count) across shards. | ||
| // Only runs when a positive shard count is configured; otherwise it is left | ||
| // to the pre-existing binary-splitting strategy below. | ||
| sharded := false | ||
| if shardCount := d.ShardCount; shardCount > 0 { | ||
| logicalplan.TraverseBottomUp(nil, &root, func(parent, current *logicalplan.Node) bool { | ||
| if aggr, ok := (*current).(*logicalplan.Aggregation); ok { | ||
| switch aggr.Op { | ||
| case parser.SUM, parser.COUNT: | ||
| subqueries := newRemoteAggregation(aggr, shardCount) | ||
| *current = &logicalplan.Aggregation{ | ||
| Op: parser.SUM, | ||
| Expr: &ShardedRemoteExecutions{Expressions: subqueries}, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems the Thanos PromQL engine already is doing things a bit differently |
||
| Param: aggr.Param, | ||
| Grouping: aggr.Grouping, | ||
| Without: aggr.Without, | ||
| } | ||
| sharded = true | ||
| } | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| }) | ||
| } | ||
|
|
||
| return false | ||
| }) | ||
| // Strategy 2 (pre-existing): offload binary-expression operands that | ||
| // contain an aggregation to Remote fragments. Skipped when the | ||
| // aggregation-sharding pass already distributed the plan, to avoid | ||
| // double-distributing the same subtrees. | ||
| if !sharded { | ||
| logicalplan.TraverseBottomUp(nil, &root, func(parent, current *logicalplan.Node) bool { | ||
| if (*current).Type() == logicalplan.BinaryNode && d.hasAggregation(current) { | ||
| ch := (*current).Children() | ||
| for _, child := range ch { | ||
| temp := (*child).Clone() | ||
| *child = NewRemoteNode(temp) | ||
| *(*child).Children()[0] = temp | ||
| } | ||
| } | ||
| return false | ||
| }) | ||
| } | ||
|
|
||
| return root, *warns | ||
| } | ||
|
|
@@ -47,3 +94,37 @@ func (d *DistributedOptimizer) hasAggregation(root *logicalplan.Node) bool { | |
| }) | ||
| return isAggr | ||
| } | ||
|
|
||
| // newRemoteAggregation produces one Remote-wrapped per-shard copy of the given | ||
| // aggregation, tagging each copy's selectors with its shard identity. | ||
| func newRemoteAggregation(rootAggregation *logicalplan.Aggregation, shardNum int) []logicalplan.Node { | ||
| nodes := make([]logicalplan.Node, 0, shardNum) | ||
| for i := range shardNum { | ||
| rc := rootAggregation.Expr.Clone() | ||
| node := insertShardNum(&Remote{ | ||
| Expr: &logicalplan.Aggregation{ | ||
| Op: rootAggregation.Op, | ||
| Expr: rc, | ||
| Param: rootAggregation.Param, | ||
| Grouping: rootAggregation.Grouping, | ||
| Without: rootAggregation.Without, | ||
| }, | ||
| }, shardNum, i) | ||
| nodes = append(nodes, node) | ||
| } | ||
| return nodes | ||
| } | ||
|
|
||
| // insertShardNum tags every vector selector in the subtree with the shard it | ||
| // belongs to. NOTE: the label used here does not match the merged | ||
| // metric-name-hash storage sharding; this is experimental. | ||
| func insertShardNum(root logicalplan.Node, shardCount int, shardIdx int) logicalplan.Node { | ||
| logicalplan.TraverseBottomUp(nil, &root, func(parent, current *logicalplan.Node) bool { | ||
| if (*current).Type() == logicalplan.VectorSelectorNode { | ||
| cur := (*current).(*logicalplan.VectorSelector) | ||
| cur.LabelMatchers = append(cur.LabelMatchers, labels.MustNewMatcher(labels.MatchEqual, "__CORTEX_DQE_SHARD__", fmt.Sprintf("%d_%d", shardCount, shardIdx))) | ||
| } | ||
| return false | ||
| }) | ||
| return root | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package distributed_execution | ||
|
|
||
| import ( | ||
| "context" | ||
| ) | ||
|
|
||
| type fragmentMetadataKey struct{} | ||
|
|
||
| type fragmentMetadata struct { | ||
| queryID uint64 | ||
| fragmentID uint64 | ||
| childIDToAddr map[uint64]string | ||
| isRoot bool | ||
| } | ||
|
|
||
| // InjectFragmentMetaData stores the distributed execution metadata for the current | ||
| // fragment into the context. This metadata is propagated from the query-scheduler to | ||
| // the querier so that the querier knows which fragment it is executing, where to pull | ||
| // its child fragments' results from, and whether it is the root (coordinator) fragment. | ||
| func InjectFragmentMetaData(ctx context.Context, fragmentID uint64, queryID uint64, isRoot bool, childIDToAddr map[uint64]string) context.Context { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is context the right place to put this? I'd like to understand the usecase better. |
||
| return context.WithValue(ctx, fragmentMetadataKey{}, fragmentMetadata{ | ||
| queryID: queryID, | ||
| fragmentID: fragmentID, | ||
| childIDToAddr: childIDToAddr, | ||
| isRoot: isRoot, | ||
| }) | ||
| } | ||
|
|
||
| // ExtractFragmentMetaData retrieves the distributed execution metadata for the current | ||
| // fragment from the context. The final return value reports whether metadata was present. | ||
| func ExtractFragmentMetaData(ctx context.Context) (isRoot bool, queryID uint64, fragmentID uint64, childAddrs map[uint64]string, ok bool) { | ||
| metadata, ok := ctx.Value(fragmentMetadataKey{}).(fragmentMetadata) | ||
| if !ok { | ||
| return false, 0, 0, nil, false | ||
| } | ||
| return metadata.isRoot, metadata.queryID, metadata.fragmentID, metadata.childIDToAddr, true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package distributed_execution | ||
|
|
||
| import ( | ||
| "context" | ||
| "reflect" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestFragmentMetadata(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| queryID uint64 | ||
| fragID uint64 | ||
| isRoot bool | ||
| childIDs []uint64 | ||
| childAddr []string | ||
| }{ | ||
| { | ||
| name: "basic test", | ||
| queryID: 123, | ||
| fragID: 456, | ||
| isRoot: true, | ||
| childIDs: []uint64{1, 2, 3}, | ||
| childAddr: []string{"addr1", "addr2", "addr3"}, | ||
| }, | ||
| { | ||
| name: "empty children", | ||
| queryID: 789, | ||
| fragID: 101, | ||
| isRoot: false, | ||
| childIDs: []uint64{}, | ||
| childAddr: []string{}, | ||
| }, | ||
| { | ||
| name: "single child", | ||
| queryID: 999, | ||
| fragID: 888, | ||
| isRoot: true, | ||
| childIDs: []uint64{42}, | ||
| childAddr: []string{"10.0.0.1:8080"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // injection | ||
| ctx := context.Background() | ||
|
|
||
| childIDToAddr := make(map[uint64]string) | ||
| for i, childID := range tt.childIDs { | ||
| childIDToAddr[childID] = tt.childAddr[i] | ||
| } | ||
| newCtx := InjectFragmentMetaData(ctx, tt.fragID, tt.queryID, tt.isRoot, childIDToAddr) | ||
|
|
||
| // extraction | ||
| isRoot, queryID, fragmentID, childAddrs, ok := ExtractFragmentMetaData(newCtx) | ||
|
|
||
| // verify results | ||
| if !ok { | ||
| t.Error("ExtractFragmentMetaData failed, ok = false") | ||
| } | ||
|
|
||
| if isRoot != tt.isRoot { | ||
| t.Errorf("isRoot = %v, want %v", isRoot, tt.isRoot) | ||
| } | ||
|
|
||
| if queryID != tt.queryID { | ||
| t.Errorf("queryID = %v, want %v", queryID, tt.queryID) | ||
| } | ||
|
|
||
| if fragmentID != tt.fragID { | ||
| t.Errorf("fragmentID = %v, want %v", fragmentID, tt.fragID) | ||
| } | ||
|
|
||
| // create expected childIDToAddr map | ||
| expectedChildAddrs := make(map[uint64]string) | ||
| for i, childID := range tt.childIDs { | ||
| expectedChildAddrs[childID] = tt.childAddr[i] | ||
| } | ||
|
|
||
| if !reflect.DeepEqual(childAddrs, expectedChildAddrs) { | ||
| t.Errorf("childAddrs = %v, want %v", childAddrs, expectedChildAddrs) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestExtractFragmentMetaDataWithEmptyContext(t *testing.T) { | ||
| ctx := context.Background() | ||
| isRoot, queryID, fragmentID, childAddrs, ok := ExtractFragmentMetaData(ctx) | ||
| if ok { | ||
| t.Error("ExtractFragmentMetaData should return ok=false for empty context") | ||
| } | ||
| if isRoot || queryID != 0 || fragmentID != 0 || childAddrs != nil { | ||
| t.Error("ExtractFragmentMetaData should return zero values for empty context") | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a separate node for this?
I'd expect sum(metric) etc to be optimized into: