Add StartupOptions for time-based retries on startup (#488)

* first draft time-based retries in startup

* set startupdelay from configureclientoptions, fix syntax and spacing

* allow configuring of startupclientoptions from options

* rough draft startupoptions

* fix draft

* clarify summary for startupoptions

* testing out retryoptions settings

* update with new structure, need logic to calculate retry options for specified timeout

* fix usage of startupconfigclientmanager, logic for retry time to use cancellationtoken

* in progress fixing connect tests

* fix connecttest, clientoptions in progress, need to fix maxretry logic

* fix tests

* extend startup timeout on test

* remove unused variable

* remove unused usings

* remove max retries change

* in progress working on fix for multiple clients with timeout

* in progress fix initializeasync

* in progress fixing failover logic

* progress

* fix with updates to logic for replicas

* fix tests and logic in initializeasync

* remove unused using

* in progress fixing timing of cts

* stuck on logic for srv dns changes, iasyncenumerable

* progress

* progress changing timeout to per store/replica, change options properties to internal

* fix logic to be per store/replica

* remove cancellationtoken from func

* progress on looping over clients

* fix iterating over clients, unit tests in progress

* add backoff to if statement

* working draft of design to retry replicas until timeout

* updates to fix unit tests, bugs

* fix variable naming hasNextClient

* add comment to catch in initializeasync

* prevent unnecessary delay when load finished

* remove custom exception, fix logic in executewithfailover

* move comment

* fix small mistakes, rename variables

* PR revisions

* restructure startup retry logic, fix tests

* Update src/Microsoft.Extensions.Configuration.AzureAppConfiguration/StartupOptions.cs

Co-authored-by: Jimmy Campbell <[email protected]>

* remove unnecessary param isstartup, fix text

* PR revisions, fix configurestartupoptions summary

* Apply suggestions from code review

Co-authored-by: Jimmy Campbell <[email protected]>

* combine if statemenets

* simplify logic for catching operationcanceledexception

Co-authored-by: Avani Gupta <[email protected]>

* move call to getclients

* PR revisions, make tests more specific

* add new methods for jitter and startup backoff calculation, fix smaller issues with namespace/names

* use new fixed + exponential backoff function

* remove debugging statemenets

* PR revisions

* PR revisions, remove new exponential backoff method and update old one to use jitter

* update jitter range logic

* fix isFailoverable, change jitter ratio to 0.25

* update IsFailoverable

---------

Co-authored-by: Jimmy Campbell <[email protected]>
Co-authored-by: Avani Gupta <[email protected]>

Author: 3 people

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs

@@ -116,6 +116,11 @@ internal IEnumerable<IKeyValueAdapter> Adapters
         /// </summary>
         internal FeatureFilterTelemetry FeatureFilterTelemetry { get; set; } = new FeatureFilterTelemetry();
 
+        /// <summary>
+        /// Options used to configure provider startup.
+        /// </summary>
+        internal StartupOptions Startup { get; set; } = new StartupOptions();
+
         /// <summary>
         /// Specify what key-values to include in the configuration provider.
         /// <see cref="Select"/> can be called multiple times to include multiple sets of key-values.
@@ -351,7 +356,7 @@ public AzureAppConfigurationOptions TrimKeyPrefix(string prefix)
         }
 
         /// <summary>
-        /// Configure the client used to communicate with Azure App Configuration.
+        /// Configure the client(s) used to communicate with Azure App Configuration.
         /// </summary>
         /// <param name="configure">A callback used to configure Azure App Configuration client options.</param>
         public AzureAppConfigurationOptions ConfigureClientOptions(Action<ConfigurationClientOptions> configure)
@@ -429,6 +434,16 @@ public AzureAppConfigurationOptions Map(Func<ConfigurationSetting, ValueTask<Con
             return this;
         }
 
+        /// <summary>
+        /// Configure the provider behavior when loading data from Azure App Configuration on startup.
+        /// </summary>
+        /// <param name="configure">A callback used to configure Azure App Configuration startup options.</param>
+        public AzureAppConfigurationOptions ConfigureStartupOptions(Action<StartupOptions> configure)
+        {
+            configure?.Invoke(Startup);
+            return this;
+        }
+
         private static ConfigurationClientOptions GetDefaultClientOptions()
         {
             var clientOptions = new ConfigurationClientOptions(ConfigurationClientOptions.ServiceVersion.V2022_11_01_Preview);

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs

@@ -88,9 +88,9 @@ public ILoggerFactory LoggerFactory
             }
         }
 
