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.

Sign up for GitHub

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

Jump to bottom

Expose more series size stats and use in lazy posting #7957

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Expose more series size stats and use in lazy posting #7957

wants to merge 2 commits into from

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Dec 4, 2024 •
edited
Loading

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Fixes #7955

Changes

  • Add new stats field to meta.json index_stats section, including series_p90_size, series_p99_size, series_p999_size and series_p9999_size.
  • Compactor will try to calculate p90 to p9999 quantile series size and write the stats to meta.json.
  • Add new flag estimated-series-size-stat to configure which stats to use for lazy posting. By default it is max but users can configure their own to p90 to p9999. I didn't call this flag lazy-expanded-posting-estimated-series-size-stat because this estimated series size might be used by other features in the future for estimation.
  • Lazy expanded posting uses the value above for series estimation.

Note:
For quantile series size calculation, I have tried both go-tdigest (already used by hedged requests feature) and DataDog/sketches-go. ddsketches-go seems 10s faster than go-tdigest when processing a block with 20M series. So I go with ddsketches-go in the implementation for performance.

Verification

Existing tests should still pass

@yeya24 yeya24 changed the title Expose more series size stats Expose more series size stats and use in lazy posting Dec 4, 2024
@yeya24 yeya24 force-pushed the expose-series-stats1 branch from 06c8d30 to 447cad7 Compare December 4, 2024 18:08
@yeya24 yeya24 requested a review from MichaHoffmann December 4, 2024 18:14
GiedriusS
GiedriusS reviewed Dec 5, 2024
View reviewed changes
docs/components/store.md
@@ -94,6 +94,12 @@ Flags:
--no-cache-index-header option is specified.
--enable-auto-gomemlimit Enable go runtime to automatically limit memory
consumption.
--estimated-series-size-stat=max
Copy link
Member

Choose a reason for hiding this comment

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

Same question as in the other PR: how does one know which choice is the "correct" one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

We expose metrics about how much lazy posting optimized and how much we overfetch for series. If users want to optimize lazy posting more then they can use a smaller percentile stat.

It is hard to say correct or not. It is more like how to tune lazy posting to make it optimize more scenarios. In general, a block with 20M series, it is P9999 size should be only smaller than 2000 series. It is probably more like how much I am able to tolerate if things go wrong. And users can play with this config and the metrics above.

@yeya24
Copy link
Contributor Author

yeya24 commented Dec 9, 2024

Can you please take a look at this PR? @GiedriusS @MichaHoffmann

@yeya24 yeya24 force-pushed the expose-series-stats1 branch from 4f11043 to 95b8c17 Compare December 10, 2024 07:26
@yeya24
add stddev
bb3c830 
Signed-off-by: Ben Ye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GiedriusS GiedriusS GiedriusS left review comments

@MichaHoffmann MichaHoffmann Awaiting requested review from MichaHoffmann

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Store Gateway: use different stats for lazy posting series size estimation
2 participants
@yeya24 @GiedriusS