From a2bd029bfb3790aae5d900f7493037bb11242a91 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Wed, 14 Apr 2021 08:35:29 -0700 Subject: [PATCH 1/4] adding substatuscode --- Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs index 16d21e49d8..474cf6dd8d 100644 --- a/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs +++ b/Microsoft.Azure.Cosmos/src/BulkExecutionRetryPolicy.cs @@ -19,6 +19,7 @@ namespace Microsoft.Azure.Cosmos /// internal sealed class BulkExecutionRetryPolicy : IDocumentClientRetryPolicy { + private const int SubstatusCodeBatchResponseSizeExceeded = 3402; private const int MaxRetryOn410 = 10; private readonly IDocumentClientRetryPolicy nextRetryPolicy; private readonly OperationType operationType; @@ -86,8 +87,6 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) this.nextRetryPolicy.OnBeforeSendRequest(request); } - private bool IsReadRequest => this.operationType == OperationType.Read; - private async Task ShouldRetryInternalAsync( HttpStatusCode? statusCode, SubStatusCodes? subStatusCode, @@ -126,9 +125,9 @@ await partitionKeyRangeCache.TryGetOverlappingRangesAsync( } // 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 (this.IsReadRequest - && statusCode == HttpStatusCode.RequestEntityTooLarge) + // Operations that exceed the 4Mb limit are returned as 413/3402, while the operations within the 4Mb limit will be 200 + if (statusCode == HttpStatusCode.RequestEntityTooLarge + && (int)subStatusCode == SubstatusCodeBatchResponseSizeExceeded) { return ShouldRetryResult.RetryAfter(TimeSpan.Zero); } From 189a363a20ca61ac051c627c7b86dea60af8ed41 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Wed, 14 Apr 2021 10:31:41 -0700 Subject: [PATCH 2/4] tests --- .../Batch/BatchAsyncBatcherTests.cs | 86 +++++++++++++++++-- 1 file changed, 80 insertions(+), 6 deletions(-) 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 5de3e67fb7..beb67973c4 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 @@ -267,6 +267,62 @@ private readonly BatchAsyncBatcherExecuteDelegate ExecutorWith413 return new PartitionKeyRangeBatchExecutionResult(request.PartitionKeyRangeId, request.Operations, batchresponse); }; + private readonly BatchAsyncBatcherExecuteDelegate ExecutorWith413_3402 + = async (PartitionKeyRangeServerBatchRequest request, ITrace trace, 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) + { + SubStatusCode = (SubStatusCodes)3402, + 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, + trace: trace, + cancellationToken: cancellationToken); + + ResponseMessage responseMessage = new ResponseMessage((HttpStatusCode)207) + { + Content = responseContent + }; + + TransactionalBatchResponse batchresponse = await TransactionalBatchResponse.FromResponseMessageAsync( + responseMessage, + batchRequest, + MockCosmosUtil.Serializer, + true, + trace: trace, + CancellationToken.None); + + return new PartitionKeyRangeBatchExecutionResult(request.PartitionKeyRangeId, request.Operations, batchresponse); + }; + // The response will include all but 2 operation responses private readonly BatchAsyncBatcherExecuteDelegate ExecutorWithLessResponses = async (PartitionKeyRangeServerBatchRequest request, ITrace trace, CancellationToken cancellationToken) => @@ -610,7 +666,7 @@ public async Task RetrierGetsCalledOnOverFlow() } [TestMethod] - public async Task RetrierGetsCalledOn413_OnRead() + public async Task RetrierGetsCalledOn413_3402() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), @@ -622,48 +678,66 @@ public async Task RetrierGetsCalledOn413_OnRead() OperationType.Read, new ResourceThrottleRetryPolicy(1)); + IDocumentClientRetryPolicy retryPolicy3 = new BulkExecutionRetryPolicy( + GetSplitEnabledContainer(), + OperationType.Create, + new ResourceThrottleRetryPolicy(1)); + ItemBatchOperation operation1 = this.CreateItemBatchOperation(); ItemBatchOperation operation2 = this.CreateItemBatchOperation(); + ItemBatchOperation operation3 = this.CreateItemBatchOperation(); operation1.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy1)); operation2.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy2)); + operation3.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy3)); Mock retryDelegate = new Mock(); - BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(2, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413, retryDelegate.Object, BatchAsyncBatcherTests.MockClientContext()); + BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(3, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413_3402, retryDelegate.Object, BatchAsyncBatcherTests.MockClientContext()); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation1)); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation3)); await batchAsyncBatcher.DispatchAsync(metric); retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny(), It.IsAny()), Times.Never); retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Once); - retryDelegate.Verify(a => a(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Once); + retryDelegate.Verify(a => a(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } [TestMethod] - public async Task RetrierGetsCalledOn413_OnWrite() + public async Task RetrierGetsCalledOn413_NoSubstatus() { IDocumentClientRetryPolicy retryPolicy1 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), - OperationType.Create, + OperationType.Read, new ResourceThrottleRetryPolicy(1)); IDocumentClientRetryPolicy retryPolicy2 = new BulkExecutionRetryPolicy( + GetSplitEnabledContainer(), + OperationType.Read, + new ResourceThrottleRetryPolicy(1)); + + IDocumentClientRetryPolicy retryPolicy3 = new BulkExecutionRetryPolicy( GetSplitEnabledContainer(), OperationType.Create, new ResourceThrottleRetryPolicy(1)); ItemBatchOperation operation1 = this.CreateItemBatchOperation(); ItemBatchOperation operation2 = this.CreateItemBatchOperation(); + ItemBatchOperation operation3 = this.CreateItemBatchOperation(); operation1.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy1)); operation2.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy2)); + operation3.AttachContext(new ItemBatchOperationContext(string.Empty, retryPolicy3)); Mock retryDelegate = new Mock(); - BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(2, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413, retryDelegate.Object, BatchAsyncBatcherTests.MockClientContext()); + BatchAsyncBatcher batchAsyncBatcher = new BatchAsyncBatcher(3, 1000, MockCosmosUtil.Serializer, this.ExecutorWith413, retryDelegate.Object, BatchAsyncBatcherTests.MockClientContext()); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation1)); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); + Assert.IsTrue(batchAsyncBatcher.TryAdd(operation3)); await batchAsyncBatcher.DispatchAsync(metric); retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny(), It.IsAny()), Times.Never); retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Never); + retryDelegate.Verify(a => a(It.Is(o => o == operation3), It.IsAny(), It.IsAny()), Times.Never); retryDelegate.Verify(a => a(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } From 6178854797f02de92550ee9215f1f088cfc8382c Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Wed, 14 Apr 2021 11:00:25 -0700 Subject: [PATCH 3/4] more tests --- .../Batch/BatchAsyncOperationContextTests.cs | 6 +++--- .../BulkPartitionKeyRangeGoneRetryPolicyTests.cs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) 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 cc01ad6cce..94dcac4e31 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 @@ -121,13 +121,13 @@ public async Task ShouldRetry_WithPolicy_On429() } [TestMethod] - public async Task ShouldRetry_WithPolicy_On413_OnRead() + public async Task ShouldRetry_WithPolicy_On413_3402() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), OperationType.Read, new ResourceThrottleRetryPolicy(1)); - TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge) { SubStatusCode = (SubStatusCodes)3402 }; 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); @@ -135,7 +135,7 @@ public async Task ShouldRetry_WithPolicy_On413_OnRead() } [TestMethod] - public async Task ShouldRetry_WithPolicy_On413_OnWrite() + public async Task ShouldRetry_WithPolicy_On413_0() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), 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 a68e79b2a1..e1c8ad0cdf 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/BulkPartitionKeyRangeGoneRetryPolicyTests.cs @@ -45,20 +45,20 @@ public async Task RetriesOn429() } [TestMethod] - public async Task RetriesOn413_OnRead() + public async Task RetriesOn413_3204() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), OperationType.Read, new ResourceThrottleRetryPolicy(1)); - TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge); + TransactionalBatchOperationResult result = new TransactionalBatchOperationResult(HttpStatusCode.RequestEntityTooLarge) { SubStatusCode = (SubStatusCodes)3402 }; ShouldRetryResult shouldRetryResult = await retryPolicy.ShouldRetryAsync(result.ToResponseMessage(), default); Assert.IsTrue(shouldRetryResult.ShouldRetry); } [TestMethod] - public async Task RetriesOn413_OnWrite() + public async Task RetriesOn413_0() { IDocumentClientRetryPolicy retryPolicy = new BulkExecutionRetryPolicy( Mock.Of(), From 2a425ea2e7e84d2f659f494f2d85ef5fc5385c01 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Mon, 24 May 2021 11:17:12 -0700 Subject: [PATCH 4/4] merge with master --- .../Batch/BatchAsyncBatcherTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 bf872fe86f..3940417ef5 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 @@ -697,10 +697,10 @@ public async Task RetrierGetsCalledOn413_3402() Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation3)); await batchAsyncBatcher.DispatchAsync(metric); - retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny(), It.IsAny()), Times.Never); - retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Once); - retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Once); - retryDelegate.Verify(a => a(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + 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.Is(o => o == operation2), It.IsAny()), Times.Once); + retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Exactly(2)); } [TestMethod] @@ -735,10 +735,10 @@ public async Task RetrierGetsCalledOn413_NoSubstatus() Assert.IsTrue(batchAsyncBatcher.TryAdd(operation2)); Assert.IsTrue(batchAsyncBatcher.TryAdd(operation3)); await batchAsyncBatcher.DispatchAsync(metric); - retryDelegate.Verify(a => a(It.Is(o => o == operation1), It.IsAny(), It.IsAny()), Times.Never); - retryDelegate.Verify(a => a(It.Is(o => o == operation2), It.IsAny(), It.IsAny()), Times.Never); - retryDelegate.Verify(a => a(It.Is(o => o == operation3), It.IsAny(), It.IsAny()), Times.Never); - retryDelegate.Verify(a => a(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + 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.Is(o => o == operation3), It.IsAny()), Times.Never); + retryDelegate.Verify(a => a(It.IsAny(), It.IsAny()), Times.Never); } private static ContainerInternal GetSplitEnabledContainer()