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

Refactoring: Create a common ring configuration #2391

Merged
merged 6 commits into from
Sep 12, 2023

Conversation

cyriltovena
Copy link
Contributor

@cyriltovena cyriltovena commented Sep 11, 2023

I was about to copy the code again for compaction and decided to end it here.

This basically refactors all component to use the same RingConfig struct. Added bonus the generated doc is now more consistent.

@cyriltovena cyriltovena marked this pull request as ready for review September 11, 2023 13:43
@cyriltovena cyriltovena requested review from a team as code owners September 11, 2023 13:43
pkg/validation/limits.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -38 to -49
-distributor.ring.heartbeat-period duration
Period at which to heartbeat to the ring. 0 = disabled. (default 5s)
-distributor.ring.heartbeat-timeout duration
The heartbeat timeout after which distributors are considered unhealthy within the ring. 0 = never (timeout disabled). (default 1m0s)
-distributor.ring.instance-addr string
IP address to advertise in the ring.
-distributor.ring.instance-id string
Instance ID to register in the ring. (default "<hostname>")
-distributor.ring.instance-interface-names string
Name of network interface to read address from. (default [<private network interfaces>])
-distributor.ring.instance-port int
Port to advertise in the ring (defaults to server.http-listen-port). (default 4040)
Copy link
Collaborator

@kolesnikovae kolesnikovae Sep 12, 2023

Choose a reason for hiding this comment

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

Just to double check: we're hiding these config options from the short output, not removing them, right?

Copy link
Contributor Author

@cyriltovena cyriltovena Sep 12, 2023

Choose a reason for hiding this comment

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

Correct those are removed from the normal output (all has it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants