From 088abb4194293bccbcf7f78f1f9f1bc968244f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Tue, 14 Dec 2021 13:00:12 +0100 Subject: [PATCH 1/2] Added ability to keep instance in the ring when lifecycler is shutting down. --- ring/basic_lifecycler.go | 14 ++++++++---- ring/basic_lifecycler_test.go | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 237bf49c6..33c4927d6 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -51,6 +51,8 @@ type BasicLifecyclerConfig struct { HeartbeatPeriod time.Duration TokensObservePeriod time.Duration NumTokens int + + KeepInTheRingOnShutdown bool } // BasicLifecycler is a basic ring lifecycler which allows to hook custom @@ -227,11 +229,15 @@ heartbeatLoop: } } - // Remove the instance from the ring. - if err := l.unregisterInstance(context.Background()); err != nil { - return errors.Wrapf(err, "failed to unregister instance from the ring (ring: %s)", l.ringName) + if l.cfg.KeepInTheRingOnShutdown { + level.Info(l.logger).Log("msg", "keeping instance the ring", "ring", l.ringName) + } else { + // Remove the instance from the ring. + if err := l.unregisterInstance(context.Background()); err != nil { + return errors.Wrapf(err, "failed to unregister instance from the ring (ring: %s)", l.ringName) + } + level.Info(l.logger).Log("msg", "instance removed from the ring", "ring", l.ringName) } - level.Info(l.logger).Log("msg", "instance removed from the ring", "ring", l.ringName) return nil } diff --git a/ring/basic_lifecycler_test.go b/ring/basic_lifecycler_test.go index c2a5f61c9..2756ebf22 100644 --- a/ring/basic_lifecycler_test.go +++ b/ring/basic_lifecycler_test.go @@ -190,6 +190,46 @@ func TestBasicLifecycler_UnregisterOnStop(t *testing.T) { assert.False(t, ok) } +func TestBasicLifecycler_KeepInTheRingOnStop(t *testing.T) { + ctx := context.Background() + cfg := prepareBasicLifecyclerConfig() + cfg.KeepInTheRingOnShutdown = true + + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) + require.NoError(t, err) + + delegate.onRegister = func(_ *BasicLifecycler, _ Desc, _ bool, _ string, _ InstanceDesc) (InstanceState, Tokens) { + return ACTIVE, Tokens{1, 2, 3, 4, 5} + } + delegate.onStopping = func(lifecycler *BasicLifecycler) { + require.NoError(t, lifecycler.changeState(context.Background(), LEAVING)) + } + + require.NoError(t, services.StartAndAwaitRunning(ctx, lifecycler)) + assert.Equal(t, ACTIVE, lifecycler.GetState()) + assert.Equal(t, Tokens{1, 2, 3, 4, 5}, lifecycler.GetTokens()) + assert.True(t, lifecycler.IsRegistered()) + assert.NotZero(t, lifecycler.GetRegisteredAt()) + assert.Equal(t, float64(cfg.NumTokens), testutil.ToFloat64(lifecycler.metrics.tokensOwned)) + assert.Equal(t, float64(cfg.NumTokens), testutil.ToFloat64(lifecycler.metrics.tokensToOwn)) + + require.NoError(t, services.StopAndAwaitTerminated(ctx, lifecycler)) + assert.Equal(t, LEAVING, lifecycler.GetState()) + assert.Equal(t, Tokens{1, 2, 3, 4, 5}, lifecycler.GetTokens()) + assert.True(t, lifecycler.IsRegistered()) + assert.NotZero(t, lifecycler.GetRegisteredAt()) + assert.Equal(t, float64(cfg.NumTokens), testutil.ToFloat64(lifecycler.metrics.tokensOwned)) + assert.Equal(t, float64(cfg.NumTokens), testutil.ToFloat64(lifecycler.metrics.tokensToOwn)) + + // Assert on the instance is in the ring. + inst, ok := getInstanceFromStore(t, store, testInstanceID) + assert.True(t, ok) + assert.Equal(t, cfg.Addr, inst.GetAddr()) + assert.Equal(t, LEAVING, inst.GetState()) + assert.Equal(t, Tokens{1, 2, 3, 4, 5}, Tokens(inst.GetTokens())) + assert.Equal(t, cfg.Zone, inst.GetZone()) +} + func TestBasicLifecycler_HeartbeatWhileRunning(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() From c64c65729ef2981e0236cfbce35769be8da854fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Tue, 14 Dec 2021 13:51:49 +0100 Subject: [PATCH 2/2] Changelog and doc. --- CHANGELOG.md | 1 + ring/basic_lifecycler.go | 6 ++++-- ring/basic_lifecycler_test.go | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f50d722e..fd9ad944c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,5 +22,6 @@ * [ENHANCEMENT] Optimise memberlist receive path when used as a backing store for rings with a large number of members. #76 #77 #84 #91 * [ENHANCEMENT] Memberlist: prepare the data to send on the write before starting counting the elapsed time for `-memberlist.packet-write-timeout`, in order to reduce chances we hit the timeout when sending a packet to other node. #89 * [ENHANCEMENT] flagext: for cases such as `DeprecatedFlag()` that need a logger, add RegisterFlagsWithLogger. #80 +* [ENHANCEMENT] Added option to BasicLifecycler to keep instance in the ring when stopping. #97 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85 diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 33c4927d6..96186acd0 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -52,7 +52,9 @@ type BasicLifecyclerConfig struct { TokensObservePeriod time.Duration NumTokens int - KeepInTheRingOnShutdown bool + // If true lifecycler doesn't unregister instance from the ring when it's stopping. Default value is false, + // which means unregistering. + KeepInstanceInTheRingOnShutdown bool } // BasicLifecycler is a basic ring lifecycler which allows to hook custom @@ -229,7 +231,7 @@ heartbeatLoop: } } - if l.cfg.KeepInTheRingOnShutdown { + if l.cfg.KeepInstanceInTheRingOnShutdown { level.Info(l.logger).Log("msg", "keeping instance the ring", "ring", l.ringName) } else { // Remove the instance from the ring. diff --git a/ring/basic_lifecycler_test.go b/ring/basic_lifecycler_test.go index 2756ebf22..eb9140e32 100644 --- a/ring/basic_lifecycler_test.go +++ b/ring/basic_lifecycler_test.go @@ -193,7 +193,7 @@ func TestBasicLifecycler_UnregisterOnStop(t *testing.T) { func TestBasicLifecycler_KeepInTheRingOnStop(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - cfg.KeepInTheRingOnShutdown = true + cfg.KeepInstanceInTheRingOnShutdown = true lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err)