Only count ingesters with tokens when calculating local limits

CHANGELOG.md

@@ -33,6 +33,7 @@
 * [BUGFIX] Ingester: turn native histogram validation errors in TSDB into soft ingester errors that result in returning 4xx to the end-user instead of 5xx. In the case of TSDB validation errors, the counter `cortex_discarded_samples_total` will be increased with the `reason` label set to `"invalid-native-histogram"`. #7736 #7773
 * [BUGFIX] Do not wrap error message with `sampled 1/<frequency>` if it's not actually sampled. #7784
 * [BUGFIX] Store-gateway: do not track cortex_querier_blocks_consistency_checks_failed_total metric if query has been canceled or interrued due to any error not related to blocks consistency check failed. #7752
+* [BUGFIX] Ingester: ignore instances with no tokens when calculating local limits to prevent discards during ingester scale-up #7881
 
 ### Mixin
 

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/golang/snappy v0.0.4
 	github.com/google/gopacket v1.1.19
 	github.com/gorilla/mux v1.8.1
-	github.com/grafana/dskit v0.0.0-20240403100540-1435abf0da58
+	github.com/grafana/dskit v0.0.0-20240411153331-40cf812a9efe
 	github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
 	github.com/json-iterator/go v1.1.12

go.sum

@@ -496,8 +496,8 @@ github.com/gosimple/slug v1.1.1 h1:fRu/digW+NMwBIP+RmviTK97Ho/bEj/C9swrCspN3D4=
 github.com/gosimple/slug v1.1.1/go.mod h1:ER78kgg1Mv0NQGlXiDe57DpCyfbNywXXZ9mIorhxAf0=
 github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc h1:PXZQA2WCxe85Tnn+WEvr8fDpfwibmEPgfgFEaC87G24=
 github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc/go.mod h1:AHHlOEv1+GGQ3ktHMlhuTUwo3zljV3QJbC0+8o2kn+4=
-github.com/grafana/dskit v0.0.0-20240403100540-1435abf0da58 h1:ph674hL86kFIWcrqUCXW/D0RdSFu2ToIjqvzRnPAzPg=
-github.com/grafana/dskit v0.0.0-20240403100540-1435abf0da58/go.mod h1:HvSf3uf8Ps2vPpzHeAFyZTdUcbVr+Rxpq1xcx7J/muc=
+github.com/grafana/dskit v0.0.0-20240411153331-40cf812a9efe h1:DSrTlgHuavOulNS3K6tkYcj94RBj+2/kXDDHiw8c7gE=
+github.com/grafana/dskit v0.0.0-20240411153331-40cf812a9efe/go.mod h1:HvSf3uf8Ps2vPpzHeAFyZTdUcbVr+Rxpq1xcx7J/muc=
 github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc h1:BW+LjKJDz0So5LI8UZfW5neWeKpSkWqhmGjQFzcFfLM=
 github.com/grafana/e2e v0.1.2-0.20240118170847-db90b84177fc/go.mod h1:JVmqPBe8A/pZWwRoJW5ZjyALeY5OXMzPl7LrVXOdZAI=
 github.com/grafana/goautoneg v0.0.0-20231010094147-47ce5e72a9ae h1:Yxbw9jKGJVC6qAK5Ubzzb/qZwM6rRMMqaDc/d4Vp3pM=

pkg/ingester/limiter.go

@@ -122,8 +122,8 @@ type limiterRingStrategy interface {
 // ingesterRingLimiterRingCount is the interface exposed by a ring implementation which allows
 // to count members
 type ingesterRingLimiterRingCount interface {
-	InstancesCount() int
-	InstancesInZoneCount(zone string) int
+	InstancesWithTokensCount() int
+	InstancesWithTokensInZoneCount(zone string) int
 	ZonesCount() int
 }
 
@@ -157,12 +157,12 @@ func (is *ingesterRingLimiterStrategy) convertGlobalToLocalLimit(userID string,
 	var ingestersInZoneCount int
 	if zonesCount > 1 {
 		// In this case zone-aware replication is enabled, and ingestersInZoneCount is initially set to
-		// the total number of ingesters in the corresponding zone
-		ingestersInZoneCount = is.ring.InstancesInZoneCount(is.ingesterZone)
+		// the total number of ingesters with tokens in the corresponding zone
+		ingestersInZoneCount = is.ring.InstancesWithTokensInZoneCount(is.ingesterZone)
 	} else {
 		// In this case zone-aware replication is disabled, and ingestersInZoneCount is initially set to
-		// the total number of ingesters
-		ingestersInZoneCount = is.ring.InstancesCount()
+		// the total number of ingesters with tokens
+		ingestersInZoneCount = is.ring.InstancesWithTokensCount()
 	}
 	// If shuffle sharding is enabled and the total number of ingesters in the zone is greater than the
 	// expected number of ingesters per sharded zone, then we should honor the latter because series/metadata

pkg/ingester/limiter_test.go

@@ -922,10 +922,18 @@ func (m *ringCountMock) InstancesCount() int {
 	return m.instancesCount
 }
 
+func (m *ringCountMock) InstancesWithTokensCount() int {
+	return m.instancesCount
+}
+
 func (m *ringCountMock) InstancesInZoneCount(_ string) int {
 	return m.instancesInZoneCount
 }
 
+func (m *ringCountMock) InstancesWithTokensInZoneCount(_ string) int {
+	return m.instancesInZoneCount
+}
+
 func (m *ringCountMock) ZonesCount() int {
 	return m.zonesCount
 }

pkg/ingester/owned_series_test.go

@@ -351,6 +351,35 @@ func TestOwnedSeriesServiceWithIngesterRing(t *testing.T) {
 				c.checkUpdateReasonForUser(t, "")
 			},
 		},
+		"shard size = 0, add PENDING ingester with no tokens": {
+			limits: map[string]*validation.Limits{
+				ownedServiceTestUser: {
+					MaxGlobalSeriesPerUser:   ownedServiceTestUserSeriesLimit,
+					IngestionTenantShardSize: 0,
+				},
+			},
+			testFunc: func(t *testing.T, c *ownedSeriesWithIngesterRingTestContext, _ map[string]*validation.Limits) {
+				c.pushUserSeries(t)
+				c.updateOwnedSeriesAndCheckResult(t, false, 1, recomputeOwnedSeriesReasonNewUser)
+				c.checkUpdateReasonForUser(t, "")
+
+				// initial state: all series are owned by the first ingester.
+				c.checkTestedIngesterOwnedSeriesState(t, ownedServiceSeriesCount, 0, ownedServiceTestUserSeriesLimit)
+
+				// add a PENDING ingester with no tokens
+				updateRingAndWaitForWatcherToReadUpdate(t, c.kvStore, func(desc *ring.Desc) {
+					desc.AddIngester("second-ingester", "localhost", c.ingesterZone, []uint32{}, ring.PENDING, time.Now())
+				})
+
+				// verify no change in state before owned series run
+				c.checkTestedIngesterOwnedSeriesState(t, ownedServiceSeriesCount, 0, ownedServiceTestUserSeriesLimit)
+
+				// the ring has changed but the token ranges have not, so no recompute should happen
+				c.updateOwnedSeriesAndCheckResult(t, true, 0, "")
+				c.checkTestedIngesterOwnedSeriesState(t, ownedServiceSeriesCount, 0, ownedServiceTestUserSeriesLimit)
+				c.checkUpdateReasonForUser(t, "")
+			},
+		},
 		"unchanged ring, shard size from 0 to ingester count": {
 			limits: map[string]*validation.Limits{
 				ownedServiceTestUser: {