Improved error message when there are not enough healthy instances for the replication set

Signed-off-by: Marco Pracucci <[email protected]>

Author: pracucci

pkg/ring/replication_strategy.go

@@ -9,7 +9,7 @@ type ReplicationStrategy interface {
 	// Filter out unhealthy instances and checks if there're enough instances
 	// for an operation to succeed. Returns an error if there are not enough
 	// instances.
-	Filter(instances []IngesterDesc, op Operation, replicationFactor int, heartbeatTimeout time.Duration) (healthy []IngesterDesc, maxFailures int, err error)
+	Filter(instances []IngesterDesc, op Operation, replicationFactor int, heartbeatTimeout time.Duration, zoneAwarenessEnabled bool) (healthy []IngesterDesc, maxFailures int, err error)
 
 	// ShouldExtendReplicaSet returns true if given an instance that's going to be
 	// added to the replica set, the replica set size should be extended by 1
@@ -25,7 +25,7 @@ type DefaultReplicationStrategy struct{}
 // - Filters out dead ingesters so the one doesn't even try to write to them.
 // - Checks there is enough ingesters for an operation to succeed.
 // The ingesters argument may be overwritten.
-func (s *DefaultReplicationStrategy) Filter(ingesters []IngesterDesc, op Operation, replicationFactor int, heartbeatTimeout time.Duration) ([]IngesterDesc, int, error) {
+func (s *DefaultReplicationStrategy) Filter(ingesters []IngesterDesc, op Operation, replicationFactor int, heartbeatTimeout time.Duration, zoneAwarenessEnabled bool) ([]IngesterDesc, int, error) {
 	// We need a response from a quorum of ingesters, which is n/2 + 1.  In the
 	// case of a node joining/leaving, the actual replica set might be bigger
 	// than the replication factor, so use the bigger or the two.
@@ -49,8 +49,14 @@ func (s *DefaultReplicationStrategy) Filter(ingesters []IngesterDesc, op Operati
 	// This is just a shortcut - if there are not minSuccess available ingesters,
 	// after filtering out dead ones, don't even bother trying.
 	if len(ingesters) < minSuccess {
-		err := fmt.Errorf("at least %d live replicas required, could only find %d",
-			minSuccess, len(ingesters))
+		var err error
+
+		if zoneAwarenessEnabled {
+			err = fmt.Errorf("at least %d live replicas required across different availability zones, could only find %d", minSuccess, len(ingesters))
+		} else {
+			err = fmt.Errorf("at least %d live replicas required, could only find %d", minSuccess, len(ingesters))
+		}
+
 		return nil, 0, err
 	}
 

pkg/ring/replication_strategy_test.go

@@ -91,7 +91,7 @@ func TestRingReplicationStrategy(t *testing.T) {
 
 		t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) {
 			strategy := &DefaultReplicationStrategy{}
-			liveIngesters, maxFailure, err := strategy.Filter(ingesters, tc.op, tc.RF, 100*time.Second)
+			liveIngesters, maxFailure, err := strategy.Filter(ingesters, tc.op, tc.RF, 100*time.Second, false)
 			if tc.ExpectedError == "" {
 				assert.NoError(t, err)
 				assert.Equal(t, tc.LiveIngesters, len(liveIngesters))

pkg/ring/ring.go

@@ -263,7 +263,7 @@ func (r *Ring) Get(key uint32, op Operation, buf []IngesterDesc) (ReplicationSet
 		ingesters = append(ingesters, ingester)
 	}
 
-	liveIngesters, maxFailure, err := r.strategy.Filter(ingesters, op, r.cfg.ReplicationFactor, r.cfg.HeartbeatTimeout)
+	liveIngesters, maxFailure, err := r.strategy.Filter(ingesters, op, r.cfg.ReplicationFactor, r.cfg.HeartbeatTimeout, r.cfg.ZoneAwarenessEnabled)
 	if err != nil {
 		return ReplicationSet{}, err
 	}

pkg/ring/ring_test.go

@@ -147,7 +147,7 @@ func TestRing_Get_ZoneAwareness(t *testing.T) {
 			numZones:             1,
 			replicationFactor:    3,
 			zoneAwarenessEnabled: true,
-			expectedErr:          "at least 2 live replicas required, could only find 1",
+			expectedErr:          "at least 2 live replicas required across different availability zones, could only find 1",
 		},
 		"should succeed if there are ingesters in 2 zones on RF = 3": {
 			numIngesters:         16,

pkg/storegateway/replication_strategy.go

@@ -9,7 +9,7 @@ import (
 
 type BlocksReplicationStrategy struct{}
 
-func (s *BlocksReplicationStrategy) Filter(instances []ring.IngesterDesc, op ring.Operation, replicationFactor int, heartbeatTimeout time.Duration) ([]ring.IngesterDesc, int, error) {
+func (s *BlocksReplicationStrategy) Filter(instances []ring.IngesterDesc, op ring.Operation, _ int, heartbeatTimeout time.Duration, _ bool) ([]ring.IngesterDesc, int, error) {
 	// Filter out unhealthy instances.
 	for i := 0; i < len(instances); {
 		if instances[i].IsHealthy(op, heartbeatTimeout) {