Explicit healty states (#1890)

* Be explicit about which states are considered healthy.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* When reporting, all states are OK and we only check timestamp.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Update IsHealthy test to cover reporting.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Use switch, it's bit nicer.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Added parenthesis around state conditions.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

Author: pstibrany

pkg/ring/model.go

@@ -182,12 +182,20 @@ func (d *Desc) TokensFor(id string) (tokens, other Tokens) {
 
 // IsHealthy checks whether the ingester appears to be alive and heartbeating
 func (i *IngesterDesc) IsHealthy(op Operation, heartbeatTimeout time.Duration) bool {
-	if op == Write && i.State != ACTIVE {
-		return false
-	} else if op == Read && i.State == JOINING {
-		return false
+	healthy := false
+
+	switch op {
+	case Write:
+		healthy = (i.State == ACTIVE)
+
+	case Read:
+		healthy = (i.State == ACTIVE) || (i.State == LEAVING) || (i.State == PENDING)
+
+	case Reporting:
+		healthy = true
 	}
-	return time.Now().Sub(time.Unix(i.Timestamp, 0)) <= heartbeatTimeout
+
+	return healthy && time.Now().Sub(time.Unix(i.Timestamp, 0)) <= heartbeatTimeout
 }
 
 // Merge merges other ring into this one. Returns sub-ring that represents the change,

pkg/ring/model_test.go

@@ -11,34 +11,39 @@ func TestIngesterDesc_IsHealthy(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
-		ingester      *IngesterDesc
-		timeout       time.Duration
-		writeExpected bool
-		readExpected  bool
+		ingester       *IngesterDesc
+		timeout        time.Duration
+		writeExpected  bool
+		readExpected   bool
+		reportExpected bool
 	}{
 		"ALIVE ingester with last keepalive newer than timeout": {
-			ingester:      &IngesterDesc{State: ACTIVE, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
-			timeout:       time.Minute,
-			writeExpected: true,
-			readExpected:  true,
+			ingester:       &IngesterDesc{State: ACTIVE, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
+			timeout:        time.Minute,
+			writeExpected:  true,
+			readExpected:   true,
+			reportExpected: true,
 		},
 		"ALIVE ingester with last keepalive older than timeout": {
-			ingester:      &IngesterDesc{State: ACTIVE, Timestamp: time.Now().Add(-90 * time.Second).Unix()},
-			timeout:       time.Minute,
-			writeExpected: false,
-			readExpected:  false,
+			ingester:       &IngesterDesc{State: ACTIVE, Timestamp: time.Now().Add(-90 * time.Second).Unix()},
+			timeout:        time.Minute,
+			writeExpected:  false,
+			readExpected:   false,
+			reportExpected: false,
 		},
 		"JOINING ingester with last keepalive newer than timeout": {
-			ingester:      &IngesterDesc{State: JOINING, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
-			timeout:       time.Minute,
-			writeExpected: false,
-			readExpected:  false,
+			ingester:       &IngesterDesc{State: JOINING, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
+			timeout:        time.Minute,
+			writeExpected:  false,
+			readExpected:   false,
+			reportExpected: true,
 		},
 		"LEAVING ingester with last keepalive newer than timeout": {
-			ingester:      &IngesterDesc{State: LEAVING, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
-			timeout:       time.Minute,
-			writeExpected: false,
-			readExpected:  true,
+			ingester:       &IngesterDesc{State: LEAVING, Timestamp: time.Now().Add(-30 * time.Second).Unix()},
+			timeout:        time.Minute,
+			writeExpected:  false,
+			readExpected:   true,
+			reportExpected: true,
 		},
 	}
 
@@ -51,6 +56,9 @@ func TestIngesterDesc_IsHealthy(t *testing.T) {
 
 			actual = testData.ingester.IsHealthy(Read, testData.timeout)
 			assert.Equal(t, testData.readExpected, actual)
+
+			actual = testData.ingester.IsHealthy(Reporting, testData.timeout)
+			assert.Equal(t, testData.reportExpected, actual)
 		})
 	}
 }