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

jsonnet: set unregister_on_shutdown for store-gateway #4713

Conversation

dimitarvdimitrov
Copy link
Contributor

What this PR does

This was only set for the multi-zone StatefulSets, but the regular
StatefulSets could also benefit from less resharding during rollouts.

This is already being done in helm with #4690
so this PR also changes jsonnet.

Another option is to change the default value of the parameter in Mimir
itself.

Signed-off-by: Dimitar Dimitrov dimitar.dimitrov@grafana.com

Checklist

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

This was only set for the multi-zone StatefulSets, but the regular
StatefulSets could also benefit from less resharding during rollouts.

Another option is to change the default value of the parameter in Mimir
itself.

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Another option is to change the default value of the parameter in Mimir
itself.

I discourage this because may make trying Mimir just a bit harder (need to manually forget store-gateways from the ring when scaling them down).

CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov
Copy link
Contributor Author

I discourage this because may make trying Mimir just a bit harder (need to manually forget store-gateways from the ring when scaling them down).

FWIW the docs for scaling out mention this option. But maybe an extra step isn't desirable.

I don't have an opinion - this is more trying to keep jsonnet and helm in sync. If it's preferable to have this as true, then I can undo the change in helm.

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@pracucci
Copy link
Collaborator

Another option is to change the default value of the parameter in Mimir
itself.

I discourage this because may make trying Mimir just a bit harder (need to manually forget store-gateways from the ring when scaling them down).

My comment was about changing the default value in Mimir. I think this PR is correct and we should keep jsonnet == Helm, which is unregister_on_shutdown = false.

@dimitarvdimitrov
Copy link
Contributor Author

oh, sorry, i had overlooked that. In this case I'll merge this PR now. Thanks for the review

@dimitarvdimitrov dimitarvdimitrov merged commit 51d18b5 into main Apr 14, 2023
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/jsonnet-dont-unregister-store-gateway-on-shutdown branch April 14, 2023 08:50
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