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

Store gateway spends most of the time waiting to load series and chun… #3927

Merged
merged 4 commits into from Jan 11, 2023

Conversation

duricanikolic
Copy link
Contributor

What this PR does

Store gateway spends most of the time waiting to load series and chunks. During that waiting time, a context.Canceled error might occur. The latter must not be handled as a consistency check, but should be propagated as context.Canceled.

Which issue(s) this PR fixes or relates to

Checklist

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

…ks. During that waiting time, a context.Canceled error might occur. The latter must not be handled as a consistency check, but should be propagated as context.Canceled.
@duricanikolic duricanikolic requested a review from a team as a code owner January 11, 2023 15:14
@CLAassistant
Copy link

CLAassistant commented Jan 11, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@colega colega left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! (I'd recommend reviewing with whitespace changes ignored).

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, nicely done!

pkg/querier/blocks_store_queryable_test.go Show resolved Hide resolved
pkg/querier/blocks_store_queryable_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing my comments

@56quarters 56quarters merged commit ad089a2 into main Jan 11, 2023
@56quarters 56quarters deleted the feature/fail-on-context-concel-in-receive branch January 11, 2023 17:06
@callumj
Copy link
Contributor

callumj commented Feb 3, 2023

@duricanikolic I've noticed that this consistency check can also show up when a Ruler runs an expensive rule against the SGW that results in an response from the SGW with rpc error: code = DeadlineExceeded desc = context deadline exceeded - should that also be considered as something that doesn't surface as a consistency check failure?

Conversation: https://grafana.slack.com/archives/C039863E8P7/p1675357321799079

@duricanikolic
Copy link
Contributor Author

@duricanikolic I've noticed that this consistency check can also show up when a Ruler runs an expensive rule against the SGW that results in an response from the SGW with rpc error: code = DeadlineExceeded desc = context deadline exceeded - should that also be considered as something that doesn't surface as a consistency check failure?

Conversation: https://grafana.slack.com/archives/C039863E8P7/p1675357321799079

@pracucci, @56quarters What do you think about this?

@pracucci
Copy link
Collaborator

pracucci commented Feb 6, 2023

It should have been fixed in #4007. @callumj does it happen in Mimir 2.6 too?

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.

None yet

7 participants