-        public AzureAppConfigurationProvider(IConfigurationClientManager clientManager, AzureAppConfigurationOptions options, bool optional)
+        public AzureAppConfigurationProvider(IConfigurationClientManager configClientManager, AzureAppConfigurationOptions options, bool optional)
         {
-            _configClientManager = clientManager ?? throw new ArgumentNullException(nameof(clientManager));
+            _configClientManager = configClientManager ?? throw new ArgumentNullException(nameof(configClientManager));
             _options = options ?? throw new ArgumentNullException(nameof(options));
             _optional = optional;
 
@@ -127,16 +127,13 @@ public override void Load()
         {
             var watch = Stopwatch.StartNew();
 
-            var loadStartTime = DateTimeOffset.UtcNow;
-
-            // Guaranteed to have atleast one available client since it is a application startup path.
-            IEnumerable<ConfigurationClient> availableClients = _configClientManager.GetAvailableClients(loadStartTime);
-
             try
             {
+                using var startupCancellationTokenSource = new CancellationTokenSource(_options.Startup.Timeout);
+
                 // Load() is invoked only once during application startup. We don't need to check for concurrent network
                 // operations here because there can't be any other startup or refresh operation in progress at this time.
-                InitializeAsync(_optional, availableClients, CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult();
+                LoadAsync(_optional, startupCancellationTokenSource.Token).ConfigureAwait(false).GetAwaiter().GetResult();
             }
             catch (ArgumentException)
             {
@@ -205,7 +202,8 @@ public async Task RefreshAsync(CancellationToken cancellationToken)
                         if (InitializationCacheExpires < utcNow)
                         {
                             InitializationCacheExpires = utcNow.Add(MinCacheExpirationInterval);
-                            await InitializeAsync(ignoreFailures: false, availableClients, cancellationToken).ConfigureAwait(false);
+
+                            await InitializeAsync(availableClients, cancellationToken).ConfigureAwait(false);
                         }
 
                         return;
@@ -548,42 +546,135 @@ private async Task<Dictionary<string, string>> PrepareData(Dictionary<string, Co
             return applicationData;
         }
 
-        private async Task InitializeAsync(bool ignoreFailures, IEnumerable<ConfigurationClient> availableClients, CancellationToken cancellationToken = default)
+        private async Task LoadAsync(bool ignoreFailures, CancellationToken cancellationToken)
         {
-            Dictionary<string, ConfigurationSetting> data = null;
-            Dictionary<KeyValueIdentifier, ConfigurationSetting> watchedSettings = null;
-            
+            var startupStopwatch = Stopwatch.StartNew();
+
+            int postFixedWindowAttempts = 0;
+
+            var startupExceptions = new List<Exception>();
+
             try
             {
-                await ExecuteWithFailOverPolicyAsync(
-                    availableClients,
-                    async (client) =>
+                while (true)
+                {
+                    IEnumerable<ConfigurationClient> clients = _configClientManager.GetAllClients();
+
+                    if (await TryInitializeAsync(clients, startupExceptions, cancellationToken).ConfigureAwait(false))
                     {
-                        data = await LoadSelectedKeyValues(
-                            client,
-                            cancellationToken)
-                            .ConfigureAwait(false);
-
-                        watchedSettings = await LoadKeyValuesRegisteredForRefresh(
-                            client,
-                            data,
-                            cancellationToken)
-                            .ConfigureAwait(false);
-
-                        watchedSettings = UpdateWatchedKeyValueCollections(watchedSettings, data);
-                    },
-                    cancellationToken)
-                    .ConfigureAwait(false);
+                        break;
+                    }
+
+                    TimeSpan delay;
+
+                    if (startupStopwatch.Elapsed.TryGetFixedBackoff(out TimeSpan backoff))
+                    {
+                        delay = backoff;
+                    }
+                    else
+                    {
+                        postFixedWindowAttempts++;
+
+                        delay = FailOverConstants.MinStartupBackoffDuration.CalculateBackoffDuration(
+                            FailOverConstants.MaxBackoffDuration,
+                            postFixedWindowAttempts);
+                    }
+
+                    try
+                    {
+                        await Task.Delay(delay, cancellationToken).ConfigureAwait(false);
+                    }
+                    catch (OperationCanceledException)
+                    {
+                        throw new TimeoutException(
+                            $"The provider timed out while attempting to load.",
+                            new AggregateException(startupExceptions));
+                    }
+                }
             }
             catch (Exception exception) when (
                 ignoreFailures &&
                 (exception is RequestFailedException ||
+                exception is KeyVaultReferenceException ||
+                exception is TimeoutException ||
                 exception is OperationCanceledException ||
                 exception is InvalidOperationException ||
                 ((exception as AggregateException)?.InnerExceptions?.Any(e =>
                     e is RequestFailedException ||
                     e is OperationCanceledException) ?? false)))
             { }
+        }
+
+        private async Task<bool> TryInitializeAsync(IEnumerable<ConfigurationClient> clients, List<Exception> startupExceptions, CancellationToken cancellationToken = default)
+        {
+            try
+            {
+                await InitializeAsync(clients, cancellationToken).ConfigureAwait(false);
+            }
+            catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+            {
+                return false;
+            }
+            catch (RequestFailedException exception)
+            {
+                if (IsFailOverable(exception))
+                {
+                    startupExceptions.Add(exception);
+
+                    return false;
+                }
+
+                throw;
+            }
+            catch (AggregateException exception)
+            {
+                if (exception.InnerExceptions?.Any(e => e is OperationCanceledException) ?? false)
+                {
+                    if (!cancellationToken.IsCancellationRequested)
+                    {
+                        startupExceptions.Add(exception);
+                    }
+
+                    return false;
+                }
+
+                if (IsFailOverable(exception))
+                {
+                    startupExceptions.Add(exception);
+
+                    return false;
+                }
+                
+                throw;
+            }
+
+            return true;
+        }
+
+        private async Task InitializeAsync(IEnumerable<ConfigurationClient> clients, CancellationToken cancellationToken = default)
+        {
+            Dictionary<string, ConfigurationSetting> data = null;
+            Dictionary<KeyValueIdentifier, ConfigurationSetting> watchedSettings = null;
+
+            await ExecuteWithFailOverPolicyAsync(
+                clients,
+                async (client) =>
+                {
+                    data = await LoadSelectedKeyValues(
+                        client,
+                        cancellationToken)
+                        .ConfigureAwait(false);
+
+                    watchedSettings = await LoadKeyValuesRegisteredForRefresh(
+                        client,
+                        data,
+                        cancellationToken)
+                        .ConfigureAwait(false);
+
+                    watchedSettings = UpdateWatchedKeyValueCollections(watchedSettings, data);
+                },
+                cancellationToken)
+                .ConfigureAwait(false);
 
             // Update the cache expiration time for all refresh registered settings and feature flags
             foreach (KeyValueWatcher changeWatcher in _options.ChangeWatchers.Concat(_options.MultiKeyWatchers))
@@ -599,17 +690,10 @@ e is RequestFailedException ||
                     adapter.InvalidateCache();
                 }
 
-                try
-                {
-                    Dictionary<string, ConfigurationSetting> mappedData = await MapConfigurationSettings(data).ConfigureAwait(false);
-                    SetData(await PrepareData(mappedData, cancellationToken).ConfigureAwait(false));
-                    _watchedSettings = watchedSettings;
-                    _mappedData = mappedData;
-                }
-                catch (KeyVaultReferenceException) when (ignoreFailures)
-                {
-                    // ignore failures
-                }
+                Dictionary<string, ConfigurationSetting> mappedData = await MapConfigurationSettings(data).ConfigureAwait(false);
+                SetData(await PrepareData(mappedData, cancellationToken).ConfigureAwait(false));
+                _watchedSettings = watchedSettings;
+                _mappedData = mappedData;
             }
         }
 
@@ -856,7 +940,10 @@ private void UpdateCacheExpirationTime(KeyValueWatcher changeWatcher)
             changeWatcher.CacheExpires = DateTimeOffset.UtcNow.Add(cacheExpirationTime);
         }
 
