Modifying #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,36 @@ 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
+
+	var instances []ring.InstanceDesc
+	if req.GetMethod() == "GET" && d.isUnaryReadPath(r.URL.Path) {
+		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]
+		})
+	} else {
+		//Picking 1 instance at Random for Non-Get Unary Read requests, as shuffling through large number of instances might increase complexity
+		randN := rand.Intn(len(replicationSet.Instances))
+		instances = replicationSet.Instances[randN : randN+1]
 	}
 
-	respondFromHTTPGRPCResponse(w, resp)
+	var lastErr error
+	for _, instance := range instances {
+		resp, err := d.doRequest(ctx, instance, req)
+		// storing the last error message
+		if err != nil {
+			lastErr = err
+			continue
+		}
+		// Return on the first succeeded request
+		respondFromHTTPGRPCResponse(w, resp)
+		return
+	}
+	// throwing the last error if the for loop finish without succeeding
+	if lastErr != nil {
+		respondFromError(lastErr, w, logger)
+	}
 }
 
 func respondFromError(err error, w http.ResponseWriter, logger log.Logger) {

pkg/alertmanager/distributor_test.go

@@ -196,6 +196,24 @@ 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:         1,
+			replicationFactor:  3,
+			isRead:             true,
+			expStatusCode:      http.StatusOK,
+			expectedTotalCalls: 3,
+			route:              "/status",
 		}, {
 			name:                "Write /status not supported",
 			numAM:               5,