-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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: Add a ring to the compactor used to control concurrency when not running standalone #4574
Conversation
All: {QueryScheduler, QueryFrontend, Querier, Ingester, Distributor, Ruler}, | ||
Read: {QueryScheduler, QueryFrontend, Querier, Ruler}, | ||
All: {QueryScheduler, QueryFrontend, Querier, Ingester, Distributor, Ruler, Compactor}, | ||
Read: {QueryScheduler, QueryFrontend, Querier, Ruler, Compactor}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reasons for putting the compactor on the read path:
- having consistent performance on all write nodes is more critical than read nodes
- the compactor is a read path optimization and is not critical to writes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a bunch of comments, but I think I'm fine with the compactor always depending on the ring, even when run with -target=scheduler
. I believe it's worse to maintain two code paths and I suspect we'll eventually need it anyway for horizontally scalable compaction.
e4d098f
to
8fffd56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
…rings to persist tokens
…other than a scheduler in the future.
3619395
to
584a47c
Compare
What this PR does / why we need it:
To simplify operations, we want to enable a horizontally scalable "all" target as well as the new "read" "write" targets, without having to run a separate compactor.
This PR adds a ring to the compactor which is used for leader election by looking to see which instance has a specific key in the ring.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist