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

Manipulate ingesters query min time when -querier.query-ingesters-within is set #2904

Conversation

pracucci
Copy link
Contributor

What this PR does:
When running the Cortex blocks storage, the blocks retention in the ingesters may be way higher than the configured -querier.query-ingesters-within (eg. in the cortex-mixin we do have a default of 4 days blocks retention in the ingesters, to give operators enough time to recover from any issue causing blocks not being shipped from ingesters to storage, in case of an outage).

Running the blocks storage with a -querier.query-ingesters-within significantly higher than the retention means that (under normal conditions) all samples older than -querier.query-store-after and more recent than -querier.query-ingesters-within are both fetched from ingesters and storage, causing an undesired extra load on ingesters.

Similarly to what we do with -querier.query-store-after in the blocks storage, in this PR I'm proposing to manipulate the min time range of the query sent to ingesters to guarantee not being older than -querier.query-ingesters-within. This change applies both to chunks and blocks storage.

Which issue(s) this PR fixes:
N/A

Checklist

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

@pracucci pracucci requested a review from pstibrany July 21, 2020 09:47
Copy link
Contributor

@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.

Nice work, LGTM.

@pstibrany
Copy link
Contributor

pstibrany commented Jul 28, 2020

My only concern is that test seems to rely on time.Now() in test and time.Now() in distributor queryable to not cross the 1s boundary, and may be flaky.

Sorry for the noise, I can see now that test checks for expected times with allowed delta of 5ms. Furthermore, it's not 1s but 1ms boundary that I was thinking about. Forget my comment.

@pracucci pracucci force-pushed the manipulate-query-mint-based-on-query-ingesters-within branch from 586bc24 to 949192b Compare July 28, 2020 10:20
@pracucci pracucci merged commit 73cc255 into cortexproject:master Jul 28, 2020
@pracucci pracucci deleted the manipulate-query-mint-based-on-query-ingesters-within branch July 28, 2020 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants