Repository: jclouds Updated Branches: refs/heads/master 7c5927038 -> b29f716a0
JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/b29f716a Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/b29f716a Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/b29f716a Branch: refs/heads/master Commit: b29f716a02c41cc64b7f5dd2efc8f10793cce5b9 Parents: 6a10544 Author: Ignasi Barrera <n...@apache.org> Authored: Fri May 4 09:27:25 2018 +0200 Committer: Ignasi Barrera <n...@apache.org> Committed: Fri May 4 11:03:43 2018 +0200 ---------------------------------------------------------------------- .../http/handlers/RateLimitRetryHandler.java | 2 +- .../jclouds/azurecompute/arm/domain/Error.java | 58 +++++++++++++ .../arm/handlers/AzureComputeErrorHandler.java | 13 ++- .../handlers/AzureRateLimitRetryHandler.java | 28 +++++++ .../handlers/AzureRetryableErrorHandler.java | 81 ++++++++++++++++++ .../AzureRetryableErrorHandlerTest.java | 88 ++++++++++++++++++++ 6 files changed, 267 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java b/core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java index 862ffd4..e5fda77 100644 --- a/core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java +++ b/core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java @@ -87,7 +87,7 @@ public abstract class RateLimitRetryHandler implements HttpRetryHandler { } } - private boolean delayRequestUntilAllowed(final HttpCommand command, final HttpResponse response) { + protected boolean delayRequestUntilAllowed(final HttpCommand command, final HttpResponse response) { Optional<Long> millisToNextAvailableRequest = millisToNextAvailableRequest(command, response); if (!millisToNextAvailableRequest.isPresent()) { logger.error("Cannot retry after rate limit error, no retry information provided in the response"); http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java ---------------------------------------------------------------------- diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java new file mode 100644 index 0000000..19a045f --- /dev/null +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.azurecompute.arm.domain; + +import java.util.List; + +import org.jclouds.javax.annotation.Nullable; +import org.jclouds.json.SerializedNames; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; + +@AutoValue +public abstract class Error { + + public abstract Details details(); + + @SerializedNames({ "error" }) + public static Error create(Details details) { + return new AutoValue_Error(details); + } + + Error() { + + } + + @AutoValue + public abstract static class Details { + public abstract String code(); + public abstract String message(); + public abstract List<Details> details(); + + @SerializedNames({ "code", "message", "details" }) + public static Details create(String code, String message, @Nullable List<Details> details) { + return new AutoValue_Error_Details(code, message, details == null ? ImmutableList.<Details> of() + : ImmutableList.copyOf(details)); + } + + Details() { + + } + } + +} http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java ---------------------------------------------------------------------- diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java index 8492d51..6673001 100644 --- a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java @@ -16,6 +16,8 @@ */ package org.jclouds.azurecompute.arm.handlers; +import static org.jclouds.azurecompute.arm.handlers.AzureRateLimitRetryHandler.isRateLimitError; + import java.io.IOException; import javax.inject.Singleton; @@ -38,7 +40,10 @@ public class AzureComputeErrorHandler implements HttpErrorHandler { @Override public void handleError(final HttpCommand command, final HttpResponse response) { - // it is important to always read fully and close streams + // It is important to always read fully and close streams + // For 429 errors the response body might have already been consumed as + // some errors report information in the response body that needs to be + // handled by the retry handlers. String message = parseMessage(response); Exception exception = message == null ? new HttpResponseException(command, response) @@ -70,7 +75,11 @@ public class AzureComputeErrorHandler implements HttpErrorHandler { exception = new IllegalStateException(message, exception); break; case 429: - exception = new AzureComputeRateLimitExceededException(response, exception); + if (isRateLimitError(response)) { + exception = new AzureComputeRateLimitExceededException(response, exception); + } else { + exception = new IllegalStateException(message, exception); + } break; default: } http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java ---------------------------------------------------------------------- diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java index ee5f5e5..e2c6270 100644 --- a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java @@ -16,6 +16,7 @@ */ package org.jclouds.azurecompute.arm.handlers; +import javax.inject.Inject; import javax.inject.Singleton; import org.jclouds.http.HttpCommand; @@ -26,14 +27,41 @@ import com.google.common.annotations.Beta; import com.google.common.base.Optional; import com.google.common.net.HttpHeaders; +/** + * Handles 429 Too Many Requests responses. + * <p> + * The Azure ARM provider also returns this 429 HTTP status code for some errors + * when resources are busy or in a state where they cannot be modified. In this + * case this handler delegates to the {@link AzureRetryableErrorHandler} to + * determine if the requests can be retried. + */ @Beta @Singleton public class AzureRateLimitRetryHandler extends RateLimitRetryHandler { + private final AzureRetryableErrorHandler retryableErrorHandler; + + @Inject + AzureRateLimitRetryHandler(AzureRetryableErrorHandler retryableErrorHandler) { + this.retryableErrorHandler = retryableErrorHandler; + } + + @Override + protected boolean delayRequestUntilAllowed(HttpCommand command, HttpResponse response) { + if (!isRateLimitError(response)) { + return retryableErrorHandler.shouldRetryRequest(command, response); + } + return super.delayRequestUntilAllowed(command, response); + } + @Override protected Optional<Long> millisToNextAvailableRequest(HttpCommand command, HttpResponse response) { String secondsToNextAvailableRequest = response.getFirstHeaderOrNull(HttpHeaders.RETRY_AFTER); return secondsToNextAvailableRequest != null ? Optional.of(Long.valueOf(secondsToNextAvailableRequest) * 1000) : Optional.<Long> absent(); } + + public static boolean isRateLimitError(HttpResponse response) { + return response.getFirstHeaderOrNull(HttpHeaders.RETRY_AFTER) != null; + } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java ---------------------------------------------------------------------- diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java new file mode 100644 index 0000000..07f7b5d --- /dev/null +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.azurecompute.arm.handlers; + +import static org.jclouds.azurecompute.arm.handlers.AzureRateLimitRetryHandler.isRateLimitError; + +import javax.annotation.Resource; +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.jclouds.azurecompute.arm.domain.Error; +import org.jclouds.http.HttpCommand; +import org.jclouds.http.HttpResponse; +import org.jclouds.http.functions.ParseJson; +import org.jclouds.http.handlers.BackoffLimitedRetryHandler; +import org.jclouds.logging.Logger; + +import com.google.common.annotations.Beta; + +/** + * This handles failed responses that return a <code>RetryableError</code>. + * <p> + * In order to determine if the error is retryable, the response body must be + * read, so this handler will have to buffer the response payload in memory so + * the response body can be read again in subsequent steps of the response + * processing flow. + */ +@Singleton +@Beta +public class AzureRetryableErrorHandler extends BackoffLimitedRetryHandler { + + private static final String RETRYABLE_ERROR_CODE = "RetryableError"; + + @Resource + protected Logger logger = Logger.NULL; + private final ParseJson<Error> parseError; + + @Inject + AzureRetryableErrorHandler(ParseJson<Error> parseError) { + this.parseError = parseError; + } + + @Override + public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { + // Only consider retryable errors and discard rate limit ones + if (response.getStatusCode() != 429 || isRateLimitError(response)) { + return false; + } + + try { + // Note that this will consume the response body. At this point, + // subsequent retry handlers or error handlers will not be able to read + // again the payload, but that should only be attempted when the + // command is not retryable and an exception should be thrown. + Error error = parseError.apply(response); + logger.debug("processing error: %s", error); + + boolean isRetryable = RETRYABLE_ERROR_CODE.equals(error.details().code()); + return isRetryable ? super.shouldRetryRequest(command, response) : false; + } catch (Exception ex) { + // If we can't parse the error, just assume it is not a retryable error + logger.warn("could not parse error. Request won't be retried: %s", ex.getMessage()); + return false; + } + } + +} http://git-wip-us.apache.org/repos/asf/jclouds/blob/b29f716a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java ---------------------------------------------------------------------- diff --git a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java new file mode 100644 index 0000000..49103ae --- /dev/null +++ b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.azurecompute.arm.handlers; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import org.jclouds.http.HttpCommand; +import org.jclouds.http.HttpRequest; +import org.jclouds.http.HttpResponse; +import org.jclouds.http.HttpRetryHandler; +import org.jclouds.json.config.GsonModule; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import com.google.common.net.HttpHeaders; +import com.google.inject.Guice; +import com.google.inject.Injector; + +@Test(groups = "unit", testName = "AzureRetryableErrorHandlerTest") +public class AzureRetryableErrorHandlerTest { + + private HttpRetryHandler handler; + + @BeforeClass + public void setup() { + // Initialize an injector with just the Json features to get all + // serialization stuff + Injector injector = Guice.createInjector(new GsonModule()); + handler = injector.getInstance(AzureRetryableErrorHandler.class); + } + + @Test + public void testDoesNotRetryWhenNot429() { + HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost";).build()); + HttpResponse response = HttpResponse.builder().statusCode(400).build(); + + assertFalse(handler.shouldRetryRequest(command, response)); + } + + @Test + public void testDoesNotRetryWhenRateLimitError() { + HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost";).build()); + HttpResponse response = HttpResponse.builder().statusCode(429).addHeader(HttpHeaders.RETRY_AFTER, "15").build(); + + assertFalse(handler.shouldRetryRequest(command, response)); + } + + @Test + public void testDoesNotRetryWhenCannotParseError() { + HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost";).build()); + HttpResponse response = HttpResponse.builder().statusCode(429).payload("foo").build(); + + assertFalse(handler.shouldRetryRequest(command, response)); + } + + @Test + public void testDoesNotRetryWhenErrorNotRetryable() { + String nonRetryable = "{\"error\":{\"code\":\"ReferencedResourceNotProvisioned\",\"message\":\"Not provisioned\"}}"; + HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost";).build()); + HttpResponse response = HttpResponse.builder().statusCode(429).payload(nonRetryable).build(); + + assertFalse(handler.shouldRetryRequest(command, response)); + } + + @Test + public void testRetriesWhenRetryableError() { + String retryable = "{\"error\":{\"code\":\"RetryableError\",\"message\":\"Resource busy\"}}"; + HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost";).build()); + HttpResponse response = HttpResponse.builder().statusCode(429).payload(retryable).build(); + + assertTrue(handler.shouldRetryRequest(command, response)); + } +}
