Skip to content

change: separate distributor into api and service module #3112

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

Merged
merged 5 commits into from
Sep 1, 2020

Conversation

jtlisi
Copy link
Contributor

@jtlisi jtlisi commented Aug 31, 2020

What this PR does:

  • Decouples the distributor service from the API

Which issue(s) this PR fixes:

Fixes #3109

TLDR: Querier will no longer host distributor endpoints

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
jtlisi added 2 commits August 31, 2020 17:21
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Good idea.

mm.RegisterModule(Distributor, t.initDistributor)
mm.RegisterModule(Distributor, nil)
mm.RegisterModule(DistributorAPI, t.initDistributorAPI)
mm.RegisterModule(DistributorService, t.initDistributorService)
Copy link
Contributor

Choose a reason for hiding this comment

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

DistributorService should use modules.UserInvisibleModule option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense

API: {Server},
Ring: {API, RuntimeConfig, MemberlistKV},
Overrides: {RuntimeConfig},
Distributor: {DistributorService, DistributorAPI},
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need both Distributor and DistributorAPI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the DistributorAPI can be removed and the route registration can be moved to the Distributor module

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

LGTM.

@jtlisi jtlisi merged commit fa3f32a into master Sep 1, 2020
@jtlisi jtlisi deleted the 20200831_decouple_distributor_from_api branch September 1, 2020 19:06
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.

Distributor endpoints hosted for Ruler and Querier
2 participants