-        private async Task<T> ExecuteWithFailOverPolicyAsync<T>(IEnumerable<ConfigurationClient> clients, Func<ConfigurationClient, Task<T>> funcToExecute, CancellationToken cancellationToken = default)
+        private async Task<T> ExecuteWithFailOverPolicyAsync<T>(
+            IEnumerable<ConfigurationClient> clients,
+            Func<ConfigurationClient, Task<T>> funcToExecute,
+            CancellationToken cancellationToken = default)
         {
             using IEnumerator<ConfigurationClient> clientEnumerator = clients.GetEnumerator();
 
@@ -929,7 +1016,10 @@ private async Task<T> ExecuteWithFailOverPolicyAsync<T>(IEnumerable<Configuratio
             }
         }
 
-        private async Task ExecuteWithFailOverPolicyAsync(IEnumerable<ConfigurationClient> clients, Func<ConfigurationClient, Task> funcToExecute, CancellationToken cancellationToken = default)
+        private async Task ExecuteWithFailOverPolicyAsync(
+            IEnumerable<ConfigurationClient> clients,
+            Func<ConfigurationClient, Task> funcToExecute,
+            CancellationToken cancellationToken = default)
         {
             await ExecuteWithFailOverPolicyAsync<object>(clients, async (client) =>
             {
@@ -948,18 +1038,28 @@ private bool IsFailOverable(AggregateException ex)
 
         private bool IsFailOverable(RequestFailedException rfe)
         {
+            if (rfe.Status == HttpStatusCodes.TooManyRequests ||
+                rfe.Status == (int)HttpStatusCode.RequestTimeout ||
+                rfe.Status >= (int)HttpStatusCode.InternalServerError)
+            {
+                return true;
+            }
+
+            Exception innerException;
 
-            // The InnerException could be SocketException or WebException when endpoint is invalid and IOException if it is network issue.
-            if (rfe.InnerException != null && rfe.InnerException is HttpRequestException hre && hre.InnerException != null)
+            if (rfe.InnerException is HttpRequestException hre)
+            {
+                innerException = hre.InnerException;
+            }
+            else
             {
-                return hre.InnerException is WebException ||
-                       hre.InnerException is SocketException ||
-                       hre.InnerException is IOException;
+                innerException = rfe.InnerException;
             }
 
-            return rfe.Status == HttpStatusCodes.TooManyRequests ||
-                   rfe.Status == (int)HttpStatusCode.RequestTimeout ||
-                   rfe.Status >= (int)HttpStatusCode.InternalServerError;
+            // The InnerException could be SocketException or WebException when an endpoint is invalid and IOException if it's a network issue.
+            return innerException is WebException ||
+                   innerException is SocketException ||
+                   innerException is IOException;
         }
 
         private async Task<Dictionary<string, ConfigurationSetting>> MapConfigurationSettings(Dictionary<string, ConfigurationSetting> data)

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationClientManager.cs

@@ -4,12 +4,10 @@
 
 using Azure.Core;
 using Azure.Data.AppConfiguration;
-using Microsoft.Extensions.Configuration.AzureAppConfiguration.Constants;
 using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Net;
 
 namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
 {
@@ -62,6 +60,11 @@ public IEnumerable<ConfigurationClient> GetAvailableClients(DateTimeOffset time)
             return _clients.Where(client => client.BackoffEndTime <= time).Select(c => c.Client).ToList();
         }
 
+        public IEnumerable<ConfigurationClient> GetAllClients()
+        {
+            return _clients.Select(c => c.Client).ToList();
+        }
+
         public void UpdateClientStatus(ConfigurationClient client, bool successful)
         {
             if (client == null)
@@ -96,7 +99,7 @@ public bool UpdateSyncToken(Uri endpoint, string syncToken)
                 throw new ArgumentNullException(nameof(syncToken));
             }
 
-            ConfigurationClientWrapper clientWrapper = this._clients.SingleOrDefault(c => new EndpointComparer().Equals(c.Endpoint, endpoint));
+            ConfigurationClientWrapper clientWrapper = _clients.SingleOrDefault(c => new EndpointComparer().Equals(c.Endpoint, endpoint));
 
             if (clientWrapper != null)
             {

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/FailOverConstants.cs

@@ -4,12 +4,15 @@
 
 using System;
 
-namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.Constants
+namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
 {
     internal class FailOverConstants
     {
         // Timeouts to retry requests to config stores and their replicas after failure.
+
         public static readonly TimeSpan MinBackoffDuration = TimeSpan.FromSeconds(30);
         public static readonly TimeSpan MaxBackoffDuration = TimeSpan.FromMinutes(10);
+        // Minimum backoff duration for retries that occur after the fixed backoff window during startup.
+        public static readonly TimeSpan MinStartupBackoffDuration = TimeSpan.FromSeconds(30);
     }
 }