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

Helm: Add support for dedicated ruler query path #7964

Conversation

alex5517
Copy link
Contributor

@alex5517 alex5517 commented Apr 25, 2024

What this PR does

This PR provides the option to deploy a read path dedicated to the ruler.
This allows you to scale user/dashboards queries independently from the ruler usage.

Another PR that does the same but has no activity #5081

Closes #5081

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.

@alex5517 alex5517 requested a review from a team as a code owner April 25, 2024 07:30
@alex5517 alex5517 changed the title Add support for dedicated ruler query path Helm: Add support for dedicated ruler query path Apr 25, 2024
@lasermoth
Copy link
Contributor

@dimitarvdimitrov Sorry for the ping, I wonder if you would have capacity to review this?

@dimitarvdimitrov
Copy link
Contributor

i'll take a look later this week; apologies for the delay. At a glance one thing that's missing is a test values file. This is one example. After creating the file and running make build-helm-tests there will all the resources generated from the values file. You can commit and push them. This doc has more details on what those manifests are useful for

@alex5517
Copy link
Contributor Author

@dimitarvdimitrov,

I added the tests as requested :)

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.

nice work! Thank you for cleaning this up and adding autoscaling. I only left a single suggestion regarding the outdated comments from the original PR

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! Thank you for your work @alex5517!

@dimitarvdimitrov dimitarvdimitrov merged commit a09f931 into grafana:main May 28, 2024
31 checks passed
@dimitarvdimitrov
Copy link
Contributor

This change will be included in the weekly 5.4.0-weekly.292 and in the next stable release - 5.4.0 or 6.0.0

@alex5517 alex5517 deleted the feat/helm-support-dedicated-ruler-read-path branch May 28, 2024 11:58
narqo pushed a commit to narqo/grafana-mimir that referenced this pull request Jun 6, 2024
* Add support for dedicated ruler query path

* Add feature to changelog

* Default to false

* Add ci test for ruler dedicated query path

* rename test file + build tests

* update comments

* Update operations/helm/charts/mimir-distributed/CHANGELOG.md

Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>

---------

Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
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.

3 participants