Skip to content

rulers: Add support to persist tokens in rulers #5987

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.

Sign up for GitHub

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 6 commits into from
May 31, 2024
Merged

rulers: Add support to persist tokens in rulers #5987

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c8fd360
Feat: Add support to store tokens in rulers
rapphil May 30, 2024
0037fd1
Add changelog
rapphil May 30, 2024
d030c6b
Add new flag to v1 guarantees documentation
rapphil May 30, 2024
cd0246f
Fix lint error in documentation
rapphil May 31, 2024
883c08e
Make changelog consistent with convention
rapphil May 31, 2024
6337a96
Fix lint error in documentation
rapphil May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Changelog

## master / unreleased

* [ENHANCEMENT] rulers: Add support to persist tokens in rulers. #5987
* [CHANGE] Upgrade Dockerfile Node version from 14x to 18x. #5906
* [CHANGE] Ingester: Remove `-querier.query-store-for-labels-enabled` flag. Querying long-term store for labels is always enabled. #5984
* [ENHANCEMENT] Query Frontend/Querier: Added store gateway postings touched count and touched size in Querier stats and log in Query Frontend. #5892
Expand Down
5 changes: 5 additions & 0 deletions docs/configuration/config-file-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4262,6 +4262,11 @@ ring:
# CLI flag: -ruler.ring.zone-awareness-enabled
[zone_awareness_enabled: <boolean> | default = false]

# File path where tokens are stored. If empty, tokens are not stored at
# shutdown and restored at startup.
# CLI flag: -ruler.ring.tokens-file-path
[tokens_file_path: <string> | default = ""]

# Name of network interface to read address from.
# CLI flag: -ruler.ring.instance-interface-names
[instance_interface_names: <list of string> | default = [eth0 en0]]
Expand Down
1 change: 1 addition & 0 deletions pkg/ruler/ruler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ func enableSharding(r *Ruler, ringStore kv.Client) error {
// chained via "next delegate").
delegate := ring.BasicLifecyclerDelegate(r)
delegate = ring.NewLeaveOnStoppingDelegate(delegate, r.logger)
delegate = ring.NewTokensPersistencyDelegate(r.cfg.Ring.TokensFilePath, ring.JOINING, delegate, r.logger)
delegate = ring.NewAutoForgetDelegate(r.cfg.Ring.HeartbeatTimeout*ringAutoForgetUnhealthyPeriods, delegate, r.logger)

rulerRingName := "ruler"
Expand Down
2 changes: 2 additions & 0 deletions pkg/ruler/ruler_ring.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type RingConfig struct {
HeartbeatTimeout time.Duration `yaml:"heartbeat_timeout"`
ReplicationFactor int `yaml:"replication_factor"`
ZoneAwarenessEnabled bool `yaml:"zone_awareness_enabled"`
TokensFilePath string `yaml:"tokens_file_path"`

// Instance details
InstanceID string `yaml:"instance_id" doc:"hidden"`
Expand Down Expand Up @@ -75,6 +76,7 @@ func (cfg *RingConfig) RegisterFlags(f *flag.FlagSet) {
f.DurationVar(&cfg.FinalSleep, "ruler.ring.final-sleep", 0*time.Second, "The sleep seconds when ruler is shutting down. Need to be close to or larger than KV Store information propagation delay")
f.IntVar(&cfg.ReplicationFactor, "ruler.ring.replication-factor", 1, "EXPERIMENTAL: The replication factor to use when loading rule groups for API HA.")
f.BoolVar(&cfg.ZoneAwarenessEnabled, "ruler.ring.zone-awareness-enabled", false, "EXPERIMENTAL: True to enable zone-awareness and load rule groups across different availability zones for API HA.")
f.StringVar(&cfg.TokensFilePath, "ruler.ring.tokens-file-path", "", "EXPERIMENTAL: File path where tokens are stored. If empty, tokens are not stored at shutdown and restored at startup.")
Copy link
Member

Choose a reason for hiding this comment

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

I am fine with this. Please include it in https://github.com/cortexproject/cortex/blob/e6a4e495ea2430eba8db8b7291b7944f03ab6471/docs/configuration/v1-guarantees.md too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why we need to mark this feature as experimental anymore. Thought the initial feature was added long time ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yeya24 are you referring to this new flag for rulers or something else? if the former, this feature does not exist in rulers and therefore should be experimental before stabilization?

Copy link
Contributor

Choose a reason for hiding this comment

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

For other components. I think it might be the same for Ruler.
But I am okay to mark this as experimental for now. We can clean it up later in #5922


// Instance flags
cfg.InstanceInterfaceNames = []string{"eth0", "en0"}
Expand Down