Skip to content
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

Enable native histograms fully on the read path #4513

Merged
merged 6 commits into from
Mar 17, 2023

Conversation

krajorama
Copy link
Contributor

@krajorama krajorama commented Mar 16, 2023

What this PR does

Now that the query-frontend can receive the implicit native histograms via protobuf, we can enable native histograms fully.

Remove previous restriction via limit.

Which issue(s) this PR fixes or relates to

Related to: #3478
Fixes: #3992

Checklist

  • Tests updated
  • N/A Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Now that the query-frontend can receive the implicit native histograms
via protobuf, we can enable native histograms fully.

Remove previous restriction via limit.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama requested a review from a team as a code owner March 16, 2023 07:25
@krajorama krajorama marked this pull request as draft March 16, 2023 07:25
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some minor comments, but lgtm overall.

integration/querier_sharding_test.go Outdated Show resolved Hide resolved
integration/querier_sharding_test.go Outdated Show resolved Hide resolved
@@ -515,14 +535,25 @@ func testMetadataQueriesWithBlocksStorage(
firstSeriesInIngesterHead prompb.TimeSeries,
blockRangePeriod time.Duration,
) {
// This is hacky, don't use for anything serious
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's worse code out there :)

integration/query_frontend_test.go Show resolved Hide resolved
integration/querier_sharding_test.go Outdated Show resolved Hide resolved
integration/query_frontend_cache_test.go Outdated Show resolved Hide resolved
integration/query_frontend_cache_test.go Outdated Show resolved Hide resolved
pkg/frontend/querymiddleware/querysharding.go Show resolved Hide resolved
@krajorama krajorama marked this pull request as ready for review March 16, 2023 17:40
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama enabled auto-merge (squash) March 17, 2023 07:32
@krajorama krajorama merged commit 4eff59b into main Mar 17, 2023
@krajorama krajorama deleted the krajo/from-sparsehistograms2 branch March 17, 2023 07:32
krajorama added a commit that referenced this pull request Mar 17, 2023
Followup to #4513

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama added a commit that referenced this pull request Mar 17, 2023
* Add query format to native histograms flag description

Followup to #4513

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable query sharding when tenant accepts native histograms
3 participants