-
Notifications
You must be signed in to change notification settings - Fork 832
scheduler: logical plan fragmenter #7018
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?
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 |
---|---|---|
|
@@ -6,9 +6,12 @@ import ( | |
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/cortexproject/cortex/pkg/util/logical_plan" | ||
"github.com/cortexproject/cortex/pkg/distributed_execution" | ||
) | ||
|
||
// Tests fragmentation of logical plans, verifying that the fragments contain correct metadata. | ||
// Note: The number of fragments is determined by the distributed optimizer's strategy - | ||
// if the optimizer logic changes, this test will need to be updated accordingly. | ||
func TestFragmenter(t *testing.T) { | ||
type testCase struct { | ||
name string | ||
|
@@ -19,8 +22,6 @@ func TestFragmenter(t *testing.T) { | |
} | ||
|
||
now := time.Now() | ||
|
||
// more tests will be added when distributed optimizer and fragmenter are implemented | ||
tests := []testCase{ | ||
{ | ||
name: "simple logical query plan - no fragmentation", | ||
|
@@ -29,18 +30,37 @@ func TestFragmenter(t *testing.T) { | |
end: now, | ||
expectedFragments: 1, | ||
}, | ||
{ | ||
name: "binary operation with aggregations", | ||
query: "sum(rate(node_cpu_seconds_total{mode!=\"idle\"}[5m])) + sum(rate(node_memory_Active_bytes[5m]))", | ||
start: now, | ||
end: now, | ||
expectedFragments: 3, | ||
}, | ||
{ | ||
name: "multiple binary operation with aggregations", | ||
query: "sum(rate(http_requests_total{job=\"api\"}[5m])) + sum(rate(http_requests_total{job=\"web\"}[5m])) + sum(rate(http_requests_total{job=\"cache\"}[5m])) + sum(rate(http_requests_total{job=\"db\"}[5m]))", | ||
start: now, | ||
end: now, | ||
expectedFragments: 7, | ||
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. The test should try to compare the output fragments instead of number of fragments to be safer |
||
}, | ||
} | ||
|
||
for _, tc := range tests { | ||
t.Run(tc.name, func(t *testing.T) { | ||
lp, err := logical_plan.CreateTestLogicalPlan(tc.query, tc.start, tc.end, 0) | ||
lp, err := distributed_execution.CreateTestLogicalPlan(tc.query, tc.start, tc.end, 0) | ||
require.NoError(t, err) | ||
|
||
fragmenter := NewDummyFragmenter() | ||
res, err := fragmenter.Fragment((*lp).Root()) | ||
fragmenter := NewPlanFragmenter() | ||
res, err := fragmenter.Fragment(uint64(1), (*lp).Root()) | ||
|
||
require.NoError(t, err) | ||
require.Equal(t, tc.expectedFragments, len(res)) | ||
|
||
// check the metadata of the fragments of binary expressions | ||
if len(res) == 3 { | ||
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. Why do we need this check? This only works for the second test case? |
||
require.Equal(t, []uint64{res[0].FragmentID, res[1].FragmentID}, res[2].ChildIDs) | ||
} | ||
}) | ||
} | ||
} |
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.
Nit. You can remove this else as we already return above