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

Querier: deprecates max-query-into-future #7496

Merged

Conversation

wilfriedroset
Copy link
Collaborator

What this PR does

max-query-into-future has been inherited from Cortex when it was running on chunks storage. Its purposes was to avoid errors when data was missing.

Since Mimir does not rely on chunks storage anymore, there is no errors nor performance penalty. As such, we can deprecate the setting.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

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

@wilfriedroset wilfriedroset force-pushed the deprecate-max-query-into-future branch from 64ab698 to cb1a39a Compare February 28, 2024 14:04
@wilfriedroset
Copy link
Collaborator Author

See slack for the extended context: https://grafana.slack.com/archives/C039863E8P7/p1709045532353379

@wilfriedroset wilfriedroset force-pushed the deprecate-max-query-into-future branch from cb1a39a to 9f66ab8 Compare February 28, 2024 14:16
@wilfriedroset wilfriedroset marked this pull request as ready for review February 28, 2024 16:27
@wilfriedroset wilfriedroset requested review from jdbaldry and a team as code owners February 28, 2024 16:27
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

Thanks! A few minor suggestions

`max-query-into-future` has been inherited from Cortex when it was
running on chunks storage. Its purposes was to avoid errors when data
was missing.

Since Mimir does not rely on chunks storage anymore, there is no errors
nor performance  penalty. As such, we can deprecate the setting.

Signed-off-by: Wilfried Roset <[email protected]>
@wilfriedroset wilfriedroset force-pushed the deprecate-max-query-into-future branch from 9f66ab8 to a5c3310 Compare February 28, 2024 20:44
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

docs LGTM

@dimitarvdimitrov
Copy link
Contributor

this doesn't work nicely with -store.max-labels-query-length and not setting start and end. Prometheus defaults start and end to math.MaxInt64 which is way far into the future. Then -store.max-labels-query-length clamps the start the query to X days before that, which is still very far into the future. The result is that the query doesn't return any results.

@wilfriedroset
Copy link
Collaborator Author

Sorry about the delay @dimitarvdimitrov I was on PTO 😅
This is totally unexpected, I see you have submitted a PR.
let's followup on it.

@dimitarvdimitrov
Copy link
Contributor

no need to apologise 😄 I was just leaving a trace in the PR

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.

4 participants