-
Notifications
You must be signed in to change notification settings - Fork 524
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
ingester: ActiveSeriesRequest can return native histograms information #7986
Conversation
b8fdaa7
to
1b4938c
Compare
1b4938c
to
7660981
Compare
7660981
to
8539e11
Compare
Allow ActiveSeriesRequest to return only native histograms and include active bucket count in the label __nh_bucket_count__ Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
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.
Looks good to me. Do you have benchmark results to post by any chance?
This comment was marked as outdated.
This comment was marked as outdated.
8539e11
to
e19a6b2
Compare
Labels are already packed due to tag==stringlabels in TSDB so Add() doesn't work on them. And after they are decoded for the response we'd have to insert the bucket count label into the correctly sorted position which would mean never being able to just send the encoded labels. Instead of adding a new interface I'll add a backwards compatible array of integers as a compromise. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
I had to change the implementation to account for tag=stringlabels
|
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.
Looks good to me with some small nits.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
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.
LGTM.
What this PR does
ingester: ActiveSeriesRequest can return native histograms information
The interface
ActiveSeries(ActiveSeriesRequest)
, gets a newType
field that requests active series by default (0) but can request native histograms (1). In case of histograms the new bucket_count array is filled with bucket counts in the response. Both these changes should be backwards compatible, enabling a smooth upgrade.Internally I flipped the order of postings iterators from sharding over active series to filtering active native histogram series over sharding to be able to get the bucket counts from the postings iterator directly. Also removes the use of the intermediate postings list to be able to directly stream the results.
Two alternative were considered:
Which issue(s) this PR fixes or relates to
Related to #7981
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.