Skip to content

Conversation

SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Jan 31, 2025

The #6276 exposes consistency check attempts as a flag; this PR emits more histogram buckets on the cortex-querier_storegateway_refetches_per_query metric.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@SungJin1212 SungJin1212 force-pushed the Emit-more-buckets-for-cortex-querier_storegateway_refetches_per_query branch 4 times, most recently from b50d72c to e50e81d Compare January 31, 2025 08:42
friedrichg
friedrichg previously approved these changes Jan 31, 2025
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 31, 2025
@@ -123,7 +123,7 @@ func newBlocksStoreQueryableMetrics(reg prometheus.Registerer) *blocksStoreQuery
Namespace: "cortex",
Name: "querier_storegateway_refetches_per_query",
Help: "Number of re-fetches attempted while querying store-gateway instances due to missing blocks.",
Buckets: []float64{0, 1, 2},
Buckets: []float64{0, 1, 2, 4, 8, 16},
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a customized value of 5 retries. I don't know if it makes sense to have 16 retries. Shall we keep upper limit to 8?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

That's right. But I don't think we should recommend users to retry up to 16 times. I mean in general 3 to 7 retries make the most of sense. But maybe that's our usecase.

Do you have a usecase of using that many retries?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, you're right. let's fix it.

@SungJin1212 SungJin1212 force-pushed the Emit-more-buckets-for-cortex-querier_storegateway_refetches_per_query branch from e50e81d to b3d3e16 Compare February 3, 2025 01:06
@friedrichg friedrichg dismissed their stale review February 3, 2025 19:41

there are pending questions

@SungJin1212 SungJin1212 force-pushed the Emit-more-buckets-for-cortex-querier_storegateway_refetches_per_query branch 2 times, most recently from 4880120 to e85f1f9 Compare February 4, 2025 01:23
@SungJin1212 SungJin1212 force-pushed the Emit-more-buckets-for-cortex-querier_storegateway_refetches_per_query branch from e85f1f9 to ed3a42a Compare February 4, 2025 21:48
@yeya24 yeya24 merged commit caaddfb into cortexproject:master Feb 6, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/querier lgtm This PR has been approved by a maintainer size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants