Modify WritableInstancesInZoneCount to WritableInstancesWithTokensCou…

…nt. (#562)

* Modify WritableInstancesInZoneCount to WritableInstancesWithTokensCount.

* Update CHANGELOG.md

CHANGELOG.md

@@ -220,7 +220,7 @@
 * [ENHANCEMENT] tracing: add ExtractTraceSpanID function.
 * [EHNANCEMENT] crypto/tls: Support reloading client certificates #537 #552
 * [ENHANCEMENT] Add read only support for ingesters in the ring and lifecycler. #553
-* [ENHANCEMENT] Added new ring methods to expose writable instances per zone counts. #560
+* [ENHANCEMENT] Added new ring methods to expose number of writable instances with tokens per zone, and overall. #560 #562
 * [CHANGE] Backoff: added `Backoff.ErrCause()` which is like `Backoff.Err()` but returns the context cause if backoff is terminated because the context has been canceled. #538
 * [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

ring/model.go

@@ -570,16 +570,16 @@ func (d *Desc) instancesWithTokensCountPerZone() map[string]int {
 	return instancesCountPerZone
 }
 
-func (d *Desc) writableInstancesCountPerZone() map[string]int {
-	instancesCountPerZone := map[string]int{}
+func (d *Desc) writableInstancesWithTokensCount() int {
+	writableInstancesWithTokensCount := 0
 	if d != nil {
 		for _, ingester := range d.Ingesters {
-			if !ingester.ReadOnly {
-				instancesCountPerZone[ingester.Zone]++
+			if len(ingester.Tokens) > 0 && !ingester.ReadOnly {
+				writableInstancesWithTokensCount++
 			}
 		}
 	}
-	return instancesCountPerZone
+	return writableInstancesWithTokensCount
 }
 
 func (d *Desc) writableInstancesWithTokensCountPerZone() map[string]int {

ring/ring.go

@@ -87,8 +87,8 @@ type ReadRing interface {
 	// InstancesWithTokensInZoneCount returns the number of instances in the ring that are registered in given zone and have tokens.
 	InstancesWithTokensInZoneCount(zone string) int
 
-	// WritableInstancesInZoneCount returns the number of writable instances in the ring that are registered in given zone.
-	WritableInstancesInZoneCount(zone string) int
+	// WritableInstancesWithTokensCount returns the number of writable instances in the ring that have tokens.
+	WritableInstancesWithTokensCount() int
 
 	// WritableInstancesWithTokensInZoneCount returns the number of writable instances in the ring that are registered in given zone and have tokens.
 	WritableInstancesWithTokensInZoneCount(zone string) int
@@ -210,8 +210,8 @@ type Ring struct {
 	// Nubmber of registered instances with tokens per zone.
 	instancesWithTokensCountPerZone map[string]int
 
-	// Number of registered instances per zone that are writable.
-	writableInstancesCountPerZone map[string]int
+	// Number of registered instances are writable and have tokens.
+	writableInstancesWithTokensCount int
 
 	// Nubmber of registered instances with tokens per zone that are writable.
 	writableInstancesWithTokensCountPerZone map[string]int
@@ -368,7 +368,7 @@ func (r *Ring) updateRingState(ringDesc *Desc) {
 	instancesWithTokensCount := ringDesc.instancesWithTokensCount()
 	instancesCountPerZone := ringDesc.instancesCountPerZone()
 	instancesWithTokensCountPerZone := ringDesc.instancesWithTokensCountPerZone()
-	writableInstancesCountPerZone := ringDesc.writableInstancesCountPerZone()
+	writableInstancesWithTokensCount := ringDesc.writableInstancesWithTokensCount()
 	writableInstancesWithTokensCountPerZone := ringDesc.writableInstancesWithTokensCountPerZone()
 
 	r.mtx.Lock()
@@ -381,7 +381,7 @@ func (r *Ring) updateRingState(ringDesc *Desc) {
 	r.instancesWithTokensCount = instancesWithTokensCount
 	r.instancesCountPerZone = instancesCountPerZone
 	r.instancesWithTokensCountPerZone = instancesWithTokensCountPerZone
-	r.writableInstancesCountPerZone = writableInstancesCountPerZone
+	r.writableInstancesWithTokensCount = writableInstancesWithTokensCount
 	r.writableInstancesWithTokensCountPerZone = writableInstancesWithTokensCountPerZone
 	r.oldestRegisteredTimestamp = oldestRegisteredTimestamp
 	r.lastTopologyChange = now
@@ -848,7 +848,7 @@ func (r *Ring) shuffleShard(identifier string, size int, lookbackPeriod time.Dur
 		instancesWithTokensCount:                shardDesc.instancesWithTokensCount(),
 		instancesCountPerZone:                   shardDesc.instancesCountPerZone(),
 		instancesWithTokensCountPerZone:         shardDesc.instancesWithTokensCountPerZone(),
-		writableInstancesCountPerZone:           shardDesc.writableInstancesCountPerZone(),
+		writableInstancesWithTokensCount:        shardDesc.writableInstancesWithTokensCount(),
 		writableInstancesWithTokensCountPerZone: shardDesc.writableInstancesWithTokensCountPerZone(),
 
 		oldestRegisteredTimestamp: shardDesc.getOldestRegisteredTimestamp(),
@@ -1158,12 +1158,12 @@ func (r *Ring) InstancesWithTokensInZoneCount(zone string) int {
 	return r.instancesWithTokensCountPerZone[zone]
 }
 
-// WritableInstancesInZoneCount returns the number of writable instances in the ring that are registered in given zone.
-func (r *Ring) WritableInstancesInZoneCount(zone string) int {
+// WritableInstancesWithTokensCount returns the number of writable instances in the ring that have tokens.
+func (r *Ring) WritableInstancesWithTokensCount() int {
 	r.mtx.RLock()
 	defer r.mtx.RUnlock()
 
-	return r.writableInstancesCountPerZone[zone]
+	return r.writableInstancesWithTokensCount
 }
 
 // WritableInstancesWithTokensInZoneCount returns the number of writable instances in the ring that are registered in given zone and have tokens.

ring/ring_test.go

@@ -1259,12 +1259,12 @@ func TestRing_GetWritableInstancesWithTokensCounts(t *testing.T) {
 
 	tests := map[string]struct {
 		ringInstances                                   map[string]InstanceDesc
-		expectedWritableInstancesCountPerZone           map[string]int
+		expectedWritableInstancesWithTokensCount        int
 		expectedWritableInstancesWithTokensCountPerZone map[string]int
 	}{
 		"empty ring": {
 			ringInstances:                                   nil,
-			expectedWritableInstancesCountPerZone:           map[string]int{},
+			expectedWritableInstancesWithTokensCount:        0,
 			expectedWritableInstancesWithTokensCountPerZone: map[string]int{},
 		},
 		"single zone, no tokens": {
@@ -1274,7 +1274,7 @@ func TestRing_GetWritableInstancesWithTokensCounts(t *testing.T) {
 				"instance-3": {Addr: "127.0.0.3", Zone: "zone-a", State: PENDING, Tokens: []uint32{}},
 				"instance-4": {Addr: "127.0.0.4", Zone: "zone-a", State: JOINING, Tokens: []uint32{}},
 			},
-			expectedWritableInstancesCountPerZone:           map[string]int{"zone-a": 3},
+			expectedWritableInstancesWithTokensCount:        0,
 			expectedWritableInstancesWithTokensCountPerZone: map[string]int{"zone-a": 0},
 		},
 		"single zone, some tokens": {
@@ -1288,7 +1288,7 @@ func TestRing_GetWritableInstancesWithTokensCounts(t *testing.T) {
 				"instance-7": {Addr: "127.0.0.7", Zone: "zone-a", State: JOINING, Tokens: gen.GenerateTokens(128, nil), ReadOnly: true},
 				"instance-8": {Addr: "127.0.0.8", Zone: "zone-a", State: JOINING, Tokens: []uint32{}, ReadOnly: true},
 			},
-			expectedWritableInstancesCountPerZone:           map[string]int{"zone-a": 5},
+			expectedWritableInstancesWithTokensCount:        3,
 			expectedWritableInstancesWithTokensCountPerZone: map[string]int{"zone-a": 3},
 		},
 		"multiple zones": {
@@ -1302,7 +1302,7 @@ func TestRing_GetWritableInstancesWithTokensCounts(t *testing.T) {
 				"instance-7": {Addr: "127.0.0.7", Zone: "zone-c", State: JOINING, Tokens: gen.GenerateTokens(128, nil)},
 				"instance-8": {Addr: "127.0.0.8", Zone: "zone-d", State: JOINING, Tokens: []uint32{}, ReadOnly: true},
 			},
-			expectedWritableInstancesCountPerZone:           map[string]int{"zone-a": 1, "zone-b": 1, "zone-c": 2, "zone-d": 1},
+			expectedWritableInstancesWithTokensCount:        3,
 			expectedWritableInstancesWithTokensCountPerZone: map[string]int{"zone-a": 1, "zone-b": 0, "zone-c": 2, "zone-d": 0},
 		},
 	}
@@ -1322,13 +1322,11 @@ func TestRing_GetWritableInstancesWithTokensCounts(t *testing.T) {
 					ZoneAwarenessEnabled: true,
 				},
 				ringDesc:                                ringDesc,
-				writableInstancesCountPerZone:           ringDesc.writableInstancesCountPerZone(),
+				writableInstancesWithTokensCount:        ringDesc.writableInstancesWithTokensCount(),
 				writableInstancesWithTokensCountPerZone: ringDesc.writableInstancesWithTokensCountPerZone(),
 			}
 
-			for z, instances := range testData.expectedWritableInstancesCountPerZone {
-				assert.Equal(t, instances, ring.WritableInstancesInZoneCount(z))
-			}
+			assert.Equal(t, testData.expectedWritableInstancesWithTokensCount, ring.WritableInstancesWithTokensCount())
 			for z, instances := range testData.expectedWritableInstancesWithTokensCountPerZone {
 				assert.Equal(t, instances, ring.WritableInstancesWithTokensInZoneCount(z))
 			}

ring/util_test.go

@@ -83,7 +83,7 @@ func (r *RingMock) InstancesWithTokensInZoneCount(_ string) int {
 	return 0
 }
 
-func (r *RingMock) WritableInstancesInZoneCount(_ string) int {
+func (r *RingMock) WritableInstancesWithTokensCount() int {
 	return 0
 }