Modifying cortexproject#4422 to retry only for GET requests. Retrying AlertManager UnaryPath GET Requests on next replica if one fail.

Signed-off-by: Krishna Teja Puttagunta <[email protected]>

Author: krishnateja325

CHANGELOG.md

@@ -2088,3 +2088,4 @@ This release has several exciting features, the most notable of them being setti
 * [FEATURE] You can specify "heap ballast" to reduce Go GC Churn #1489
 * [BUGFIX] HA Tracker no longer always makes a request to Consul/Etcd when a request is not from the active replica #1516
 * [BUGFIX] Queries are now correctly cancelled by the query-frontend #1508
+* [ENHANCEMENT] AlertManager: Retrying AlertManager Get Requests (Get Alertmanager status, Get Alertmanager Receivers) on next replica on error #4840

pkg/alertmanager/distributor.go

@@ -254,14 +254,41 @@ func (d *Distributor) doUnary(userID string, w http.ResponseWriter, r *http.Requ
 	defer sp.Finish()
 	// Until we have a mechanism to combine the results from multiple alertmanagers,
 	// we forward the request to only only of the alertmanagers.
-	amDesc := replicationSet.Instances[rand.Intn(len(replicationSet.Instances))]
-	resp, err := d.doRequest(ctx, amDesc, req)
-	if err != nil {
-		respondFromError(err, w, logger)
-		return
+
+	instances := replicationSet.Instances
+	// Randomize the list of instances to not always query the same one.
+	rand.Shuffle(len(instances), func(i, j int) {
+		instances[i], instances[j] = instances[j], instances[i]
+
+	})
+	finalInstances := make([]ring.InstanceDesc, 0)
+	if req.GetMethod() == "GET" && d.isUnaryReadPath(r.URL.Path) {
+		finalInstances = instances
+
+	} else {
+		//using only 1 random instance if the request is not a GET request
+		finalInstances = append(finalInstances, instances[0])
+	}
+
+	var lastErr error
+	for _, instance := range finalInstances {
+		resp, err := d.doRequest(ctx, instance, req)
+		// storing the last error message
+		if err != nil {
+			lastErr = err
+		}
+
+		// Return on the first succeeded request
+		if err == nil {
+			respondFromHTTPGRPCResponse(w, resp)
+			return
+		}
+	}
+	// throwing the last error if the for loop finish without succeeding
+	if lastErr != nil {
+		respondFromError(lastErr, w, logger)
 	}
 
-	respondFromHTTPGRPCResponse(w, resp)
 }
 
 func respondFromError(err error, w http.ResponseWriter, logger log.Logger) {

pkg/alertmanager/distributor_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/weaveworks/common/httpgrpc"
 	"github.com/weaveworks/common/user"
+	"go.uber.org/atomic"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/health/grpc_health_v1"
 
@@ -37,6 +38,7 @@ func TestDistributor_DistributeRequest(t *testing.T) {
 		name                string
 		numAM, numHappyAM   int
 		replicationFactor   int
+		failedReqCount      int
 		isRead              bool
 		isDelete            bool
 		expStatusCode       int
@@ -196,6 +198,25 @@ func TestDistributor_DistributeRequest(t *testing.T) {
 			expStatusCode:      http.StatusOK,
 			expectedTotalCalls: 1,
 			route:              "/status",
+		}, {
+			name:               "Read /status should try all alert managers on error",
+			numAM:              3,
+			numHappyAM:         0,
+			replicationFactor:  3,
+			isRead:             true,
+			expStatusCode:      http.StatusInternalServerError,
+			expectedTotalCalls: 3,
+			route:              "/status",
+		}, {
+			name:               "Read /status is sent to 3 AM when 2 are not happy",
+			numAM:              3,
+			numHappyAM:         3,
+			failedReqCount:     2,
+			replicationFactor:  3,
+			isRead:             true,
+			expStatusCode:      http.StatusOK,
+			expectedTotalCalls: 3,
+			route:              "/status",
 		}, {
 			name:                "Write /status not supported",
 			numAM:               5,
@@ -229,7 +250,7 @@ func TestDistributor_DistributeRequest(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			route := "/alertmanager/api/v1" + c.route
-			d, ams, cleanup := prepare(t, c.numAM, c.numHappyAM, c.replicationFactor, c.responseBody)
+			d, ams, cleanup := prepare(t, c.numAM, c.numHappyAM, c.failedReqCount, c.replicationFactor, c.responseBody)
 			t.Cleanup(cleanup)
 
 			ctx := user.InjectOrgID(context.Background(), "1")
@@ -306,20 +327,21 @@ func TestDistributor_IsPathSupported(t *testing.T) {
 
 	for path, isSupported := range supported {
 		t.Run(path, func(t *testing.T) {
-			d, _, cleanup := prepare(t, 1, 1, 1, []byte{})
+			d, _, cleanup := prepare(t, 1, 1, 0, 1, []byte{})
 			t.Cleanup(cleanup)
 			require.Equal(t, isSupported, d.IsPathSupported(path))
 		})
 	}
 }
 
-func prepare(t *testing.T, numAM, numHappyAM, replicationFactor int, responseBody []byte) (*Distributor, []*mockAlertmanager, func()) {
+func prepare(t *testing.T, numAM, numHappyAM, totalFailedReq, replicationFactor int, responseBody []byte) (*Distributor, []*mockAlertmanager, func()) {
 	ams := []*mockAlertmanager{}
+	failedReqCount := atomic.NewInt32(int32(totalFailedReq))
 	for i := 0; i < numHappyAM; i++ {
-		ams = append(ams, newMockAlertmanager(i, true, responseBody))
+		ams = append(ams, newMockAlertmanager(i, true, failedReqCount, responseBody))
 	}
 	for i := numHappyAM; i < numAM; i++ {
-		ams = append(ams, newMockAlertmanager(i, false, responseBody))
+		ams = append(ams, newMockAlertmanager(i, false, failedReqCount, responseBody))
 	}
 
 	// Use a real ring with a mock KV store to test ring RF logic.
@@ -383,14 +405,16 @@ type mockAlertmanager struct {
 	myAddr           string
 	happy            bool
 	responseBody     []byte
+	failedReqCount   *atomic.Int32
 }
 
-func newMockAlertmanager(idx int, happy bool, responseBody []byte) *mockAlertmanager {
+func newMockAlertmanager(idx int, happy bool, failedReqCount *atomic.Int32, responseBody []byte) *mockAlertmanager {
 	return &mockAlertmanager{
 		receivedRequests: make(map[string]map[int]int),
 		myAddr:           fmt.Sprintf("127.0.0.1:%05d", 10000+idx),
 		happy:            happy,
 		responseBody:     responseBody,
+		failedReqCount:   failedReqCount,
 	}
 }
 
@@ -409,7 +433,8 @@ func (am *mockAlertmanager) HandleRequest(_ context.Context, in *httpgrpc.HTTPRe
 		am.receivedRequests[path] = m
 	}
 
-	if am.happy {
+	failedCountLocal := am.failedReqCount.Dec()
+	if am.happy && failedCountLocal < 0 {
 		m[http.StatusOK]++
 		return &httpgrpc.HTTPResponse{
 			Code: http.StatusOK,