-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat: improve performance of first_over_time
and last_over_time
queries by sharding them
#11605
Changes from all commits
de536b6
235e386
689c161
09f6d66
f7463ae
750cec7
9281f44
8f589fe
810b07c
5e106bf
7b3cadc
b10c1de
928ccfa
f89ea65
5ddf9df
104892b
67dd511
8e22df0
6404308
0d420dc
f8c9c2b
2bfd0b5
af67edd
361babc
2fb80d7
1b6cc02
1f04c45
d2b055b
87bbd75
307bfc1
4dc234f
995b982
5231b6f
9e10b2c
2325e85
69f504b
27a5d66
1b09ad3
bd7f275
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 |
---|---|---|
|
@@ -65,6 +65,10 @@ func TestMappingEquivalence(t *testing.T) { | |
`, | ||
false, | ||
}, | ||
{`first_over_time({a=~".+"} | logfmt | unwrap value [1s])`, false}, | ||
{`first_over_time({a=~".+"} | logfmt | unwrap value [1s]) by (a)`, false}, | ||
{`last_over_time({a=~".+"} | logfmt | unwrap value [1s])`, false}, | ||
{`last_over_time({a=~".+"} | logfmt | unwrap value [1s]) by (a)`, false}, | ||
// topk prefers already-seen values in tiebreakers. Since the test data generates | ||
// the same log lines for each series & the resulting promql.Vectors aren't deterministically | ||
// sorted by labels, we don't expect this to pass. | ||
|
@@ -141,7 +145,7 @@ func TestMappingEquivalenceSketches(t *testing.T) { | |
query string | ||
realtiveError float64 | ||
}{ | ||
{`quantile_over_time(0.70, {a=~".+"} | logfmt | unwrap value [1s]) by (a)`, 0.03}, | ||
{`quantile_over_time(0.70, {a=~".+"} | logfmt | unwrap value [1s]) by (a)`, 0.05}, | ||
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. did this need to change from 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. When I change the test series to have the values spread out this would fail. 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. which change is that? the addition of nanos in the timestamp within 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. Yes. |
||
{`quantile_over_time(0.99, {a=~".+"} | logfmt | unwrap value [1s]) by (a)`, 0.02}, | ||
} { | ||
q := NewMockQuerier( | ||
|
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.
is it possible that a shard which only retrieves data for a single relevant stream could return a vector? or would the vector for that single series always be wrapped in a matrix?
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.
It would be a matrix with one vector I believe. Only for instant queries we return a vector https://github.com/grafana/loki/blob/main/pkg/logql/engine.go#L388