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

Loki: Revert distributor defaulting to inmemory #4638

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

DylanGuedes
Copy link
Contributor

@DylanGuedes DylanGuedes commented Nov 3, 2021

What this PR does / why we need it:

  • Reverts PR 4440, which modified the distributor ring to inmemory (instead of consul). This is inconsistent with other rings default behavior and is also not retrocompatible with previous versions.
    • That change was required at that time because, by changing the default rate limiting to global, a ring was necessary, and, since by default the ring KV was consul, the project couldn't run with the default configs. That's not the case anymore: right now we are defaulting to reusing the ingester config, which users already have properly configured otherwise they wouldn't be able to run the project anyway.

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

Special notes for your reviewer:
N/A

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

- [PR 4440](https://github.com/grafana/loki/pull/4440/files) modified
  the distributor setup to use inmemory by default (instead of consul).
That was necessary because, by changing the default rate limiting to
global, a ring was necessary, and since by default the ring was set to
consul, the project couldn't run with the default configs. That's not
the case anymore: right now we are reusing the same config set for the
ingester (which users already have properly configured otherwise they
wouldn't be able to run the project). List of changes:
  - Remove changelog entries
  - Remove upgrading guide entries
  - Remove added tests (unnecessary checks)
  - Remove value overriding
@DylanGuedes DylanGuedes marked this pull request as ready for review November 3, 2021 18:09
@owen-d owen-d merged commit 0d66319 into grafana:main Nov 3, 2021
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