fix(kayenta): Retry in case of network errors (#615)

spinnaker · Sep 23, 2019 · c056453 · c056453
1 parent 08ee564
commit c056453
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 3 deletions.
diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java
@@ -108,9 +108,11 @@ public String executeQuery(
           if (retries >= retryConfiguration.getAttempts()) {
             throw e;
           }
+          Object error = e.getResponse() != null ? e.getResponse().getStatus() : e.getCause();
           log.warn(
-              "Got {} http status when querying for metrics. Retrying request (current attempt: {}, max attempts: {})",
-              e.getResponse().getStatus(),
+              "Got {} result when querying for metrics. Retrying request (current attempt: "
+                  + "{}, max attempts: {})",
+              error,
               retries,
               retryConfiguration.getAttempts());
         } else {
@@ -122,7 +124,8 @@ public String executeQuery(
           throw e;
         }
         log.warn(
-            "Got error when querying for metrics. Retrying request (current attempt: {}, max attempts: {})",
+            "Got error when querying for metrics. Retrying request (current attempt: {}, max "
+                + "attempts: {})",
             retries,
             retryConfiguration.getAttempts(),
             e);
@@ -137,6 +140,10 @@ public String executeQuery(
   }
 
   private boolean isRetryable(RetrofitError e) {
+    if (isNetworkError(e)) {
+      // retry in case of network errors
+      return true;
+    }
     HttpStatus responseStatus = HttpStatus.resolve(e.getResponse().getStatus());
     if (responseStatus == null) {
       return false;
@@ -145,6 +152,10 @@ private boolean isRetryable(RetrofitError e) {
         || retryConfiguration.getSeries().contains(responseStatus.series());
   }
 
+  private boolean isNetworkError(RetrofitError e) {
+    return e.getResponse() == null && e.getCause() instanceof IOException;
+  }
+
   public Map<String, ?> processQueryAndReturnMap(
       String metricsAccountName,
       String storageAccountName,

diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java
@@ -40,6 +40,7 @@
 import com.netflix.kayenta.storage.StorageServiceRepository;
 import com.netflix.spectator.api.Registry;
 import java.io.IOException;
+import java.net.SocketTimeoutException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -223,6 +224,34 @@ public void retriesIoExceptionTillMaxAttemptsAndThrowsException() throws IOExcep
     verifyZeroInteractions(storageService);
   }
 
+  @Test
+  public void retriesNetworkErrorTillMaxAttemptsAndThrowsException() throws IOException {
+    when(metricsService.queryMetrics(
+            anyString(),
+            any(CanaryConfig.class),
+            any(CanaryMetricConfig.class),
+            any(CanaryScope.class)))
+        .thenThrow(RetrofitError.networkError("url", new SocketTimeoutException()));
+
+    assertThatThrownBy(
+            () ->
+                processor.executeQuery(
+                    METRICS,
+                    STORAGE,
+                    mock(CanaryConfig.class, RETURNS_DEEP_STUBS),
+                    1,
+                    mock(CanaryScope.class)))
+        .isInstanceOf(RetrofitError.class);
+
+    verify(metricsService, times(ATTEMPTS))
+        .queryMetrics(
+            anyString(),
+            any(CanaryConfig.class),
+            any(CanaryMetricConfig.class),
+            any(CanaryScope.class));
+    verifyZeroInteractions(storageService);
+  }
+
   private RetrofitError getRetrofitErrorWithHttpStatus(int status) {
     return RetrofitError.httpError(
         "url", new Response("url", status, "reason", Collections.emptyList(), null), null, null);