feat: Introduce shardable probabilistic topk for instant queries. #14243
Conversation
| width := math.Ceil(math.E / epsilon) | ||
| depth := math.Ceil(math.Log(1.0 / delta)) |
There was a problem hiding this comment.
There was a problem hiding this comment.
this is the calculation I used when doing all of our experiments, but I can't remember if I got good results using it
There was a problem hiding this comment.
I figured we have to stay here because this makes the matrix roughly the size of our max 10k vector.
There was a problem hiding this comment.
@jeschkies what's the point of using the sketch if we're just going to send all the labels of everything we've observed in each sharded query over the wire?
If we're doing that then we might as well just have a list of tuples, labelset + count, and remove the sketch. Or use the map approach. We'd be trading space for not having to search into the slice with the map 🤷♂️
| width := math.Ceil(math.E / epsilon) | ||
| depth := math.Ceil(math.Log(1.0 / delta)) |
There was a problem hiding this comment.
this is the calculation I used when doing all of our experiments, but I can't remember if I got good results using it
@cstyan you are right. My rationale was that we could first validate if the team and our users accept instant queries with a special probabilistic key word and see what the implementation looks like. Then worry about the algorithm. As for the lables I'd keep it super simple and use a min-heap with max 10k labels. As Ed explained the most prominent use case is for finding the top outliers in a high cardinality set. |
github-actions
bot
added
sig/operator
area/helm
type/docs
There was a problem hiding this comment.
This is a bit hard to review with all the other unintended commits, but I think we can proceed with this implementation. I'm not entirely following how the labels tracking and heap are wired up ATM
Also, we should either clean up and reuse or completely remove the sketch/topk.go file as part of this PR.
|
|
||
| // Add our metric if we haven't seen it | ||
| if _, ok := v.observed[metricString]; !ok { | ||
| heap.Push(v, metric) |
There was a problem hiding this comment.
where does the heap actually exist?
There was a problem hiding this comment.
HeapCountMinSketchVector embeds a CountMinSketchVector and treats the metrics array as a heap. We should look into https://grafana.com/blog/2024/04/23/the-loser-tree-data-structure-how-to-optimize-merges-and-make-your-programs-run-faster/ actually.
jeschkies
force-pushed
the
karsten/ptopk
branch
from
0a04574
to
acb7754
Compare
|
It works 🎉 |
topk actually is. Signed-off-by: Callum Styan <callumstyan@gmail.com>
What this PR does / why we need it:
This change introduces a very simplified shardable topk approximation through the new vector aggregation
approx_topk.We use a count min sketch and track all labels not just the top
k. Since this list can grow quite large the feature is only supported for instant queries. Grouping is also not supported and should be handled by an innersum byorsum withouteven though this might not be the same behaviour astopk by.The sharding works by turning the
approx_topk(k, inner)query into the following expression:__count_min_sketch__is calculated for each shard and merge on the frontend.eval_cmsiterates through the labels list and determines the count for each.topkselects then the top items.The number of labels tracked on the querier side when evaluating
__count_min_sketch__is limited by the heap. It does count all values but the ke-value pairs might not be known.Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mdproduction/helm/loki/Chart.yamland updateproduction/helm/loki/CHANGELOG.mdandproduction/helm/loki/README.md. Example PRdeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR