From dc9c32e495c5290780ed8fe6509b23f21159ee38 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Thu, 14 Jan 2021 10:33:39 -0800 Subject: [PATCH 1/6] adding retry on 413 --- .../src/BulkPartitionKeyRangeGoneRetryPolicy.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs index 0191c13e52..f5682e87d5 100644 --- a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs @@ -103,6 +103,11 @@ private async Task ShouldRetryInternalAsync( } } + if (statusCode == HttpStatusCode.RequestEntityTooLarge) + { + return ShouldRetryResult.RetryAfter(TimeSpan.Zero); + } + return null; } } From 093465775e79d8db13734e9b71494172abe9d0a7 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Thu, 14 Jan 2021 10:36:19 -0800 Subject: [PATCH 2/6] adding note --- .../src/BulkPartitionKeyRangeGoneRetryPolicy.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs index f5682e87d5..0b549e4e1a 100644 --- a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs @@ -103,6 +103,8 @@ private async Task ShouldRetryInternalAsync( } } + // Batch API can return 413 which means the response is bigger than 4Mb. + // Operations that exceed the 4Mb limit are returned as 413, while the operations within the 4Mb limit will be 200 if (statusCode == HttpStatusCode.RequestEntityTooLarge) { return ShouldRetryResult.RetryAfter(TimeSpan.Zero); From 9bb5831878268db05ab3d1c209019df6608eafab Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Thu, 14 Jan 2021 10:46:22 -0800 Subject: [PATCH 3/6] Adding tests --- .../Batch/BatchAsyncBatcherTests.cs | 80 +++++++++++++++++++ .../Batch/BatchAsyncOperationContextTests.cs | 13 +++ ...lkPartitionKeyRangeGoneRetryPolicyTests.cs | 12 +++ 3 files changed, 105 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs index e0e85b85a2..d7158c2169 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs @@ -203,6 +203,59 @@ private BatchAsyncBatcherExecuteDelegate ExecutorWithCompletingPartitionMigratio return new PartitionKeyRangeBatchExecutionResult(request.PartitionKeyRangeId, request.Operations, batchresponse); }; + private readonly BatchAsyncBatcherExecuteDelegate ExecutorWith413 + = async (PartitionKeyRangeServerBatchRequest request, CancellationToken cancellationToken) => + { + List results = new List(); + ItemBatchOperation[] arrayOperations = new ItemBatchOperation[request.Operations.Count]; + int index = 0; + foreach (ItemBatchOperation operation in request.Operations) + { + if (index == 0) + { + // First operation is fine + results.Add( + new TransactionalBatchOperationResult(HttpStatusCode.OK) + { + ETag = operation.Id + }); + } + else + { + // second operation is too big + results.Add( + new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge) + { + ETag = operation.Id + }); + } + + arrayOperations[index++] = operation; + } + + MemoryStream responseContent = await new BatchResponsePayloadWriter(results).GeneratePayloadAsync(); + + SinglePartitionKeyServerBatchRequest batchRequest = await SinglePartitionKeyServerBatchRequest.CreateAsync( + partitionKey: null, + operations: new ArraySegment(arrayOperations), + serializerCore: MockCosmosUtil.Serializer, + cancellationToken: cancellationToken); + + ResponseMessage responseMessage = new ResponseMessage((HttpStatusCode)207) + { + Content = responseContent + }; + + TransactionalBatchResponse batchresponse = await TransactionalBatchResponse.FromResponseMessageAsync( + responseMessage, + batchRequest, + MockCosmosUtil.Serializer, + true, + CancellationToken.None); + + return new PartitionKeyRangeBatchExecutionResult(request.PartitionKeyRangeId, request.Operations, batchresponse); + }; + // The response will include all but 2 operation responses private BatchAsyncBatcherExecuteDelegate ExecutorWithLessResponses = async (PartitionKeyRangeServerBatchRequest request, CancellationToken cancellationToken) => @@ -552,6 +605,33 @@ public async Task RetrierGetsCalledOnOverFlow() retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Once); } + [TestMethod] + public async Task RetrierGetsCalledOn413() + { + IDocumentClientRetryPolicy retryPolicy1 = new BulkPartitionKeyRangeGoneRetryPolicy( + GetSplitEnabledContainer(), + new ResourceThrottleRetryPolicy(1)); + + IDocumentClientRetryPolicy retryPolicy2 = new BulkPartitionKeyRangeGoneRetryPolicy( + GetSplitEnabledContainer(), + new ResourceThrottleRetryPolicy(1)); + + ItemBatchOperation operation1 = this.CreateItemBatchOperation(); + ItemBatchOperation operation2 = this.CreateItemBatchOperation(); + operation1.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy1)); + operation2.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy2)); + + Mock retryDelegate = new Mock(); + + BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(2, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413, retryDelegate.Object); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation1)); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); + await batchAsyncBatcher.DispatchAsync(metric); + retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny()), Times.Never); + retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny()), Times.Once); + retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Once); + } + private static ContainerInternal GetSplitEnabledContainer() { Mock container = new Mock(); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs index 989506ff6d..738af722d5 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs @@ -117,6 +117,19 @@ public async Task ShouldRetry_WithPolicy_On429() Assert.IsTrue(shouldRetryResult.ShouldRetry); } + [TestMethod] + public async Task ShouldRetry_WithPolicy_On413() + { + IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + Mock.Of(), + new ResourceThrottleRetryPolicy(1)); + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); + operation.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy)); + ShouldRetryResult shouldRetryResult = await operation.Context.ShouldRetryAsync(result, default); + Assert.IsTrue(shouldRetryResult.ShouldRetry); + } + [TestMethod] public async Task ShouldRetry_WithPolicy_OnSplit() { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs index 0548932c00..b7ccb4e0f1 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs @@ -41,6 +41,18 @@ public async Task RetriesOn429() Assert.IsTrue(shouldRetryResult.ShouldRetry); } + [TestMethod] + public async Task RetriesOn413() + { + IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + Mock.Of(), + new ResourceThrottleRetryPolicy(1)); + + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + ShouldRetryResult shouldRetryResult = await retryPolicy.ShouldRetryAsync(result.ToResponseMessage(), default); + Assert.IsTrue(shouldRetryResult.ShouldRetry); + } + [TestMethod] public async Task RetriesOnSplits() { From 865695bf37ab1d76af3ba2021db6c67c11f53713 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Thu, 14 Jan 2021 11:28:19 -0800 Subject: [PATCH 4/6] Renaming policy --- .../src/Batch/BatchAsyncContainerExecutor.cs | 2 +- ...etryPolicy.cs => BulkExecutionRetryPolicy.cs} | 4 ++-- .../Batch/BatchAsyncBatcherTests.cs | 16 ++++++++-------- .../Batch/BatchAsyncOperationContextTests.cs | 12 ++++++------ .../BulkPartitionKeyRangeGoneRetryPolicyTests.cs | 12 ++++++------ 5 files changed, 23 insertions(+), 23 deletions(-) rename Microsoft.Azure.Cosmos/src/{BulkPartitionKeyRangeGoneRetryPolicy.cs => BulkExecutionRetryPolicy.cs} (96%) diff --git a/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs b/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs index d8e6b54008..6bc66ec284 100644 --- a/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs +++ b/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs @@ -135,7 +135,7 @@ private static IDocumentClientRetryPolicy GetRetryPolicy( ContainerInternal containerInternal, RetryOptions retryOptions) { - return new BulkPartitionKeyRangeGoneRetryPolicy( + return new BulkExecutionRetryPolicy( containerInternal, new ResourceThrottleRetryPolicy( retryOptions.MaxRetryAttemptsOnThrottledRequests, diff --git a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs similarity index 96% rename from Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs rename to Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs index 0b549e4e1a..c83b480fbe 100644 --- a/Microsoft.Azure.Cosmos/src/BulkPartitionKeyRangeGoneRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs @@ -16,12 +16,12 @@ namespace Microsoft.Azure.Cosmos /// /// /// - internal sealed class BulkPartitionKeyRangeGoneRetryPolicy : IDocumentClientRetryPolicy + internal sealed class BulkExecutionRetryPolicy : IDocumentClientRetryPolicy { private readonly IDocumentClientRetryPolicy nextRetryPolicy; private readonly ContainerInternal container; - public BulkPartitionKeyRangeGoneRetryPolicy( + public BulkExecutionRetryPolicy( ContainerInternal container, IDocumentClientRetryPolicy nextRetryPolicy) { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs index d7158c2169..830fffe1ed 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs @@ -501,11 +501,11 @@ public async Task CannotAddToDispatchedBatch() [TestMethod] public async Task RetrierGetsCalledOnSplit() { - IDocumentClientRetryPolicy retryPolicy1 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); - IDocumentClientRetryPolicy retryPolicy2 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); @@ -530,11 +530,11 @@ public async Task RetrierGetsCalledOnSplit() [TestMethod] public async Task RetrierGetsCalledOnCompletingSplit() { - IDocumentClientRetryPolicy retryPolicy1 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); - IDocumentClientRetryPolicy retryPolicy2 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); @@ -559,11 +559,11 @@ public async Task RetrierGetsCalledOnCompletingSplit() [TestMethod] public async Task RetrierGetsCalledOnCompletingPartitionMigration() { - IDocumentClientRetryPolicy retryPolicy1 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); - IDocumentClientRetryPolicy retryPolicy2 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); @@ -608,11 +608,11 @@ public async Task RetrierGetsCalledOnOverFlow() [TestMethod] public async Task RetrierGetsCalledOn413() { - IDocumentClientRetryPolicy retryPolicy1 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); - IDocumentClientRetryPolicy retryPolicy2 = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs index 738af722d5..96ae876ff3 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs @@ -94,7 +94,7 @@ public async Task ShouldRetry_NoPolicy() [TestMethod] public async Task ShouldRetry_WithPolicy_OnSuccess() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.OK); @@ -107,7 +107,7 @@ public async Task ShouldRetry_WithPolicy_OnSuccess() [TestMethod] public async Task ShouldRetry_WithPolicy_On429() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult((HttpStatusCode)StatusCodes.TooManyRequests); @@ -120,7 +120,7 @@ public async Task ShouldRetry_WithPolicy_On429() [TestMethod] public async Task ShouldRetry_WithPolicy_On413() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); @@ -133,7 +133,7 @@ public async Task ShouldRetry_WithPolicy_On413() [TestMethod] public async Task ShouldRetry_WithPolicy_OnSplit() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.PartitionKeyRangeGone }; @@ -146,7 +146,7 @@ public async Task ShouldRetry_WithPolicy_OnSplit() [TestMethod] public async Task ShouldRetry_WithPolicy_OnCompletingSplit() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingSplit }; @@ -159,7 +159,7 @@ public async Task ShouldRetry_WithPolicy_OnCompletingSplit() [TestMethod] public async Task ShouldRetry_WithPolicy_OnCompletingPartitionMigration() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingPartitionMigration }; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs index b7ccb4e0f1..73b96a6b62 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs @@ -20,7 +20,7 @@ public class BulkPartitionKeyRangeGoneRetryPolicyTests [TestMethod] public async Task NotRetryOnSuccess() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); @@ -32,7 +32,7 @@ public async Task NotRetryOnSuccess() [TestMethod] public async Task RetriesOn429() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); @@ -44,7 +44,7 @@ public async Task RetriesOn429() [TestMethod] public async Task RetriesOn413() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), new ResourceThrottleRetryPolicy(1)); @@ -56,7 +56,7 @@ public async Task RetriesOn413() [TestMethod] public async Task RetriesOnSplits() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); @@ -68,7 +68,7 @@ public async Task RetriesOnSplits() [TestMethod] public async Task RetriesOnCompletingSplits() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); @@ -80,7 +80,7 @@ public async Task RetriesOnCompletingSplits() [TestMethod] public async Task RetriesOnCompletingPartitionMigrationSplits() { - IDocumentClientRetryPolicy retryPolicy = new BulkPartitionKeyRangeGoneRetryPolicy( + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), new ResourceThrottleRetryPolicy(1)); From 00fe66278b5bd74f305ae3a8d87c4e56bd3ea009 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Thu, 14 Jan 2021 17:13:44 -0800 Subject: [PATCH 5/6] Should be only on reads --- .../src/Batch/BatchAsyncContainerExecutor.cs | 4 +- .../src/BulkExecutionRetryPolicy.cs | 8 +++- .../Batch/BatchAsyncBatcherTests.cs | 39 ++++++++++++++++++- .../Batch/BatchAsyncOperationContextTests.cs | 22 ++++++++++- ...lkPartitionKeyRangeGoneRetryPolicyTests.cs | 21 +++++++++- 5 files changed, 89 insertions(+), 5 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs b/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs index 6bc66ec284..7e90e4fd99 100644 --- a/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs +++ b/Microsoft.Azure.Cosmos/src/Batch/BatchAsyncContainerExecutor.cs @@ -84,7 +84,7 @@ public virtual async Task AddAsync( string resolvedPartitionKeyRangeId = await this.ResolvePartitionKeyRangeIdAsync(operation, cancellationToken).ConfigureAwait(false); BatchAsyncStreamer streamer = this.GetOrAddStreamerForPartitionKeyRange(resolvedPartitionKeyRangeId); - ItemBatchOperationContext context = new ItemBatchOperationContext(resolvedPartitionKeyRangeId, BatchAsyncContainerExecutor.GetRetryPolicy(this.cosmosContainer, this.retryOptions)); + ItemBatchOperationContext context = new ItemBatchOperationContext(resolvedPartitionKeyRangeId, BatchAsyncContainerExecutor.GetRetryPolicy(this.cosmosContainer, operation.OperationType, this.retryOptions)); operation.AttachContext(context); streamer.Add(operation); return await context.OperationTask; @@ -133,10 +133,12 @@ internal virtual async Task ValidateOperationAsync( private static IDocumentClientRetryPolicy GetRetryPolicy( ContainerInternal containerInternal, + OperationType operationType, RetryOptions retryOptions) { return new BulkExecutionRetryPolicy( containerInternal, + operationType, new ResourceThrottleRetryPolicy( retryOptions.MaxRetryAttemptsOnThrottledRequests, retryOptions.MaxRetryWaitTimeInSeconds)); diff --git a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs index c83b480fbe..b84d6d293f 100644 --- a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs @@ -19,13 +19,16 @@ namespace Microsoft.Azure.Cosmos internal sealed class BulkExecutionRetryPolicy : IDocumentClientRetryPolicy { private readonly IDocumentClientRetryPolicy nextRetryPolicy; + private readonly OperationType operationType; private readonly ContainerInternal container; public BulkExecutionRetryPolicy( ContainerInternal container, + OperationType operationType, IDocumentClientRetryPolicy nextRetryPolicy) { this.container = container ?? throw new ArgumentNullException(nameof(container)); + this.operationType = operationType; this.nextRetryPolicy = nextRetryPolicy; } @@ -80,6 +83,8 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) this.nextRetryPolicy.OnBeforeSendRequest(request); } + private bool IsReadRequest => this.operationType == OperationType.Read; + private async Task ShouldRetryInternalAsync( HttpStatusCode? statusCode, SubStatusCodes? subStatusCode, @@ -105,7 +110,8 @@ private async Task ShouldRetryInternalAsync( // Batch API can return 413 which means the response is bigger than 4Mb. // Operations that exceed the 4Mb limit are returned as 413, while the operations within the 4Mb limit will be 200 - if (statusCode == HttpStatusCode.RequestEntityTooLarge) + if (this.IsReadRequest + && statusCode == HttpStatusCode.RequestEntityTooLarge) { return ShouldRetryResult.RetryAfter(TimeSpan.Zero); } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs index 830fffe1ed..5576c6f42a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncBatcherTests.cs @@ -503,10 +503,12 @@ public async Task RetrierGetsCalledOnSplit() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); ItemBatchOperation operation1 = this.CreateItemBatchOperation(); @@ -532,10 +534,12 @@ public async Task RetrierGetsCalledOnCompletingSplit() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); ItemBatchOperation operation1 = this.CreateItemBatchOperation(); @@ -561,10 +565,12 @@ public async Task RetrierGetsCalledOnCompletingPartitionMigration() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); ItemBatchOperation operation1 = this.CreateItemBatchOperation(); @@ -606,14 +612,16 @@ public async Task RetrierGetsCalledOnOverFlow() } [TestMethod] - public async Task RetrierGetsCalledOn413() + public async Task RetrierGetsCalledOn413_OnRead() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); ItemBatchOperation operation1 = this.CreateItemBatchOperation(); @@ -632,6 +640,35 @@ public async Task RetrierGetsCalledOn413() retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Once); } + [TestMethod] + public async Task RetrierGetsCalledOn413_OnWrite() + { + IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( + GetSplitEnabledContainer(), + OperationType.Create, + new ResourceThrottleRetryPolicy(1)); + + IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( + GetSplitEnabledContainer(), + OperationType.Create, + new ResourceThrottleRetryPolicy(1)); + + ItemBatchOperation operation1 = this.CreateItemBatchOperation(); + ItemBatchOperation operation2 = this.CreateItemBatchOperation(); + operation1.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy1)); + operation2.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy2)); + + Mock retryDelegate = new Mock(); + + BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(2, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413, retryDelegate.Object); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation1)); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); + await batchAsyncBatcher.DispatchAsync(metric); + retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny()), Times.Never); + retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny()), Times.Never); + retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Never); + } + private static ContainerInternal GetSplitEnabledContainer() { Mock container = new Mock(); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs index 96ae876ff3..dc218d6f61 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs @@ -96,6 +96,7 @@ public async Task ShouldRetry_WithPolicy_OnSuccess() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.OK); ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); @@ -109,6 +110,7 @@ public async Task ShouldRetry_WithPolicy_On429() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult((HttpStatusCode)StatusCodes.TooManyRequests); ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); @@ -118,10 +120,11 @@ public async Task ShouldRetry_WithPolicy_On429() } [TestMethod] - public async Task ShouldRetry_WithPolicy_On413() + public async Task ShouldRetry_WithPolicy_On413_OnRead() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); @@ -130,11 +133,26 @@ public async Task ShouldRetry_WithPolicy_On413() Assert.IsTrue(shouldRetryResult.ShouldRetry); } + [TestMethod] + public async Task ShouldRetry_WithPolicy_On413_OnWrite() + { + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( + Mock.Of(), + OperationType.Create, + new ResourceThrottleRetryPolicy(1)); + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); + operation.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy)); + ShouldRetryResult shouldRetryResult = await operation.Context.ShouldRetryAsync(result, default); + Assert.IsFalse(shouldRetryResult.ShouldRetry); + } + [TestMethod] public async Task ShouldRetry_WithPolicy_OnSplit() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.PartitionKeyRangeGone }; ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); @@ -148,6 +166,7 @@ public async Task ShouldRetry_WithPolicy_OnCompletingSplit() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingSplit }; ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); @@ -161,6 +180,7 @@ public async Task ShouldRetry_WithPolicy_OnCompletingPartitionMigration() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingPartitionMigration }; ItemBatchOperation operation = new ItemBatchOperation(OperationType.Create, 0, Cosmos.PartitionKey.Null); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs index 73b96a6b62..3850b749c4 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs @@ -22,6 +22,7 @@ public async Task NotRetryOnSuccess() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.OK); @@ -34,6 +35,7 @@ public async Task RetriesOn429() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult((HttpStatusCode)StatusCodes.TooManyRequests); @@ -42,10 +44,11 @@ public async Task RetriesOn429() } [TestMethod] - public async Task RetriesOn413() + public async Task RetriesOn413_OnRead() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); @@ -53,11 +56,25 @@ public async Task RetriesOn413() Assert.IsTrue(shouldRetryResult.ShouldRetry); } + [TestMethod] + public async Task RetriesOn413_OnWrite() + { + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( + Mock.Of(), + OperationType.Create, + new ResourceThrottleRetryPolicy(1)); + + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + ShouldRetryResult shouldRetryResult = await retryPolicy.ShouldRetryAsync(result.ToResponseMessage(), default); + Assert.IsFalse(shouldRetryResult.ShouldRetry); + } + [TestMethod] public async Task RetriesOnSplits() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.PartitionKeyRangeGone }; @@ -70,6 +87,7 @@ public async Task RetriesOnCompletingSplits() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingSplit }; @@ -82,6 +100,7 @@ public async Task RetriesOnCompletingPartitionMigrationSplits() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), + OperationType.Read, new ResourceThrottleRetryPolicy(1)); TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.CompletingPartitionMigration }; From 14c11e974d959afa065eb4a635525836d7c0b89b Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Tue, 19 Jan 2021 10:19:56 -0800 Subject: [PATCH 6/6] Adding limit --- .../src/BulkExecutionRetryPolicy.cs | 9 +++++++++ ...lkPartitionKeyRangeGoneRetryPolicyTests.cs | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs index b84d6d293f..9195415be5 100644 --- a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs @@ -18,9 +18,11 @@ namespace Microsoft.Azure.Cosmos /// internal sealed class BulkExecutionRetryPolicy : IDocumentClientRetryPolicy { + private const int MaxRetryOn410 = 10; private readonly IDocumentClientRetryPolicy nextRetryPolicy; private readonly OperationType operationType; private readonly ContainerInternal container; + private int retriesOn410 = 0; public BulkExecutionRetryPolicy( ContainerInternal container, @@ -92,6 +94,13 @@ private async Task ShouldRetryInternalAsync( { if (statusCode == HttpStatusCode.Gone) { + this.retriesOn410++; + + if (this.retriesOn410 > MaxRetryOn410) + { + return ShouldRetryResult.NoRetry(); + } + if (subStatusCode == SubStatusCodes.PartitionKeyRangeGone || subStatusCode == SubStatusCodes.CompletingSplit || subStatusCode == SubStatusCodes.CompletingPartitionMigration) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs index 3850b749c4..b86de0d195 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs @@ -82,6 +82,26 @@ public async Task RetriesOnSplits() Assert.IsTrue(shouldRetryResult.ShouldRetry); } + [TestMethod] + public async Task RetriesOnSplits_UpToMax() + { + IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( + GetSplitEnabledContainer(), + OperationType.Read, + new ResourceThrottleRetryPolicy(1)); + + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.Gone) { SubStatusCode = SubStatusCodes.PartitionKeyRangeGone }; + ShouldRetryResult shouldRetryResult; + for (int i = 0; i < 10; i++) + { + shouldRetryResult = await retryPolicy.ShouldRetryAsync(result.ToResponseMessage(), default); + Assert.IsTrue(shouldRetryResult.ShouldRetry); + } + + shouldRetryResult = await retryPolicy.ShouldRetryAsync(result.ToResponseMessage(), default); + Assert.IsFalse(shouldRetryResult.ShouldRetry); + } + [TestMethod] public async Task RetriesOnCompletingSplits() {