From 09047fe4cc110ec9719aff328c1064bfbaee624c Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 31 Jan 2022 17:29:25 +0100 Subject: [PATCH] Allow to override default of `final-sleep` and change default to `0` (#121) * Change final-sleep default value to 0s * Update CHANGELOG.md * Allow setting default values of final-sleep * Update CHANGELOG.md * Make lifecycler default value = 0 and configurable --- CHANGELOG.md | 2 ++ ring/lifecycler.go | 21 +++++++++++++-------- ring/lifecycler_test.go | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64bf7e49a..7701b20d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ * [CHANGE] grpcutil.Update: Remove gRPC LB related metadata. #102 * [CHANGE] concurrency.ForEach: deprecated and reimplemented by new `concurrency.ForEachJob`. #113 * [CHANGE] ring/client: It's now possible to set different value than `consul` as default KV store. #120 +* [CHANGE] Lifecycler: Default value of lifecycler's `final-sleep` is now `0s` (i.e. no sleep). #121 +* [CHANGE] Lifecycler: It's now possible to change default value of lifecycler's `final-sleep`. #121 * [ENHANCEMENT] Add middleware package. #38 * [ENHANCEMENT] Add the ring package #45 * [ENHANCEMENT] Add limiter package. #41 diff --git a/ring/lifecycler.go b/ring/lifecycler.go index e8d1655e7..1b2af3494 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -27,12 +27,15 @@ type LifecyclerConfig struct { RingConfig Config `yaml:"ring"` // Config for the ingester lifecycle control - NumTokens int `yaml:"num_tokens"` - HeartbeatPeriod time.Duration `yaml:"heartbeat_period"` - ObservePeriod time.Duration `yaml:"observe_period"` - JoinAfter time.Duration `yaml:"join_after"` - MinReadyDuration time.Duration `yaml:"min_ready_duration"` - InfNames []string `yaml:"interface_names"` + NumTokens int `yaml:"num_tokens"` + HeartbeatPeriod time.Duration `yaml:"heartbeat_period"` + ObservePeriod time.Duration `yaml:"observe_period"` + JoinAfter time.Duration `yaml:"join_after"` + MinReadyDuration time.Duration `yaml:"min_ready_duration"` + InfNames []string `yaml:"interface_names"` + + // FinalSleep's default value can be overridden by + // setting it before calling RegisterFlags or RegisterFlagsWithPrefix. FinalSleep time.Duration `yaml:"final_sleep"` TokensFilePath string `yaml:"tokens_file_path"` Zone string `yaml:"availability_zone"` @@ -48,12 +51,14 @@ type LifecyclerConfig struct { ListenPort int `yaml:"-"` } -// RegisterFlags adds the flags required to config this to the given FlagSet +// RegisterFlags adds the flags required to config this to the given FlagSet. +// The default values of some flags can be changed; see docs of LifecyclerConfig. func (cfg *LifecyclerConfig) RegisterFlags(f *flag.FlagSet) { cfg.RegisterFlagsWithPrefix("", f) } // RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet. +// The default values of some flags can be changed; see docs of LifecyclerConfig. func (cfg *LifecyclerConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { cfg.RingConfig.RegisterFlagsWithPrefix(prefix, f) @@ -68,7 +73,7 @@ func (cfg *LifecyclerConfig) RegisterFlagsWithPrefix(prefix string, f *flag.Flag f.DurationVar(&cfg.JoinAfter, prefix+"join-after", 0*time.Second, "Period to wait for a claim from another member; will join automatically after this.") f.DurationVar(&cfg.ObservePeriod, prefix+"observe-period", 0*time.Second, "Observe tokens after generating to resolve collisions. Useful when using gossiping ring.") f.DurationVar(&cfg.MinReadyDuration, prefix+"min-ready-duration", 15*time.Second, "Minimum duration to wait after the internal readiness checks have passed but before succeeding the readiness endpoint. This is used to slowdown deployment controllers (eg. Kubernetes) after an instance is ready and before they proceed with a rolling update, to give the rest of the cluster instances enough time to receive ring updates.") - f.DurationVar(&cfg.FinalSleep, prefix+"final-sleep", 30*time.Second, "Duration to sleep for before exiting, to ensure metrics are scraped.") + f.DurationVar(&cfg.FinalSleep, prefix+"final-sleep", cfg.FinalSleep, "Duration to sleep for before exiting, to ensure metrics are scraped.") f.StringVar(&cfg.TokensFilePath, prefix+"tokens-file-path", "", "File path where tokens are stored. If empty, tokens are not stored at shutdown and restored at startup.") hostname, err := os.Hostname() diff --git a/ring/lifecycler_test.go b/ring/lifecycler_test.go index a0bd9935f..8ce66131c 100644 --- a/ring/lifecycler_test.go +++ b/ring/lifecycler_test.go @@ -848,3 +848,18 @@ func waitRingInstance(t *testing.T, timeout time.Duration, l *Lifecycler, check return check(instance) }) } + +func TestDefaultFinalSleepValue(t *testing.T) { + t.Run("default value is 0", func(t *testing.T) { + cfg := &LifecyclerConfig{} + flagext.DefaultValues(cfg) + assert.Equal(t, time.Duration(0), cfg.FinalSleep) + }) + + t.Run("default value is overridable", func(t *testing.T) { + cfg := &LifecyclerConfig{} + cfg.FinalSleep = time.Minute + flagext.DefaultValues(cfg) + assert.Equal(t, time.Minute, cfg.FinalSleep) + }) +}