Improve wait time in retry logic for CC calls

multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/Messages.java

@@ -4,5 +4,8 @@ public class Messages {
 
     // INFO messages
     public static final String WAITING_MS_BEFORE_RETRYING_WITH_TIMEOUT_OF_MS = "Waiting: {} ms before retrying with timeout of: {} ms";
+    public static final String RATE_LIMITED_BY_CC_WAITING_S = "CC returned 429 with Retry-After: {} s. Waiting {} s (capped) before retrying.";
+    public static final String RATE_LIMITED_BY_CC_NO_HEADER_WAITING_MS = "CC returned 429 without Retry-After header. Waiting {} ms before retrying.";
+    public static final String RANDOM_WAIT_BEFORE_RETRY_MS = "Waiting {} ms (randomized) before retrying failed CC operation.";
 
 }

multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/facade/CloudOperationException.java

@@ -8,6 +8,7 @@ public class CloudOperationException extends CloudException {
     private final HttpStatus statusCode;
     private final String statusText;
     private final String description;
+    private final Long retryAfterSeconds;
 
     public CloudOperationException(HttpStatus statusCode) {
         this(statusCode, statusCode.getReasonPhrase());
@@ -22,10 +23,16 @@ public CloudOperationException(HttpStatus statusCode, String statusText, String
     }
 
     public CloudOperationException(HttpStatus statusCode, String statusText, String description, Throwable cause) {
+        this(statusCode, statusText, description, cause, null);
+    }
+
+    public CloudOperationException(HttpStatus statusCode, String statusText, String description, Throwable cause,
+                                   Long retryAfterSeconds) {
         super(getExceptionMessage(statusCode, statusText, description), cause);
         this.statusCode = statusCode;
         this.statusText = statusText;
         this.description = description;
+        this.retryAfterSeconds = retryAfterSeconds;
     }
 
     private static String getExceptionMessage(HttpStatus statusCode, String statusText, String description) {
@@ -47,4 +54,8 @@ public String getDescription() {
         return description;
     }
 
+    public Long getRetryAfterSeconds() {
+        return retryAfterSeconds;
+    }
+
 }

multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/facade/rest/CloudControllerResponseErrorHandler.java

@@ -1,37 +1,56 @@
 package org.cloudfoundry.multiapps.controller.client.facade.rest;
 
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.cloudfoundry.multiapps.controller.client.facade.CloudOperationException;
 import org.cloudfoundry.multiapps.controller.client.facade.util.CloudUtil;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.web.client.DefaultResponseErrorHandler;
 import org.springframework.web.client.RestClientException;
 
-import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
-
 public class CloudControllerResponseErrorHandler extends DefaultResponseErrorHandler {
 
     private static CloudOperationException getException(ClientHttpResponse response) throws IOException {
         HttpStatus statusCode = HttpStatus.valueOf(response.getStatusCode()
                                                            .value());
         String statusText = response.getStatusText();
+        Long retryAfterSeconds = extractRetryAfterSeconds(response, statusCode);
 
         ObjectMapper mapper = new ObjectMapper(); // can reuse, share globally
 
         if (response.getBody() != null) {
             try {
                 @SuppressWarnings("unchecked") Map<String, Object> responseBody = mapper.readValue(response.getBody(), Map.class);
                 String description = getTrimmedDescription(responseBody);
-                return new CloudOperationException(statusCode, statusText, description);
+                return new CloudOperationException(statusCode, statusText, description, null, retryAfterSeconds);
             } catch (IOException e) {
                 // Fall through. Handled below.
             }
         }
-        return new CloudOperationException(statusCode, statusText);
+        return new CloudOperationException(statusCode, statusText, null, null, retryAfterSeconds);
+    }
+
+    private static Long extractRetryAfterSeconds(ClientHttpResponse response, HttpStatus statusCode) {
+        if (statusCode != HttpStatus.TOO_MANY_REQUESTS) {
+            return null;
+        }
+        String headerValue = response.getHeaders()
+                                     .getFirst(HttpHeaders.RETRY_AFTER);
+        if (headerValue == null) {
+            return null;
+        }
+        try {
+            long parsed = Long.parseLong(headerValue);
+            return parsed >= 0 ? parsed : null;
+        } catch (NumberFormatException _) {
+            return null;
+        }
     }
 
     private static String getTrimmedDescription(Map<String, Object> responseBody) {

multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/util/ResilientCloudOperationExecutor.java

@@ -7,7 +7,11 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.function.Function;
+import java.util.function.LongConsumer;
+import java.util.function.LongSupplier;
+import java.util.function.Supplier;
 
 import org.apache.commons.collections4.SetUtils;
 import org.cloudfoundry.multiapps.common.util.MiscUtil;
@@ -23,15 +27,25 @@ public class ResilientCloudOperationExecutor extends ResilientOperationExecutor
 
     private static final Set<HttpStatus> DEFAULT_STATUSES_TO_IGNORE = Set.of(HttpStatus.GATEWAY_TIMEOUT, HttpStatus.REQUEST_TIMEOUT,
                                                                              HttpStatus.INTERNAL_SERVER_ERROR, HttpStatus.BAD_GATEWAY,
-                                                                             HttpStatus.SERVICE_UNAVAILABLE);
+                                                                             HttpStatus.SERVICE_UNAVAILABLE,
+                                                                             HttpStatus.TOO_MANY_REQUESTS);
 
     private static final int DEFAULT_TIMEOUT_RETRY_WAIT_TIME_IN_MILLIS = 30 * 1000; // 30 seconds
 
     private static final Map<Integer, Duration> RETRY_COUNT_RESPONSE_TIME_BACKOFF = Map.of(1, Duration.ofMinutes(5), 2,
                                                                                            Duration.ofMinutes(8), 3,
                                                                                            Duration.ofMinutes(15));
 
+    private static final long RATE_LIMIT_RETRY_AFTER_CAP_IN_SECONDS = 120L;
+    private static final long RATE_LIMIT_FALLBACK_WAIT_IN_MILLIS = 60_000L;
+    private static final long RANDOM_RETRY_MIN_WAIT_IN_MILLIS = 30_000L;
+    private static final long RANDOM_RETRY_MAX_WAIT_IN_MILLIS = 90_000L;
+
     private Set<HttpStatus> additionalStatusesToIgnore = Collections.emptySet();
+    private LongConsumer sleeper = MiscUtil::sleep;
+    private LongSupplier randomDelaySupplier = () -> ThreadLocalRandom.current()
+                                                                      .nextLong(RANDOM_RETRY_MIN_WAIT_IN_MILLIS,
+                                                                                RANDOM_RETRY_MAX_WAIT_IN_MILLIS + 1);
 
     @Override
     public ResilientCloudOperationExecutor withRetryCount(long retryCount) {
@@ -48,6 +62,30 @@ public ResilientCloudOperationExecutor withStatusesToIgnore(HttpStatus... status
         return this;
     }
 
+    ResilientCloudOperationExecutor withSleeper(LongConsumer sleeper) {
+        this.sleeper = sleeper;
+        return this;
+    }
+
+    ResilientCloudOperationExecutor withRandomDelaySupplier(LongSupplier randomDelaySupplier) {
+        this.randomDelaySupplier = randomDelaySupplier;
+        return this;
+    }
+
+    @Override
+    public <T> T execute(Supplier<T> operation) {
+        for (long i = 1; i < retryCount; i++) {
+            try {
+                return operation.get();
+            } catch (RuntimeException e) {
+                handle(e);
+                long waitMillis = computeWaitMillis(e, i);
+                sleeper.accept(waitMillis);
+            }
+        }
+        return operation.get();
+    }
+
     public <T> T executeWithExponentialBackoff(Function<Duration, T> operation) {
         int waitTimeBetweenRetriesInMillis = DEFAULT_TIMEOUT_RETRY_WAIT_TIME_IN_MILLIS;
         int retryIndex = 1;
@@ -87,6 +125,34 @@ protected void handle(CloudOperationException e) {
                     e);
     }
 
+    private long computeWaitMillis(RuntimeException e, long attemptIndex) {
+        if (isRateLimitException(e)) {
+            return computeRateLimitWaitMillis((CloudOperationException) e);
+        }
+        if (attemptIndex >= 2) {
+            long waitMillis = randomDelaySupplier.getAsLong();
+            LOGGER.info(Messages.RANDOM_WAIT_BEFORE_RETRY_MS, waitMillis);
+            return waitMillis;
+        }
+        return waitTimeBetweenRetriesInMillis;
+    }
+
+    private boolean isRateLimitException(RuntimeException e) {
+        return e instanceof CloudOperationException cloudException
+            && cloudException.getStatusCode() == HttpStatus.TOO_MANY_REQUESTS;
+    }
+
+    private long computeRateLimitWaitMillis(CloudOperationException e) {
+        Long retryAfterSeconds = e.getRetryAfterSeconds();
+        if (retryAfterSeconds == null) {
+            LOGGER.info(Messages.RATE_LIMITED_BY_CC_NO_HEADER_WAITING_MS, RATE_LIMIT_FALLBACK_WAIT_IN_MILLIS);
+            return RATE_LIMIT_FALLBACK_WAIT_IN_MILLIS;
+        }
+        long cappedSeconds = Math.clamp(retryAfterSeconds, 1L, RATE_LIMIT_RETRY_AFTER_CAP_IN_SECONDS);
+        LOGGER.info(Messages.RATE_LIMITED_BY_CC_WAITING_S, retryAfterSeconds, cappedSeconds);
+        return cappedSeconds * 1000L;
+    }
+
     private boolean shouldRetry(CloudOperationException e) {
         return getStatusesToIgnore().contains(e.getStatusCode());
     }

multiapps-controller-client/src/main/java/org/cloudfoundry/multiapps/controller/client/util/ResilientOperationExecutor.java

@@ -16,7 +16,7 @@ public class ResilientOperationExecutor {
     private static final long DEFAULT_RETRY_COUNT = 3;
     private static final long DEFAULT_WAIT_TIME_BETWEEN_RETRIES_IN_MILLIS = 5000;
 
-    private long waitTimeBetweenRetriesInMillis = DEFAULT_WAIT_TIME_BETWEEN_RETRIES_IN_MILLIS;
+    protected long waitTimeBetweenRetriesInMillis = DEFAULT_WAIT_TIME_BETWEEN_RETRIES_IN_MILLIS;
     protected long retryCount = DEFAULT_RETRY_COUNT;
 
     public ResilientOperationExecutor withRetryCount(long retryCount) {

multiapps-controller-client/src/test/java/org/cloudfoundry/multiapps/controller/client/facade/CloudOperationExceptionTest.java

@@ -42,4 +42,32 @@ void testCauseIsPropagatedToSuper() {
 
         Assertions.assertSame(cause, e.getCause());
     }
+
+    @Test
+    void testRetryAfterSecondsNullByDefault() {
+        CloudOperationException e = new CloudOperationException(HttpStatus.TOO_MANY_REQUESTS);
+
+        Assertions.assertNull(e.getRetryAfterSeconds());
+    }
+
+    @Test
+    void testRetryAfterSecondsStoredWhenProvided() {
+        CloudOperationException e = new CloudOperationException(HttpStatus.TOO_MANY_REQUESTS,
+                                                                HttpStatus.TOO_MANY_REQUESTS.getReasonPhrase(),
+                                                                null, null, 60L);
+
+        Assertions.assertEquals(60L, e.getRetryAfterSeconds());
+    }
+
+    @Test
+    void testFourArgConstructorLeavesRetryAfterSecondsNull() {
+        Throwable cause = new RuntimeException("boom");
+
+        CloudOperationException e = new CloudOperationException(HttpStatus.TOO_MANY_REQUESTS,
+                                                               HttpStatus.TOO_MANY_REQUESTS.getReasonPhrase(), "throttled", cause);
+
+        Assertions.assertNull(e.getRetryAfterSeconds());
+        Assertions.assertSame(cause, e.getCause());
+    }
 }
+