From 81ad9d6b27a16be72845e1fac0adfb7a25348987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Alberto=20Leiva=20Obando?= Date: Thu, 12 Sep 2024 19:01:02 -0600 Subject: [PATCH 1/5] [APIPUB-72] Develop solution for processing rate limited/left over messages when warned Add retry policy for rate limiting --- .../apiPublisherSettings.json | 3 +- .../ApiSourceResourceItemProvider.cs | 2 +- .../EdFiApiSourceTotalCountProvider.cs | 2 +- ...EdFiApiStreamResourcePageMessageHandler.cs | 2 +- ...hangeResourceKeyProcessingBlocksFactory.cs | 4 +-- .../DeleteResourceProcessingBlocksFactory.cs | 4 +-- .../PostResourceProcessingBlocksFactory.cs | 4 +-- .../Configuration/ApiPublisherSettings.cs | 4 ++- .../ConfigurationBuilderFactory.cs | 1 + .../Configuration/IRateLimiting.cs | 3 +- .../Configuration/PollyRateLimiter.cs | 29 ++++++++++++++++--- .../Processing/ChangeProcessor.cs | 18 ++++++++---- .../Processing/RateLimitingTests.cs | 1 + 13 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/EdFi.Tools.ApiPublisher.Cli/apiPublisherSettings.json b/src/EdFi.Tools.ApiPublisher.Cli/apiPublisherSettings.json index 4c86649..223ef28 100644 --- a/src/EdFi.Tools.ApiPublisher.Cli/apiPublisherSettings.json +++ b/src/EdFi.Tools.ApiPublisher.Cli/apiPublisherSettings.json @@ -13,8 +13,9 @@ "useChangeVersionPaging": false, "changeVersionPagingWindowSize": 25000, "enableRateLimit": false, - "rateLimitNumberExecutions": 100, + "rateLimitNumberExecutions": 30, "rateLimitTimeSeconds": 1, + "rateLimitMaxRetries": 10, "useReversePaging": false }, "authorizationFailureHandling": [ diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/DependencyResolution/ApiSourceResourceItemProvider.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/DependencyResolution/ApiSourceResourceItemProvider.cs index e95d4c3..52f0847 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/DependencyResolution/ApiSourceResourceItemProvider.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/DependencyResolution/ApiSourceResourceItemProvider.cs @@ -167,7 +167,7 @@ public ApiSourceResourceItemProvider(ISourceEdFiApiClientProvider sourceEdFiApiC } catch (RateLimitRejectedException) { - _logger.Warning($"{sourceEdFiApiClient.DataManagementApiSegment}{resourceItemUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{sourceEdFiApiClient.DataManagementApiSegment}{resourceItemUrl}: Rate limit exceeded. Please try again later."); return (false, null); } //---------------------------------------------------------------------------------------------- diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs index 8b1af62..9265fcb 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs @@ -137,7 +137,7 @@ await HandleResourceCountRequestErrorAsync(resourceUrl, errorHandlingBlock, apiR } catch (RateLimitRejectedException) { - _logger.Warning($"{edFiApiClient.DataManagementApiSegment}{resourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{edFiApiClient.DataManagementApiSegment}{resourceUrl}: Rate limit exceeded. Please try again later."); return (false, 0); } } diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/MessageHandlers/EdFiApiStreamResourcePageMessageHandler.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/MessageHandlers/EdFiApiStreamResourcePageMessageHandler.cs index 0751214..54e5301 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/MessageHandlers/EdFiApiStreamResourcePageMessageHandler.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/MessageHandlers/EdFiApiStreamResourcePageMessageHandler.cs @@ -199,7 +199,7 @@ public async Task> HandleStreamResourcePageAsyn } catch (RateLimitRejectedException) { - _logger.Warning($"{message.ResourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{message.ResourceUrl}: Rate limit exceeded. Please try again later."); } break; } diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/ChangeResourceKeyProcessingBlocksFactory.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/ChangeResourceKeyProcessingBlocksFactory.cs index 11e4988..c2220e1 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/ChangeResourceKeyProcessingBlocksFactory.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/ChangeResourceKeyProcessingBlocksFactory.cs @@ -235,7 +235,7 @@ private TransformManyBlock CreateG } catch (RateLimitRejectedException) { - _logger.Warning($"{message.ResourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{message.ResourceUrl}: Rate limit exceeded. Please try again later."); throw; } catch (Exception ex) @@ -367,7 +367,7 @@ private TransformManyBlock CreateChangeKeyBl } catch (RateLimitRejectedException) { - _logger.Warning($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); throw; } catch (Exception ex) diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/DeleteResourceProcessingBlocksFactory.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/DeleteResourceProcessingBlocksFactory.cs index 482862b..3f54170 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/DeleteResourceProcessingBlocksFactory.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/DeleteResourceProcessingBlocksFactory.cs @@ -187,7 +187,7 @@ private TransformManyBlock CreateG } catch (RateLimitRejectedException) { - _logger.Warning($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); throw; } catch (Exception ex) @@ -315,7 +315,7 @@ private TransformManyBlock CreateDeleteReso } catch (RateLimitRejectedException) { - _logger.Warning($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); + _logger.Fatal($"{msg.ResourceUrl}: Rate limit exceeded. Please try again later."); throw; } catch (Exception ex) diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs index 85dd4bd..d00a9a8 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs @@ -413,8 +413,8 @@ await HandlePostItemMessage( } catch (RateLimitRejectedException) { - _logger.Warning($"{postItemMessage.ResourceUrl}: Rate limit exceeded. Please try again later."); - throw; + _logger.Fatal($"{postItemMessage.ResourceUrl}: Rate limit exceeded. Please try again later."); + return Enumerable.Empty(); } catch (Exception ex) { diff --git a/src/EdFi.Tools.ApiPublisher.Core/Configuration/ApiPublisherSettings.cs b/src/EdFi.Tools.ApiPublisher.Core/Configuration/ApiPublisherSettings.cs index a388a74..594b5a9 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Configuration/ApiPublisherSettings.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Configuration/ApiPublisherSettings.cs @@ -91,10 +91,12 @@ public int MaxDegreeOfParallelismForPostResourceItem public bool EnableRateLimit { get; set; } = false; - public int RateLimitNumberExecutions { get; set; } = 100; + public int RateLimitNumberExecutions { get; set; } = 30; public double RateLimitTimeSeconds { get; set; } = 1; + public int RateLimitMaxRetries { get; set; } = 5; + public bool UseReversePaging { get; set; } = false; } } diff --git a/src/EdFi.Tools.ApiPublisher.Core/Configuration/ConfigurationBuilderFactory.cs b/src/EdFi.Tools.ApiPublisher.Core/Configuration/ConfigurationBuilderFactory.cs index 6016a62..b32ca5a 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Configuration/ConfigurationBuilderFactory.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Configuration/ConfigurationBuilderFactory.cs @@ -75,6 +75,7 @@ public IConfigurationBuilder Create(string[] commandLineArgs) ["--enableRateLimit"] = "Options:EnableRateLimit", ["--rateLimitNumberExecutions"] = "Options:RateLimitNumberExecutions", ["--rateLimitTimeSeconds"] = "Options:RateLimitTimeSeconds", + ["--rateLimitMaxRetries"] = "Options:RateLimitMaxRetries", ["--useReversePaging"] = "Options:UseReversePaging", diff --git a/src/EdFi.Tools.ApiPublisher.Core/Configuration/IRateLimiting.cs b/src/EdFi.Tools.ApiPublisher.Core/Configuration/IRateLimiting.cs index 50e54ae..809d856 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Configuration/IRateLimiting.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Configuration/IRateLimiting.cs @@ -2,6 +2,7 @@ // Licensed to the Ed-Fi Alliance under one or more agreements. // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +using Polly; using Polly.RateLimit; using System; using System.Threading.Tasks; @@ -10,6 +11,6 @@ namespace EdFi.Tools.ApiPublisher.Core.Configuration; public interface IRateLimiting { - AsyncRateLimitPolicy GetRateLimitingPolicy(); + IAsyncPolicy GetRateLimitingPolicy(); Task ExecuteAsync(Func> action); } diff --git a/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs b/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs index 7e4fee2..5aff510 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs @@ -8,12 +8,15 @@ using System.Threading.Tasks; using System.Net.Http; using System.Threading.RateLimiting; +using Serilog; namespace EdFi.Tools.ApiPublisher.Core.Configuration; public class PollyRateLimiter : IRateLimiting { - private readonly AsyncRateLimitPolicy _rateLimiter; + private readonly IAsyncPolicy _rateLimiter; + private readonly IAsyncPolicy _retryPolicyForRateLimit; + private readonly ILogger _logger = Log.ForContext(typeof(PollyRateLimiter)); public PollyRateLimiter(Options options) { @@ -21,15 +24,33 @@ public PollyRateLimiter(Options options) options.RateLimitNumberExecutions, TimeSpan.FromSeconds(options.RateLimitTimeSeconds), options.RateLimitNumberExecutions); + _retryPolicyForRateLimit = Policy + .Handle() + .WaitAndRetryAsync(options.RateLimitMaxRetries, // Number of retries + retryAttempt => TimeSpan.FromSeconds(options.RateLimitTimeSeconds), + (exception, timeSpan, retryCount, context) => + { + var delay = TimeSpan.FromSeconds(options.RateLimitTimeSeconds); + _logger.Warning($"Retry {retryCount} due to rate limit exceeded. Waiting {delay.TotalSeconds} seconds before next retry."); + } + ); } public async Task ExecuteAsync(Func> action) { - return await _rateLimiter.ExecuteAsync(action); + try + { + return await _rateLimiter.ExecuteAsync(action); + } + catch (RateLimitRejectedException) { + _logger.Fatal("Rate limit exceeded. Please try again later.dddd"); + throw; + } + } - public AsyncRateLimitPolicy GetRateLimitingPolicy() + public IAsyncPolicy GetRateLimitingPolicy() { - return _rateLimiter; + return Policy.WrapAsync(_retryPolicyForRateLimit, _rateLimiter); } } diff --git a/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs b/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs index c169a47..395ca53 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs @@ -15,6 +15,7 @@ using EdFi.Tools.ApiPublisher.Core.Processing.Messages; using EdFi.Tools.ApiPublisher.Core.Versioning; using Newtonsoft.Json; +using Polly.RateLimit; using Serilog; using Serilog.Events; using System; @@ -192,6 +193,11 @@ await _sourceIsolationApplicator.ApplySourceSnapshotIdentifierAsync(configuratio await UpdateChangeVersionAsync(configuration, changeWindow) .ConfigureAwait(false); } + catch (RateLimitRejectedException ex) + { + _logger.Fatal(ex.Message); + throw; + } catch (Exception ex) { _logger.Fatal($"An unhandled exception occurred during processing: {ex}"); @@ -886,12 +892,12 @@ private IDictionary GetKeyChangeDependencies(IDictionary(options); var methodToTest = new MockRateLimitingMethod(rateLimiter); From 72e950622010e8a946d7c8f812fa7a2841dc544a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Alberto=20Leiva=20Obando?= Date: Fri, 13 Sep 2024 09:26:39 -0600 Subject: [PATCH 2/5] Update API-Publisher-Configuration.md Add new option to documentation --- docs/API-Publisher-Configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/API-Publisher-Configuration.md b/docs/API-Publisher-Configuration.md index 87c4c24..4557ef8 100644 --- a/docs/API-Publisher-Configuration.md +++ b/docs/API-Publisher-Configuration.md @@ -29,6 +29,7 @@ Defines general behavior of the Ed-Fi API Publisher. | Options:EnableRateLimit
`--enableRateLimit` | Indicates whether or not to use rate limiting.
(_Default value: false_) | | Options:RateLimitNumberExecutions
`--rateLimitNumberExecutions` | Indicates the maximum number of executions allowed within the defined time window.
(_Default value: 100_) | | Options:RateLimitTimeSeconds
`--rateLimitTimeSeconds` | Indicates the the time span for the rate limit in seconds.
(_Default value: 1_) | +| Options:RateLimitMaxRetries
`--rateLimitMaxRetries` | Indicates the number of times the Ed-Fi API publisher will attempt to _resend_ a request, rejected by rate limiting, to the source or destination APIs before determining that the failure is permanent.
(_Default value: 10_) | ## API Connections From d6914b19efd888ffd1a49008f35e2479ada16941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Alberto=20Leiva=20Obando?= Date: Fri, 13 Sep 2024 09:35:29 -0600 Subject: [PATCH 3/5] Update PollyRateLimiter.cs Fix typo --- .../Configuration/PollyRateLimiter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs b/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs index 5aff510..ce202d2 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Configuration/PollyRateLimiter.cs @@ -43,7 +43,7 @@ public async Task ExecuteAsync(Func> action) return await _rateLimiter.ExecuteAsync(action); } catch (RateLimitRejectedException) { - _logger.Fatal("Rate limit exceeded. Please try again later.dddd"); + _logger.Fatal("Rate limit exceeded. Please try again later."); throw; } From 25da572221c6ee61b7f50c49511703f3988d9e86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Alberto=20Leiva=20Obando?= Date: Tue, 17 Sep 2024 09:17:56 -0600 Subject: [PATCH 4/5] Update documentation Update default value --- docs/API-Publisher-Configuration.md | 2 +- .../Processing/ChangeProcessor.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/API-Publisher-Configuration.md b/docs/API-Publisher-Configuration.md index 4557ef8..c481b37 100644 --- a/docs/API-Publisher-Configuration.md +++ b/docs/API-Publisher-Configuration.md @@ -27,7 +27,7 @@ Defines general behavior of the Ed-Fi API Publisher. | Options:UseChangeVersionPaging
`--useChangeVersionPaging` | Indicates whether or not to use change version paging.
(_Default value: false_) | | Options:ChangeVersionPagingWindowSize
`--changeVersionPagingWindowSize` | Indicates the change version paging window size.
(_Default value: 25000_) | | Options:EnableRateLimit
`--enableRateLimit` | Indicates whether or not to use rate limiting.
(_Default value: false_) | -| Options:RateLimitNumberExecutions
`--rateLimitNumberExecutions` | Indicates the maximum number of executions allowed within the defined time window.
(_Default value: 100_) | +| Options:RateLimitNumberExecutions
`--rateLimitNumberExecutions` | Indicates the maximum number of executions allowed within the defined time window.
(_Default value: 30_) | | Options:RateLimitTimeSeconds
`--rateLimitTimeSeconds` | Indicates the the time span for the rate limit in seconds.
(_Default value: 1_) | | Options:RateLimitMaxRetries
`--rateLimitMaxRetries` | Indicates the number of times the Ed-Fi API publisher will attempt to _resend_ a request, rejected by rate limiting, to the source or destination APIs before determining that the failure is permanent.
(_Default value: 10_) | diff --git a/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs b/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs index 395ca53..b3efc7d 100644 --- a/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs +++ b/src/EdFi.Tools.ApiPublisher.Core/Processing/ChangeProcessor.cs @@ -892,12 +892,12 @@ private IDictionary GetKeyChangeDependencies(IDictionary Date: Tue, 17 Sep 2024 09:30:43 -0600 Subject: [PATCH 5/5] Change return to remove warnings --- .../Source/Counting/EdFiApiSourceTotalCountProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs index 9265fcb..f85871c 100644 --- a/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs +++ b/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Source/Counting/EdFiApiSourceTotalCountProvider.cs @@ -74,8 +74,7 @@ public EdFiApiSourceTotalCountProvider(ISourceEdFiApiClientProvider sourceEdFiAp string requestUri = $"{edFiApiClient.DataManagementApiSegment}{resourceUrl}?offset=0&limit=1&totalCount=true{changeWindowQueryStringParameters}"; - - return RequestHelpers.SendGetRequestAsync(edFiApiClient, resourceUrl, requestUri, ct).Result; + return await RequestHelpers.SendGetRequestAsync(edFiApiClient, resourceUrl, requestUri, ct); }, new Context(), cancellationToken); string responseContent = null;