From e62f1639b3bdcf80e3378ed059a2ac5a6957d9be Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Fri, 1 Nov 2024 09:20:11 +0100 Subject: [PATCH 1/2] Implemented a way to share retry timeouts between multiple instances of the same backend. --- .../Backend/OAuthHelper/RetryAfterHelper.cs | 66 ++++++++++++++++++- .../Backend/OneDrive/MicrosoftGraphBackend.cs | 2 +- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs b/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs index f9ea955a8..ee1bde117 100644 --- a/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs +++ b/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs @@ -26,21 +26,70 @@ using System; using System.Collections.Generic; using System.Linq; using System.Net.Http.Headers; -using System.Text; using System.Threading; using System.Threading.Tasks; namespace Duplicati.Library { + /// + /// Helper class to manage the Retry-After header for a given URL. + /// public class RetryAfterHelper { + /// + /// The log tag for this class. + /// private static readonly string LOGTAG = Log.LogTagFromType(); + /// + /// Initializes a new instance of the class. + /// + private RetryAfterHelper() + { + } + // Whenever a response includes a Retry-After header, we'll update this timestamp with when we can next // send a request. And before sending any requests, we'll make sure to wait until at least this time. // Since this may be read and written by multiple threads, it is stored as a long and updated using Interlocked.Exchange. private long retryAfter = DateTimeOffset.MinValue.UtcTicks; + /// + /// The lock object to ensure thread safety for accessing the retry after helpers. + /// + private static object _lock = new object(); + /// + /// Lookup table that maps the URL to the RetryAfterHelper for that URL. + /// + private static Dictionary _retryAfterHelpers = new Dictionary(); + + /// + /// Backends generally do not keep any state, but the retry after header is stateful, + /// as it depends on the last request. This method obtains a helper object for the url, + /// so a small amount of state can be shared between instances. + /// + /// The URL to get the RetryAfterHelper for. + /// The RetryAfterHelper for the given URL. + public static RetryAfterHelper CreateOrGetRetryAfterHelper(string url) + { + lock (_lock) + { + // Remove any expired entries + foreach (var (k, h) in _retryAfterHelpers.ToArray()) + if (h.retryAfter < DateTimeOffset.UtcNow.UtcTicks) + _retryAfterHelpers.Remove(k); + + // Get the RetryAfterHelper for the given URL, or create a new one if it doesn't exist + if (!_retryAfterHelpers.TryGetValue(url, out var retryAfterHelper)) + _retryAfterHelpers[url] = retryAfterHelper = new RetryAfterHelper(); + + return retryAfterHelper; + } + } + + /// + /// Sets the Retry-After header value for the next request. + /// + /// The Retry-After header value to set. public void SetRetryAfter(RetryConditionHeaderValue retryAfter) { if (retryAfter != null) @@ -82,20 +131,29 @@ namespace Duplicati.Library } } + /// + /// Waits for the time specified in the Retry-After header before returning. + /// public void WaitForRetryAfter() { this.WaitForRetryAfterAsync(CancellationToken.None).Await(); } + /// + /// Waits for the time specified in the Retry-After header before continuing. + /// + /// The cancellation token to cancel the wait. + /// A task that completes when the wait is done. public async Task WaitForRetryAfterAsync(CancellationToken cancelToken) { TimeSpan delay = this.GetDelayTime(); if (delay > TimeSpan.Zero) { - Log.WriteProfilingMessage( + Log.WriteWarningMessage( LOGTAG, "RetryAfterWait", + null, "Waiting for {0} to respect Retry-After header", delay); @@ -103,6 +161,10 @@ namespace Duplicati.Library } } + /// + /// Gets the delay time until the next request can be made. + /// + /// The delay time until the next request can be made. private TimeSpan GetDelayTime() { // Make sure this is thread safe in case multiple calls are made concurrently to this backend diff --git a/Duplicati/Library/Backend/OneDrive/MicrosoftGraphBackend.cs b/Duplicati/Library/Backend/OneDrive/MicrosoftGraphBackend.cs index a454e2ed7..376d95a9b 100644 --- a/Duplicati/Library/Backend/OneDrive/MicrosoftGraphBackend.cs +++ b/Duplicati/Library/Backend/OneDrive/MicrosoftGraphBackend.cs @@ -186,7 +186,7 @@ namespace Duplicati.Library.Backend this.m_oAuthHelper.AutoAuthHeader = true; } - this.m_retryAfter = new RetryAfterHelper(); + this.m_retryAfter = RetryAfterHelper.CreateOrGetRetryAfterHelper(url); // Extract out the path to the backup root folder from the given URI. Since this can be an expensive operation, // we will cache the value using a lazy initializer. From 4ef0497d0e2dd2ece311c3f0200e30c2519d0df1 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Fri, 1 Nov 2024 09:24:28 +0100 Subject: [PATCH 2/2] Reverted the message severity, it is not actionable by the user. --- Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs b/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs index ee1bde117..20f6d000a 100644 --- a/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs +++ b/Duplicati/Library/Backend/OAuthHelper/RetryAfterHelper.cs @@ -150,10 +150,9 @@ namespace Duplicati.Library if (delay > TimeSpan.Zero) { - Log.WriteWarningMessage( + Log.WriteInformationMessage( LOGTAG, "RetryAfterWait", - null, "Waiting for {0} to respect Retry-After header", delay);