This repository was archived by the owner on Jul 28, 2026. It is now read-only.
[dev.scraping-service-scalability] Fork the cortex ring/kv package for use with the configstore - #927
Merged
rfratto merged 2 commits intoSep 21, 2021
Conversation
The cortex ring/kv package is focused heavily on a hash ring, making it increasingly inappropriate to use as a KV store for the configstore, which expects a ton of traffic. This initial PR does a basic copy of the cortex package into configstore/kv. Initial changes from the Cortex package: - Memberlist is removed - cortex_ metrics have been reprefixed with agent_
Contributor
|
You probably want to fork the one in dskit which has recently had some work done to strip down its dependencies: grafana/dskit#43 |
Contributor
Author
Thanks for the suggestion! I had considered this, but even if nothing has really changed, I want to be extra certain (for now) that we remain as close as possible to the existing code. I'll definitely want to revisit this after 0.19.0 and see if we can pull over stuff from dskit instead. |
Contributor
Author
|
I'm a little surprised the macOS tests are failing since I didn't really change anything. I'll look into it more tomorrow morning, but otherwise this should be a pretty small thing to review. |
rfratto
force-pushed
the
fork-cortex-kv
branch
from
September 20, 2021 21:54
223341c to
fe3089b
Compare
rfratto
force-pushed
the
fork-cortex-kv
branch
from
September 20, 2021 22:04
fe3089b to
58be14f
Compare
rfratto
changed the title
rfratto
merged commit Sep 21, 2021
d0e4332
into
grafana-cold-storage:dev.scraping-service-scalability
3 tasks
7 tasks
github-actions
Bot
added
the
frozen-due-to-age
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
The cortex kv package is focused heavily on a hash ring, making it increasingly inappropriate to use as a KV store for the configstore, which has different expectations about how it will be consumed. This initial PR does a basic copy
of the cortex package (from the specific version of Cortex we're already depending on) into configstore.
Initial changes from the Cortex package:
This is a medium-term fix. I do not anticipate that the configstore package will exist forever, but we need a way to make it performant for now until we have something long-term in place.
Which issue(s) this PR fixes
This also unveiled a bug in updating the cluster config. Oops!
Notes to the Reviewer
This PR also immediately uses the new package, which doesn't require much except to point at the new location.
PR Checklist