Avoid sort tokens on autoJoin

CHANGELOG.md

@@ -24,6 +24,7 @@
 * [ENHANCEMENT] Improving Performance on the API Gzip Handler. #5347
 * [ENHANCEMENT] Dynamodb: Add `puller-sync-time` to allow different pull time for ring. #5357
 * [ENHANCEMENT] Emit querier `max_concurrent` as a metric. #5362
+* [ENHANCEMENT] Avoid sort tokens on lifecycler autoJoin. #5394
 * [ENHANCEMENT] Do not resync blocks in running store gateways during rollout deployment and container restart. #5363
 * [ENHANCEMENT] Store Gateway: Add new metrics `cortex_bucket_store_sent_chunk_size_bytes`, `cortex_bucket_store_postings_size_bytes` and `cortex_bucket_store_empty_postings_total`. #5397
 * [BUGFIX] Ruler: Validate if rule group can be safely converted back to rule group yaml from protobuf message #5265

pkg/ring/lifecycler.go

@@ -735,11 +735,20 @@ func (i *Lifecycler) autoJoin(ctx context.Context, targetState InstanceState) er
 			ringDesc = in.(*Desc)
 		}
 
+		i.setState(targetState)
+
 		// At this point, we should not have any tokens, and we should be in PENDING state.
+		// Need to make sure we didnt change the num of tokens configured
 		myTokens, takenTokens := ringDesc.TokensFor(i.ID)
+		needTokens := i.cfg.NumTokens - len(myTokens)
 
-		newTokens := GenerateTokens(i.cfg.NumTokens-len(myTokens), takenTokens)
-		i.setState(targetState)
+		if needTokens == 0 && myTokens.Equals(i.getTokens()) {
+			// Tokens have been verified. No need to change them.
+			ringDesc.AddIngester(i.ID, i.Addr, i.Zone, i.getTokens(), i.GetState(), i.getRegisteredAt())
+			return ringDesc, true, nil
+		}
+
+		newTokens := GenerateTokens(needTokens, takenTokens)
 
 		myTokens = append(myTokens, newTokens...)
 		sort.Sort(myTokens)

pkg/ruler/lifecycle_test.go

@@ -2,7 +2,6 @@ package ruler
 
 import (
 	"context"
-	"sort"
 	"testing"
 	"time"
 
@@ -76,7 +75,7 @@ func TestRuler_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testing.T) {
 	require.NoError(t, ringStore.CAS(ctx, ringKey, func(in interface{}) (interface{}, bool, error) {
 		ringDesc := ring.GetOrCreateRingDesc(in)
 
-		instance := ringDesc.AddIngester(unhealthyInstanceID, "1.1.1.1", "", generateSortedTokens(config.Ring.NumTokens), ring.ACTIVE, time.Now())
+		instance := ringDesc.AddIngester(unhealthyInstanceID, "1.1.1.1", "", ring.GenerateTokens(config.Ring.NumTokens, nil), ring.ACTIVE, time.Now())
 		instance.Timestamp = time.Now().Add(-(ringAutoForgetUnhealthyPeriods + 1) * heartbeatTimeout).Unix()
 		ringDesc.Ingesters[unhealthyInstanceID] = instance
 
@@ -95,17 +94,6 @@ func TestRuler_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testing.T) {
 	})
 }
 
-func generateSortedTokens(numTokens int) ring.Tokens {
-	tokens := ring.GenerateTokens(numTokens, nil)
-
-	// Ensure generated tokens are sorted.
-	sort.Slice(tokens, func(i, j int) bool {
-		return tokens[i] < tokens[j]
-	})
-
-	return ring.Tokens(tokens)
-}
-
 // numTokens determines the number of tokens owned by the specified
 // address
 func numTokens(c kv.Client, name, ringKey string) int {

pkg/storegateway/gateway_test.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -114,12 +113,12 @@ func TestStoreGateway_InitialSyncWithDefaultShardingEnabled(t *testing.T) {
 		"instance already in the ring with ACTIVE state and has all tokens": {
 			initialExists: true,
 			initialState:  ring.ACTIVE,
-			initialTokens: generateSortedTokens(RingNumTokens),
+			initialTokens: ring.GenerateTokens(RingNumTokens, nil),
 		},
 		"instance already in the ring with LEAVING state and has all tokens": {
 			initialExists: true,
 			initialState:  ring.LEAVING,
-			initialTokens: generateSortedTokens(RingNumTokens),
+			initialTokens: ring.GenerateTokens(RingNumTokens, nil),
 		},
 	}
 
@@ -548,15 +547,15 @@ func TestStoreGateway_ShouldSupportLoadRingTokensFromFile(t *testing.T) {
 		expectedNumTokens int
 	}{
 		"stored tokens are less than the configured ones": {
-			storedTokens:      generateSortedTokens(RingNumTokens - 10),
+			storedTokens:      ring.GenerateTokens(RingNumTokens-10, nil),
 			expectedNumTokens: RingNumTokens,
 		},
 		"stored tokens are equal to the configured ones": {
-			storedTokens:      generateSortedTokens(RingNumTokens),
+			storedTokens:      ring.GenerateTokens(RingNumTokens, nil),
 			expectedNumTokens: RingNumTokens,
 		},
 		"stored tokens are more then the configured ones": {
-			storedTokens:      generateSortedTokens(RingNumTokens + 10),
+			storedTokens:      ring.GenerateTokens(RingNumTokens+10, nil),
 			expectedNumTokens: RingNumTokens + 10,
 		},
 	}
@@ -791,7 +790,7 @@ func TestStoreGateway_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testin
 	require.NoError(t, ringStore.CAS(ctx, RingKey, func(in interface{}) (interface{}, bool, error) {
 		ringDesc := ring.GetOrCreateRingDesc(in)
 
-		instance := ringDesc.AddIngester(unhealthyInstanceID, "1.1.1.1", "", generateSortedTokens(RingNumTokens), ring.ACTIVE, time.Now())
+		instance := ringDesc.AddIngester(unhealthyInstanceID, "1.1.1.1", "", ring.GenerateTokens(RingNumTokens, nil), ring.ACTIVE, time.Now())
 		instance.Timestamp = time.Now().Add(-(ringAutoForgetUnhealthyPeriods + 1) * heartbeatTimeout).Unix()
 		ringDesc.Ingesters[unhealthyInstanceID] = instance
 
@@ -1144,17 +1143,6 @@ func mockTSDB(t *testing.T, dir string, numSeries, numBlocks int, minT, maxT int
 	require.NoError(t, db.Close())
 }
 
-func generateSortedTokens(numTokens int) ring.Tokens {
-	tokens := ring.GenerateTokens(numTokens, nil)
-
-	// Ensure generated tokens are sorted.
-	sort.Slice(tokens, func(i, j int) bool {
-		return tokens[i] < tokens[j]
-	})
-
-	return ring.Tokens(tokens)
-}
-
 func readSamplesFromChunks(rawChunks []storepb.AggrChunk) ([]sample, error) {
 	var samples []sample