From a4d1adc3aa2d0439fabdb61782bd20bab0d3839e Mon Sep 17 00:00:00 2001 From: veudayab Date: Wed, 31 Jul 2013 18:55:32 -0700 Subject: [PATCH 1/6] Change how we calculate ExponentialRetry backo0ff interval to prevent possible overflow --- .../core/storage/RetryExponentialRetry.java | 5 +-- .../table/client/TableClientTests.java | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/RetryExponentialRetry.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/RetryExponentialRetry.java index 36b801ffb873b..8bdbf95473ead 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/RetryExponentialRetry.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/RetryExponentialRetry.java @@ -126,14 +126,15 @@ public RetryResult shouldRetry(final int currentRetryCount, final int statusCode if (currentRetryCount < this.maximumAttempts) { // Calculate backoff Interval between 80% and 120% of the desired // backoff, multiply by 2^n -1 for exponential - int incrementDelta = (int) (Math.pow(2, currentRetryCount) - 1); + double incrementDelta = (Math.pow(2, currentRetryCount) - 1); final int boundedRandDelta = (int) (this.deltaBackoffIntervalInMs * 0.8) + this.randRef.nextInt((int) (this.deltaBackoffIntervalInMs * 1.2) - (int) (this.deltaBackoffIntervalInMs * 0.8)); incrementDelta *= boundedRandDelta; // Enforce max / min backoffs - return new RetryResult(Math.min(this.resolvedMinBackoff + incrementDelta, this.resolvedMaxBackoff), true); + return new RetryResult((int) Math.round(Math.min(this.resolvedMinBackoff + incrementDelta, + this.resolvedMaxBackoff)), true); } else { return new RetryResult(-1, false); diff --git a/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/table/client/TableClientTests.java b/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/table/client/TableClientTests.java index dda98602e798f..cb2891409164e 100644 --- a/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/table/client/TableClientTests.java +++ b/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/table/client/TableClientTests.java @@ -34,7 +34,12 @@ import org.junit.Test; import com.microsoft.windowsazure.services.core.storage.AuthenticationScheme; +import com.microsoft.windowsazure.services.core.storage.OperationContext; import com.microsoft.windowsazure.services.core.storage.ResultSegment; +import com.microsoft.windowsazure.services.core.storage.RetryExponentialRetry; +import com.microsoft.windowsazure.services.core.storage.RetryLinearRetry; +import com.microsoft.windowsazure.services.core.storage.RetryPolicy; +import com.microsoft.windowsazure.services.core.storage.RetryResult; import com.microsoft.windowsazure.services.core.storage.StorageCredentialsSharedAccessSignature; import com.microsoft.windowsazure.services.core.storage.StorageException; @@ -672,4 +677,31 @@ private CloudTableClient getTableForSas(CloudTable table, SharedAccessTablePolic .generateSharedAccessSignature(policy, accessIdentifier, startPk, startRk, endPk, endRk); return new CloudTableClient(tClient.getEndpoint(), new StorageCredentialsSharedAccessSignature(sasString)); } + + @Test + public void VerifyBackoffTimeOverflow() { + RetryExponentialRetry exponentialRetry = new RetryExponentialRetry(4000, 100000); + VerifyBackoffTimeOverflow(exponentialRetry, 100000); + + RetryLinearRetry linearRetry = new RetryLinearRetry(4000, 100000); + VerifyBackoffTimeOverflow(linearRetry, 100000); + } + + private void VerifyBackoffTimeOverflow(RetryPolicy retryPolicy, int maxAttempts) { + Exception e = new Exception(); + OperationContext context = new OperationContext(); + int previousRetryInterval = 1000; // larger than zero to ensure we never get zero back + + for (int i = 0; i < maxAttempts; i++) { + RetryResult result = retryPolicy.shouldRetry(i, HttpURLConnection.HTTP_INTERNAL_ERROR, e, context); + int retryInterval = result.getBackOffIntervalInMs(); + Assert.assertTrue(String.format("Attempt: '%d'", i), result.isShouldRetry()); + Assert.assertTrue(String.format("Retry Interval: '%d', Previous Retry Interval: '%d', Attempt: '%d'", + retryInterval, previousRetryInterval, i), retryInterval >= previousRetryInterval); + previousRetryInterval = retryInterval; + } + + Assert.assertFalse(retryPolicy.shouldRetry(maxAttempts, HttpURLConnection.HTTP_INTERNAL_ERROR, e, context) + .isShouldRetry()); + } } From ae264ea9b5aa3f261a32cd6a8b0a581d4f66a2b1 Mon Sep 17 00:00:00 2001 From: veudayab Date: Fri, 2 Aug 2013 10:13:24 -0700 Subject: [PATCH 2/6] Allow setting client-request-id on opContext; Retry on IOException when they are wrapped in XMLStreamException --- .../services/core/storage/Constants.java | 5 ++++ .../core/storage/OperationContext.java | 21 ++++++++++++--- .../utils/implementation/BaseRequest.java | 2 ++ .../utils/implementation/ExecutionEngine.java | 6 +++-- .../blob/client/CloudBlobContainerTests.java | 26 +++++++++++++++++++ 5 files changed, 54 insertions(+), 6 deletions(-) diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/Constants.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/Constants.java index baea2ec847207..a1454cf02d81a 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/Constants.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/Constants.java @@ -272,6 +272,11 @@ public static class HeaderConstants { */ public static final String REQUEST_ID_HEADER = PREFIX_FOR_STORAGE_HEADER + "request-id"; + /** + * The header that indicates the client request ID. + */ + public static final String CLIENT_REQUEST_ID_HEADER = PREFIX_FOR_STORAGE_HEADER + "client-request-id"; + /** * The header for the If-Match condition. */ diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/OperationContext.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/OperationContext.java index d19a8eba6b649..f502cf36229b6 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/OperationContext.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/OperationContext.java @@ -38,9 +38,7 @@ public final class OperationContext { /** * The UUID representing the client side trace ID. */ - // V2 expose when logging is available. - @SuppressWarnings("unused") - private final String clientTraceID; + private String clientRequestID; /** * The Logger object associated with this operation. @@ -98,10 +96,17 @@ public final class OperationContext { * Creates an instance of the OperationContext class. */ public OperationContext() { - this.clientTraceID = UUID.randomUUID().toString(); + this.clientRequestID = UUID.randomUUID().toString(); this.requestResults = new ArrayList(); } + /** + * @return the clientRequestID + */ + public String getClientRequestID() { + return this.clientRequestID; + } + /** * @return the clientTimeInMs */ @@ -201,6 +206,14 @@ public void initialize() { this.setCurrentRequestObject(null); } + /** + * @param clientRequestID + * the clientRequestID to set + */ + public void setClientRequestID(final String clientRequestID) { + this.clientRequestID = clientRequestID; + } + /** * @param clientTimeInMs * the clientTimeInMs to set diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/BaseRequest.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/BaseRequest.java index 19538da0c9854..92209d4e18743 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/BaseRequest.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/BaseRequest.java @@ -200,6 +200,8 @@ public static HttpURLConnection createURLConnection(final URI uri, final int tim retConnection.setRequestProperty(Constants.HeaderConstants.STORAGE_VERSION_HEADER, Constants.HeaderConstants.TARGET_STORAGE_VERSION); retConnection.setRequestProperty(Constants.HeaderConstants.USER_AGENT, getUserAgent()); + retConnection.setRequestProperty(Constants.HeaderConstants.CLIENT_REQUEST_ID_HEADER, + opContext.getClientRequestID()); // Java6 TODO remove me, this has to be manually set or it will // sometimes default to application/x-www-form-urlencoded without us diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java index 9e60f511677bd..01152b0251574 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java @@ -168,11 +168,13 @@ public static RESULT_TYPE executeWithRet task.getResult().setException(translatedException); } catch (final XMLStreamException e) { - // Non Retryable, just throw + // Non Retryable except when the inner exception is actually an IOException translatedException = StorageException .translateException(getLastRequestObject(opContext), e, opContext); task.getResult().setException(translatedException); - throw translatedException; + if (!(e.getCause() instanceof IOException)) { + throw translatedException; + } } catch (final InvalidKeyException e) { // Non Retryable, just throw diff --git a/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/blob/client/CloudBlobContainerTests.java b/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/blob/client/CloudBlobContainerTests.java index 36a73da82be8e..227ce6b97c188 100644 --- a/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/blob/client/CloudBlobContainerTests.java +++ b/microsoft-azure-api/src/test/java/com/microsoft/windowsazure/services/blob/client/CloudBlobContainerTests.java @@ -399,6 +399,32 @@ public void testBlobLeaseBreak() throws URISyntaxException, StorageException, IO Assert.assertTrue(operationContext.getLastResult().getStatusCode() == HttpURLConnection.HTTP_ACCEPTED); } + @Test + public void testListBlobs() throws URISyntaxException, StorageException, IOException, InterruptedException { + final int length = 128; + final Random randGenerator = new Random(); + final byte[] buff = new byte[length]; + randGenerator.nextBytes(buff); + + String blobName = "testBlob" + Integer.toString(randGenerator.nextInt(50000)); + blobName = blobName.replace('-', '_'); + + final CloudBlobContainer existingContainer = bClient.getContainerReference(testSuiteContainerName); + final CloudBlob blobRef = existingContainer.getBlockBlobReference(blobName); + final BlobRequestOptions options = new BlobRequestOptions(); + OperationContext context = new OperationContext(); + context.setClientRequestID("My-Helpful-TraceId"); + + // Subscribe to sending request event once we move that. Currently, the sendingrequest event is fired + // after the connection is initiated and we don't have access to the request headers at that time. + + blobRef.upload(new ByteArrayInputStream(buff), -1, null, options, context); + + for (ListBlobItem blob : existingContainer.listBlobs()) { + Assert.assertEquals(blobRef.getClass(), blob.getClass()); + } + } + @Test public void testBlobLeaseRenew() throws URISyntaxException, StorageException, IOException, InterruptedException { final int length = 128; From f26734fe34c8bf00d6fd8931e0535c89afc8fafa Mon Sep 17 00:00:00 2001 From: veudayab Date: Wed, 11 Sep 2013 10:01:21 -0700 Subject: [PATCH 3/6] Throw a more meaningful exception when a connection is reset while parsing the result --- .../core/storage/StorageException.java | 8 +++++++- .../utils/implementation/ExecutionEngine.java | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java index a0a6f0f7349a4..6214a94e0d4d5 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.net.HttpURLConnection; +import java.net.SocketException; import javax.xml.stream.XMLStreamException; @@ -89,7 +90,12 @@ public static StorageException translateException(final HttpURLConnection reques int responseCode = 0; try { responseCode = request.getResponseCode(); - responseMessage = request.getResponseMessage(); + if (cause instanceof SocketException) { + responseMessage = request.getResponseMessage(); + } + else { + responseMessage = cause.getMessage(); + } } catch (final IOException e) { // ignore errors diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java index 01152b0251574..3a8155a8e6811 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/utils/implementation/ExecutionEngine.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URISyntaxException; import java.security.InvalidKeyException; @@ -34,6 +35,7 @@ import com.microsoft.windowsazure.services.core.storage.RetryPolicyFactory; import com.microsoft.windowsazure.services.core.storage.RetryResult; import com.microsoft.windowsazure.services.core.storage.SendingRequestEvent; +import com.microsoft.windowsazure.services.core.storage.StorageErrorCode; import com.microsoft.windowsazure.services.core.storage.StorageErrorCodeStrings; import com.microsoft.windowsazure.services.core.storage.StorageException; import com.microsoft.windowsazure.services.core.storage.utils.Utility; @@ -169,10 +171,20 @@ public static RESULT_TYPE executeWithRet } catch (final XMLStreamException e) { // Non Retryable except when the inner exception is actually an IOException - translatedException = StorageException - .translateException(getLastRequestObject(opContext), e, opContext); + + // Only in the case of xml exceptions that are due to connection issues. + if (e.getCause() instanceof SocketException) { + translatedException = new StorageException(StorageErrorCode.SERVICE_INTERNAL_ERROR.toString(), + "An unknown failure occurred : ".concat(e.getCause().getMessage()), + HttpURLConnection.HTTP_INTERNAL_ERROR, null, e); + } + else { + translatedException = StorageException.translateException(getLastRequestObject(opContext), e, + opContext); + } + task.getResult().setException(translatedException); - if (!(e.getCause() instanceof IOException)) { + if (!(e.getCause() instanceof IOException) && !(e.getCause() instanceof SocketException)) { throw translatedException; } } From 6c76ce8dd1b15f84748dc6141930634a042824a3 Mon Sep 17 00:00:00 2001 From: veudayab Date: Thu, 12 Sep 2013 17:29:57 -0700 Subject: [PATCH 4/6] Check whether cause is null before trying to get the response message --- .../services/core/storage/StorageException.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java index 6214a94e0d4d5..f3a6a3ba8934e 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/core/storage/StorageException.java @@ -90,10 +90,13 @@ public static StorageException translateException(final HttpURLConnection reques int responseCode = 0; try { responseCode = request.getResponseCode(); - if (cause instanceof SocketException) { + + // When the exception is expected(IfNotExists) or we have already parsed the exception, we pass null as + // the cause. In such cases, we should just try to get the message from the request. + if (cause == null || cause instanceof SocketException) { responseMessage = request.getResponseMessage(); } - else { + else if (cause != null) { responseMessage = cause.getMessage(); } } From 6f9e465dbc3eb1a318fc6d9434acc40daa5830fb Mon Sep 17 00:00:00 2001 From: Albert Cheng Date: Mon, 16 Sep 2013 18:22:03 -0700 Subject: [PATCH 5/6] fix a importation error for DateConverter. --- .../services/management/implementation/ManagementRestProxy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/management/implementation/ManagementRestProxy.java b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/management/implementation/ManagementRestProxy.java index 21349275cb11e..ee12354b10065 100644 --- a/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/management/implementation/ManagementRestProxy.java +++ b/microsoft-azure-api/src/main/java/com/microsoft/windowsazure/services/management/implementation/ManagementRestProxy.java @@ -26,7 +26,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import com.microsoft.windowsazure.services.blob.implementation.RFC1123DateConverter; +import com.microsoft.windowsazure.services.core.RFC1123DateConverter; import com.microsoft.windowsazure.services.core.ServiceFilter; import com.microsoft.windowsazure.services.core.UserAgentFilter; import com.microsoft.windowsazure.services.core.utils.pipeline.ClientFilterAdapter; From bdb5bab49629dccbd9ab1771ac2cb7082e5ad722 Mon Sep 17 00:00:00 2001 From: bradygaster Date: Tue, 1 Oct 2013 12:46:24 -0700 Subject: [PATCH 6/6] updating for release --- ChangeLog.txt | 6 ++++++ microsoft-azure-api/pom.xml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 166c01ca6e6f0..034592cc26044 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,9 @@ +2013.09.30 Version 0.4.6 + * Allow users to set the client-request-id for better tracking/debugging of storage requests. This is set on the OperationContext. + * Prevent a potential arithmetic overflow while calculating the exponential retry back-off interval in storage. + * Retry on IOException even when it is wrapped within an XMLStreamException in storage. + * Throw a more meaningful exception when connection is reset while parsing a request response in storage. + 2013.08.26 Version 0.4.5 * Added support for managing affinity groups * Added support for Media Services job notification diff --git a/microsoft-azure-api/pom.xml b/microsoft-azure-api/pom.xml index d347b27fb3037..6b2b5eedd96e1 100644 --- a/microsoft-azure-api/pom.xml +++ b/microsoft-azure-api/pom.xml @@ -17,7 +17,7 @@ 4.0.0 com.microsoft.windowsazure microsoft-windowsazure-api - 0.4.5 + 0.4.6 jar Microsoft Windows Azure Client API