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

@@ -38,6 +38,7 @@
   - `-grpc-store.*`
   - `-flusher.wal-dir`, `-flusher.concurrent-flushes`, `-flusher.flush-op-timeout`
 * [CHANGE] Remove support for alertmanager and ruler legacy store configuration. Before upgrading, you need to convert your configuration to use the `alertmanager-storage` and `ruler-storage` configuration on the version that you're already running, then upgrade.
+* [ENHANCEMENT] AlertManager: Retrying AlertManager Get Requests (Get Alertmanager status, Get Alertmanager Receivers) on next replica on error #4840
 * [CHANGE] Disables TSDB isolation. #4825
 * [CHANGE] Drops support Prometheus 1.x rule format on configdb. #4826
 * [ENHANCEMENT] Querier/Ruler: Retry store-gateway in case of unexpected failure, instead of failing the query. #4532

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,