From f7de51a62dcef5480588f0e472f2edc3ea4489a8 Mon Sep 17 00:00:00 2001 From: Brendan Kowitz Date: Tue, 5 Mar 2019 09:36:14 -0800 Subject: [PATCH] Use Polly instead of EnterpriseLibrary (Updated) (#369) * Use Polly instead of EnterpriseLibrary #169 * Improves handling of cancellation tokens in SPs --- .../Features/Storage/ControlPlaneDataStore.cs | 10 +++++---- .../HardDelete/HardDeleteIdentityProvider.cs | 5 +++-- .../HardDelete/HardDeleteRole.cs | 5 +++-- .../Storage/DocumentClientInitializer.cs | 2 +- .../Features/Storage/RetryExceptionPolicy.cs | 9 +++----- .../Storage/RetryExceptionPolicyFactory.cs | 21 +++++++++---------- .../StoredProcedures/StoredProcedureBase.cs | 4 +++- .../Microsoft.Health.CosmosDb.csproj | 2 +- .../Features/Storage/FhirDataStore.cs | 10 +++++---- .../StoredProcedures/HardDelete/HardDelete.cs | 5 +++-- .../Upsert/UpsertWithHistory.cs | 5 +++-- 11 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/ControlPlaneDataStore.cs b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/ControlPlaneDataStore.cs index cc0d85f39f..3f108a41ab 100644 --- a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/ControlPlaneDataStore.cs +++ b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/ControlPlaneDataStore.cs @@ -280,20 +280,22 @@ private async Task DeleteSystemDocumentsByIdAsync(string id, string partition { case "IdentityProvider": response = await _retryExceptionPolicyFactory.CreateRetryPolicy().ExecuteAsync( - async () => await _hardDeleteIdentityProvider.Execute( + async ct => await _hardDeleteIdentityProvider.Execute( _documentClient.Value, _collectionUri, id, - eTag), + eTag, + ct), cancellationToken); break; case "Role": response = await _retryExceptionPolicyFactory.CreateRetryPolicy().ExecuteAsync( - async () => await _hardDeleteRole.Execute( + async ct => await _hardDeleteRole.Execute( _documentClient.Value, _collectionUri, id, - eTag), + eTag, + ct), cancellationToken); break; default: diff --git a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteIdentityProvider.cs b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteIdentityProvider.cs index b12399a682..e2ed77b09f 100644 --- a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteIdentityProvider.cs +++ b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteIdentityProvider.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using EnsureThat; using Microsoft.Azure.Documents; @@ -16,13 +17,13 @@ namespace Microsoft.Health.ControlPlane.CosmosDb.Features.Storage.StoredProcedur { internal class HardDeleteIdentityProvider : StoredProcedureBase, IControlPlaneStoredProcedure { - public async Task>> Execute(IDocumentClient client, Uri collection, string id, string eTag) + public async Task>> Execute(IDocumentClient client, Uri collection, string id, string eTag, CancellationToken cancellationToken) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(collection, nameof(collection)); EnsureArg.IsNotNullOrWhiteSpace(id, nameof(id)); - return await ExecuteStoredProc>(client, collection, CosmosIdentityProvider.IdentityProviderPartition, id, eTag); + return await ExecuteStoredProc>(client, collection, CosmosIdentityProvider.IdentityProviderPartition, cancellationToken, id, eTag); } } } diff --git a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteRole.cs b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteRole.cs index c0468d57b7..11363805b2 100644 --- a/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteRole.cs +++ b/src/Microsoft.Health.ControlPlane.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDeleteRole.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using EnsureThat; using Microsoft.Azure.Documents; @@ -16,13 +17,13 @@ namespace Microsoft.Health.ControlPlane.CosmosDb.Features.Storage.StoredProcedur { internal class HardDeleteRole : StoredProcedureBase, IControlPlaneStoredProcedure { - public async Task>> Execute(IDocumentClient client, Uri collection, string id, string eTag) + public async Task>> Execute(IDocumentClient client, Uri collection, string id, string eTag, CancellationToken cancellationToken) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(collection, nameof(collection)); EnsureArg.IsNotNull(id, nameof(id)); - return await ExecuteStoredProc>(client, collection, CosmosRole.RolePartition, id, eTag); + return await ExecuteStoredProc>(client, collection, CosmosRole.RolePartition, cancellationToken, id, eTag); } } } diff --git a/src/Microsoft.Health.CosmosDb/Features/Storage/DocumentClientInitializer.cs b/src/Microsoft.Health.CosmosDb/Features/Storage/DocumentClientInitializer.cs index b8b36984c8..a3a7cb045e 100644 --- a/src/Microsoft.Health.CosmosDb/Features/Storage/DocumentClientInitializer.cs +++ b/src/Microsoft.Health.CosmosDb/Features/Storage/DocumentClientInitializer.cs @@ -42,7 +42,7 @@ public IDocumentClient CreateDocumentClient(CosmosDataStoreConfiguration configu { ConnectionMode = configuration.ConnectionMode, ConnectionProtocol = configuration.ConnectionProtocol, - RetryOptions = new RetryOptions() + RetryOptions = new RetryOptions { MaxRetryAttemptsOnThrottledRequests = configuration.RetryOptions.MaxNumberOfRetries, MaxRetryWaitTimeInSeconds = configuration.RetryOptions.MaxWaitTimeInSeconds, diff --git a/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicy.cs b/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicy.cs index ed7de1049c..23e9a61bf8 100644 --- a/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicy.cs +++ b/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicy.cs @@ -3,14 +3,12 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- -using System; using System.Net; using Microsoft.Azure.Documents; -using Microsoft.Practices.EnterpriseLibrary.TransientFaultHandling; namespace Microsoft.Health.CosmosDb.Features.Storage { - internal class RetryExceptionPolicy : ITransientErrorDetectionStrategy + internal class RetryExceptionPolicy { /// /// Determines whether the specified exception represents a transient failure that can be compensated by a retry. @@ -19,13 +17,12 @@ internal class RetryExceptionPolicy : ITransientErrorDetectionStrategy /// /// true if the specified exception is considered as transient; otherwise, false. /// - public bool IsTransient(Exception ex) + public static bool IsTransient(DocumentClientException ex) { // Detects "449 Retry With" - The operation encountered a transient error. This only occurs on write operations. It is safe to retry the operation. // Detects "429 Too Many Request" - The collection has exceeded the provisioned throughput limit. Retry the request after the server specified retry after duration. // For more information see: https://docs.microsoft.com/en-us/rest/api/documentdb/http-status-codes-for-documentdb - if (ex is DocumentClientException dce - && (dce.StatusCode == (HttpStatusCode)449 || dce.StatusCode == (HttpStatusCode)429)) + if (ex.StatusCode == (HttpStatusCode)449 || ex.StatusCode == (HttpStatusCode)429) { return true; } diff --git a/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicyFactory.cs b/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicyFactory.cs index f7e09067f0..01596bdd3b 100644 --- a/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicyFactory.cs +++ b/src/Microsoft.Health.CosmosDb/Features/Storage/RetryExceptionPolicyFactory.cs @@ -5,35 +5,34 @@ using System; using EnsureThat; +using Microsoft.Azure.Documents; using Microsoft.Health.CosmosDb.Configs; -using Microsoft.Practices.EnterpriseLibrary.TransientFaultHandling; +using Polly; +using Polly.Retry; namespace Microsoft.Health.CosmosDb.Features.Storage { public class RetryExceptionPolicyFactory { private readonly int _maxNumberOfRetries; - private readonly TimeSpan _minWaitTime; - private readonly TimeSpan _maxWaitTime; public RetryExceptionPolicyFactory(CosmosDataStoreConfiguration configuration) { EnsureArg.IsNotNull(configuration, nameof(configuration)); _maxNumberOfRetries = configuration.RetryOptions.MaxNumberOfRetries; - _minWaitTime = TimeSpan.FromSeconds(Math.Min(RetryStrategy.DefaultMinBackoff.TotalSeconds, configuration.RetryOptions.MaxWaitTimeInSeconds)); - _maxWaitTime = TimeSpan.FromSeconds(configuration.RetryOptions.MaxWaitTimeInSeconds); } public RetryPolicy CreateRetryPolicy() { - var strategy = new ExponentialBackoff( - _maxNumberOfRetries, - _minWaitTime, - _maxWaitTime, - RetryStrategy.DefaultClientBackoff); + var policy = Policy + .Handle(RetryExceptionPolicy.IsTransient) + .WaitAndRetryAsync( + _maxNumberOfRetries, + retryAttempt => + TimeSpan.FromSeconds(Math.Pow(2, retryAttempt))); - return new RetryPolicy(strategy); + return policy; } } } diff --git a/src/Microsoft.Health.CosmosDb/Features/Storage/StoredProcedures/StoredProcedureBase.cs b/src/Microsoft.Health.CosmosDb/Features/Storage/StoredProcedures/StoredProcedureBase.cs index 15a2178c95..1267f2b54e 100644 --- a/src/Microsoft.Health.CosmosDb/Features/Storage/StoredProcedures/StoredProcedureBase.cs +++ b/src/Microsoft.Health.CosmosDb/Features/Storage/StoredProcedures/StoredProcedureBase.cs @@ -5,6 +5,7 @@ using System; using System.IO; +using System.Threading; using System.Threading.Tasks; using EnsureThat; using Microsoft.Azure.Documents; @@ -36,7 +37,7 @@ public StoredProcedure AsStoredProcedure() }; } - protected async Task> ExecuteStoredProc(IDocumentClient client, Uri collection, string partitionId, params object[] parameters) + protected async Task> ExecuteStoredProc(IDocumentClient client, Uri collection, string partitionId, CancellationToken cancellationToken, params object[] parameters) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(collection, nameof(collection)); @@ -50,6 +51,7 @@ protected async Task> ExecuteStoredProc(IDocumentC var results = await client.ExecuteStoredProcedureAsync( GetUri(collection), partitionKey, + cancellationToken, parameters); return results; diff --git a/src/Microsoft.Health.CosmosDb/Microsoft.Health.CosmosDb.csproj b/src/Microsoft.Health.CosmosDb/Microsoft.Health.CosmosDb.csproj index 093efeada6..3aaea3709f 100644 --- a/src/Microsoft.Health.CosmosDb/Microsoft.Health.CosmosDb.csproj +++ b/src/Microsoft.Health.CosmosDb/Microsoft.Health.CosmosDb.csproj @@ -11,12 +11,12 @@ - + diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/FhirDataStore.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/FhirDataStore.cs index a8b2c3a846..fc9a755b66 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/FhirDataStore.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/FhirDataStore.cs @@ -98,13 +98,14 @@ public async Task UpsertAsync( _logger.LogDebug($"Upserting {resource.ResourceTypeName}/{resource.ResourceId}, ETag: \"{weakETag?.VersionId}\", AllowCreate: {allowCreate}, KeepHistory: {keepHistory}"); UpsertWithHistoryModel response = await _retryExceptionPolicyFactory.CreateRetryPolicy().ExecuteAsync( - async () => await _upsertWithHistoryProc.Execute( + async ct => await _upsertWithHistoryProc.Execute( _documentClient, _collectionUri, cosmosWrapper, weakETag?.VersionId, allowCreate, - keepHistory), + keepHistory, + ct), cancellationToken); return new UpsertOutcome(response.Wrapper, response.OutcomeType); @@ -187,10 +188,11 @@ public async Task UpsertAsync( _logger.LogDebug($"Obliterating {key.ResourceType}/{key.Id}"); StoredProcedureResponse> response = await _retryExceptionPolicyFactory.CreateRetryPolicy().ExecuteAsync( - async () => await _hardDelete.Execute( + async ct => await _hardDelete.Execute( _documentClient, _collectionUri, - key), + key, + ct), cancellationToken); _logger.LogDebug($"Hard-deleted {response.Response.Count} documents, which consumed {response.RequestCharge} RUs. The list of hard-deleted documents: {string.Join(", ", response.Response)}."); diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDelete.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDelete.cs index 3c139c63f7..180a4f2eac 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDelete.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/HardDelete/HardDelete.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using EnsureThat; using Microsoft.Azure.Documents; @@ -16,13 +17,13 @@ namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage.StoredProcedures.HardD { internal class HardDelete : StoredProcedureBase, IFhirStoredProcedure { - public async Task>> Execute(IDocumentClient client, Uri collection, ResourceKey key) + public async Task>> Execute(IDocumentClient client, Uri collection, ResourceKey key, CancellationToken cancellationToken) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(collection, nameof(collection)); EnsureArg.IsNotNull(key, nameof(key)); - return await ExecuteStoredProc>(client, collection, key.ToPartitionKey(), key.ResourceType, key.Id); + return await ExecuteStoredProc>(client, collection, key.ToPartitionKey(), cancellationToken, key.ResourceType, key.Id); } } } diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/Upsert/UpsertWithHistory.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/Upsert/UpsertWithHistory.cs index 061d66d1c0..ea33cb1614 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/Upsert/UpsertWithHistory.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/StoredProcedures/Upsert/UpsertWithHistory.cs @@ -4,6 +4,7 @@ // ------------------------------------------------------------------------------------------------- using System; +using System.Threading; using System.Threading.Tasks; using EnsureThat; using Microsoft.Azure.Documents; @@ -14,14 +15,14 @@ namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage.StoredProcedures.Upser { internal class UpsertWithHistory : StoredProcedureBase, IFhirStoredProcedure { - public async Task Execute(IDocumentClient client, Uri collection, FhirCosmosResourceWrapper resource, string matchVersionId, bool allowCreate, bool keepHistory) + public async Task Execute(IDocumentClient client, Uri collection, FhirCosmosResourceWrapper resource, string matchVersionId, bool allowCreate, bool keepHistory, CancellationToken cancellationToken) { EnsureArg.IsNotNull(client, nameof(client)); EnsureArg.IsNotNull(collection, nameof(collection)); EnsureArg.IsNotNull(resource, nameof(resource)); StoredProcedureResponse results = - await ExecuteStoredProc(client, collection, resource.PartitionKey, resource, matchVersionId, allowCreate, keepHistory); + await ExecuteStoredProc(client, collection, resource.PartitionKey, cancellationToken, resource, matchVersionId, allowCreate, keepHistory); return results.Response; }