-
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
Approximate quantile_over_time
#10417
Merged
jeschkies
merged 125 commits into
grafana:main
from
jeschkies:karsten/quantile-approximation
Dec 11, 2023
Merged
Changes from 1 commit
Commits
Show all changes
125 commits
Select commit
Hold shift + click to select a range
4ea54f7
Vendor tdigest and ddsketch.
jeschkies 9768704
Make StepEvaluator result generic.
jeschkies 8303daa
Define tdigest batch iterator.
jeschkies a0b6b55
Try generics.
jeschkies 5127581
Join tdigest vector into a tdigest matrix.
jeschkies b56cfc2
Start testing.
jeschkies a15bdc5
Add TDigestEvalExpr and TDigestMergeExpr.
jeschkies da18856
Define TDigestMergeExpr.
jeschkies 2ab4285
implement interface for quantile sketch, which can be backed by tdigest
cstyan 09dfd87
Merge and evaluate tdigest sketches.
jeschkies 1c45001
Skip optimization.
jeschkies b052d70
Introduce quantile sketch expr.
jeschkies f095b02
Parse query before execution.
jeschkies 0f53ba9
Run tests.
jeschkies bee822d
Use more samples.
jeschkies a8e1cc4
Correct tests.
jeschkies 2359aac
Create separate test for sketches with relative error.
jeschkies 4d71095
Satisfy linter.
jeschkies dfaca21
Remove unused code.
jeschkies e35b769
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 2e053c4
quantile_over_time should be sharded without grouping.
jeschkies a656d7a
Use constant for "true"
jeschkies fa92688
Format code.
jeschkies fe403ca
go mod ...
jeschkies 94923ea
Use StepResult interface instead of generics.
jeschkies c526f24
Format code.
jeschkies c27e5c3
Fix remaining tests.
jeschkies 96e0ea2
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies b6023cb
Return parsed query from parameters.
jeschkies 697d5de
Revert "Return parsed query from parameters."
jeschkies 24ed1a4
Introduce __quantile_sketch_over_time__
jeschkies a600de2
Remove parsed query from parameters again.
jeschkies be3ba41
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies e4b035c
Move TDigest step evaluator creation into range step evaluator.
jeschkies 8e8246b
Rename step evaluator.
jeschkies a6d20c3
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies e468e1f
Clean up a little.
jeschkies c8426b2
Start protobuf support for quantile sketches.
jeschkies 1956623
Define quantile sketch response.
jeschkies 0a80f4e
Serialize qunatile matrix.
jeschkies 3bdcced
Stub quantile matrix deserialization.
jeschkies e1fede8
Unmarshal tdigest sketch.
jeschkies 8784561
Clean build.
jeschkies ffa7b6a
Always shard queries.
jeschkies 2ebb71c
Use point receiver.
jeschkies fde4d6a
Format generated protos.
jeschkies 6fa5680
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 401699e
Merge remote-tracking branch 'refs/remotes/origin/karsten/quantile-ap…
jeschkies 4d1e839
Revert Makefile.
jeschkies 9afab86
Correct generated protos files.
jeschkies b469eac
Merge branch 'main' into karsten/quantile-approximation
cstyan f49f269
need to handle error that can be returned by QuantileSketchFromProto
cstyan 098ec7f
move quantile test so there's no import cycle
cstyan afc9fd3
fix lint issue in tdigest
cstyan 6c8647e
fix import order
cstyan ffabb0d
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies cdd91a9
Resolve broken merge
jeschkies 6a493d8
Override expression
jeschkies 1180a1e
Remove private expressions
jeschkies 64ba4f0
Clone AST
jeschkies 69818d8
Remove usages of ParseExpr.
jeschkies 299f105
Send AST to ingester as well
jeschkies 2cf808b
Avoid import cycle
jeschkies 8d8a6ea
Pass AST/Plan to ingester
jeschkies add38d6
Set plan for sample query
jeschkies ab8dbe1
Pass engine tests
jeschkies db134f2
Fix multi tenant querier
jeschkies d9e7f32
Fix querier tests
jeschkies d2af56d
Fix a few limits test
jeschkies 8b196a0
Fix most roundtrip tests
jeschkies 9aaf295
fix split by interval tests
jeschkies a4ffcb4
Format protos
jeschkies ffec435
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies aad5dbc
Satisfy linter
jeschkies 13efa17
Fix storage tests except for catch all
jeschkies d3d76d4
Satisfy linter
jeschkies 62d6025
Try ingester fix
jeschkies 8a88b4f
Parse expression
jeschkies 6c3f94a
Test frontend with protobuf.
jeschkies b8e718e
Use custom message
jeschkies df4f188
Use custom proto message
jeschkies 2fbad9d
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 0f5df0a
Parse matcher
jeschkies 3fe99f8
Do not parse empty query
jeschkies 1e82d45
Do no parse empty matcher string
jeschkies 8f8785b
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies fa81165
Add feature flag
jeschkies 426d4fd
Add changelog entry
jeschkies 2370a73
Rename t-digest
jeschkies 0b98afa
Verify sketches have the same timestamp
jeschkies 0eeaf95
set AST to nil
jeschkies 4010d5f
Handle errors on sketch merge
jeschkies 37dc194
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 0163d8c
Address PR feedback
jeschkies efa60d8
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 77602d6
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 8cde54e
Pass comma-separated list
jeschkies 0af057c
Fix clone bug
jeschkies ef88960
Test clone of noop filter
jeschkies a43581e
Test string label filter clone
jeschkies 3b8a1d5
Check for empty query plan
jeschkies 18e205d
Extract magic number for explain
jeschkies 99a7762
Rename vlaidtion rule
jeschkies 509adc4
Comment why plan can be empty
jeschkies 80788a8
Remove proto option
jeschkies 3a04ac7
Rename sketch models
jeschkies 5e3d04f
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 07f654c
Make doc
jeschkies f42aed4
Test result to response
jeschkies a1174df
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 39a5b2f
Test quantile matrix roundtriop
jeschkies db33dc9
Rename from proto method
jeschkies db511ad
Test probabilisitc quantile serialization
jeschkies 274a1e1
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies bdc8e1e
Merge branch 'main' into karsten/quantile-approximation
jeschkies fe9c94a
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 1dc2c81
Update shard aggregations
jeschkies 121db5d
Address some comments
jeschkies 75ad077
Check for empty query plan
jeschkies c0eaf3d
Test a few error cases
jeschkies 48ebd00
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies fbdd7d6
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies 95aba11
Merge branch 'main' into karsten/quantile-approximation
jeschkies 158e10a
Handle empty plan
jeschkies 2572167
Merge remote-tracking branch 'grafana/main' into karsten/quantile-app…
jeschkies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -912,6 +912,7 @@ func (i *Ingester) QuerySample(req *logproto.SampleQueryRequest, queryServer log | |
| _, ctx := stats.NewContext(queryServer.Context()) | ||
| sp := opentracing.SpanFromContext(ctx) | ||
|
|
||
| // If the plan is empty we want all series to be returned. | ||
| if req.Plan == nil { | ||
|
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. can we include comment about how we can end up with a nil plan here? 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. Done. |
||
| parsed, err := syntax.ParseSampleExpr(req.Selector) | ||
| if err != nil { | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
so if we have a nil plan here it's intentional?
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.
Yes.