diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs index 0c72a43d6..545026cb1 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs +++ b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs @@ -21,8 +21,8 @@ * ********************************************************************* */ using FiftyOne.Pipeline.CloudRequestEngine.Data; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Facade; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery; +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using FiftyOne.Pipeline.Core.Data; using FiftyOne.Pipeline.Core.FlowElements; using FiftyOne.Pipeline.Engines.Data; @@ -34,7 +34,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; -using System.Net.Http; +using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -48,69 +48,69 @@ namespace FiftyOne.Pipeline.CloudRequestEngine.FlowElements "CA1724:Type names should not match namespaces", Justification = "This would be a breaking change so will be " + "addressed in a future version.")] - public class CloudRequestEngine : + public class CloudRequestEngine : AspectEngineBase, ICloudRequestEngine { - /// - /// Separator as a character array. + /// + /// Separator as a character array. /// private static readonly char[] EVIDENCE_SEPARATOR_CHAR_ARRAY = Core.Constants.EVIDENCE_SEPERATOR.ToCharArray(); private HttpClient _httpClient; - /// - /// Raw string properties that describe - /// server destinations and query parameters. + /// + /// Raw string properties that describe + /// server destinations and query parameters. /// - public struct EndpointsAndKeys - { + public struct EndpointsAndKeys + { /// /// The URL for the cloud endpoint that will take the supplied - /// evidence and return JSON formatted data. - /// - public string DataEndpoint; - - /// + /// evidence and return JSON formatted data. + /// + public string DataEndpoint; + + /// /// The resource key to use when making requests. /// A resource key encapsulates details such as any license keys, /// the properties that should be returned and the domains that /// requests are permitted from. A new resource key can be - /// generated for free at https://configure.51degrees.com - /// - public string ResourceKey; - + /// generated for free at https://configure.51degrees.com + /// + public string ResourceKey; + /// /// The license key to use when making requests. - /// This parameter is obsolete, use resourceKey instead. - /// - public string LicenseKey; - - /// + /// This parameter is obsolete, use resourceKey instead. + /// + public string LicenseKey; + + /// /// The URL for the cloud endpoint that will return meta-data /// on properties that will be populated when the data endpoint - /// is called using the given resource key. - /// - public string PropertiesEndpoint; - - /// + /// is called using the given resource key. + /// + public string PropertiesEndpoint; + + /// /// The URL for the cloud endpoint that will return meta-data /// on the evidence that will be used when the data endpoint - /// is called using the given resource key. - /// - public string EvidenceKeysEndpoint; - - /// + /// is called using the given resource key. + /// + public string EvidenceKeysEndpoint; + + /// /// The value to use for the Origin header when making requests - /// to cloud. - /// - public string CloudRequestOrigin; - - /// - /// Not currently used. + /// to cloud. + /// + public string CloudRequestOrigin; + + /// + /// Not currently used. /// - public IList RequestedProperties; + public IList RequestedProperties; } private readonly EndpointsAndKeys _endpointsAndKeys; @@ -172,8 +172,8 @@ public struct EndpointsAndKeys /// Thrown if a required parameter is null. /// public CloudRequestEngine( - ILogger> logger, - Func, + ILogger> logger, + Func, CloudRequestData> aspectDataFactory, HttpClient httpClient, string dataEndpoint, @@ -183,27 +183,27 @@ public CloudRequestEngine( string evidenceKeysEndpoint, int timeout, List requestedProperties, - string cloudRequestOrigin = null) - : this( - logger, - aspectDataFactory, - httpClient, - new EndpointsAndKeys - { - DataEndpoint = dataEndpoint, - ResourceKey = resourceKey, - LicenseKey = licenseKey, - PropertiesEndpoint = propertiesEndpoint, - EvidenceKeysEndpoint = evidenceKeysEndpoint, - CloudRequestOrigin = cloudRequestOrigin, - RequestedProperties = requestedProperties, - }, - timeout, - new SimpleFailHandler(new InstantRecoveryStrategy())) - { } - - - + string cloudRequestOrigin = null) + : this( + logger, + aspectDataFactory, + httpClient, + new EndpointsAndKeys + { + DataEndpoint = dataEndpoint, + ResourceKey = resourceKey, + LicenseKey = licenseKey, + PropertiesEndpoint = propertiesEndpoint, + EvidenceKeysEndpoint = evidenceKeysEndpoint, + CloudRequestOrigin = cloudRequestOrigin, + RequestedProperties = requestedProperties, + }, + timeout, + new SimpleFailHandler(new InstantRecoveryStrategy())) + { } + + + /// /// Default constructor. /// @@ -217,15 +217,15 @@ public CloudRequestEngine( /// /// The HttpClient instance to use when making requests /// - /// - /// Raw string properties that describe + /// + /// Raw string properties that describe /// server destinations and query parameters. /// /// /// The timeout for HTTP requests in seconds. /// - /// - /// Controls when to suspend requests + /// + /// Controls when to suspend requests /// due to recent query failures. /// You can pick /// , @@ -242,19 +242,19 @@ public CloudRequestEngine( /// Thrown if a required parameter is null. /// public CloudRequestEngine( - ILogger> logger, - Func, + ILogger> logger, + Func, CloudRequestData> aspectDataFactory, HttpClient httpClient, EndpointsAndKeys endpointsAndKeys, - int timeout, - IFailHandler failHandler) + int timeout, + IFailHandler failHandler) : base(logger, aspectDataFactory) { if (httpClient == null) throw new ArgumentNullException(nameof(httpClient)); - if ((_failHandler = failHandler) is null) - throw new ArgumentNullException(nameof(failHandler)); - + if ((_failHandler = failHandler) is null) + throw new ArgumentNullException(nameof(failHandler)); + _endpointsAndKeys = endpointsAndKeys; try @@ -284,18 +284,18 @@ public CloudRequestEngine( "", new List(), true) - }; - - // Start the tasks to get the evidence keys and the public - // properties from the cloud service. If the stop token is - // not provided then warn via logging. - - _lazyEvidenceKeyFilter - = new Lazy( - GetCloudEvidenceKeys, - LazyThreadSafetyMode.PublicationOnly); - _lazyPublicProperties - = new Lazy>( + }; + + // Start the tasks to get the evidence keys and the public + // properties from the cloud service. If the stop token is + // not provided then warn via logging. + + _lazyEvidenceKeyFilter + = new Lazy( + GetCloudEvidenceKeys, + LazyThreadSafetyMode.PublicationOnly); + _lazyPublicProperties + = new Lazy>( GetCloudProperties, LazyThreadSafetyMode.PublicationOnly); } @@ -326,28 +326,28 @@ public CloudRequestEngine( /// public override string ElementDataKey => "cloud-response"; - /// - /// Responsible for initializing IEvidenceKeyFilter - /// only once. + /// + /// Responsible for initializing IEvidenceKeyFilter + /// only once. /// - private Lazy _lazyEvidenceKeyFilter; - - /// - /// Responsible for initializing IReadOnlyDictionary{string, ProductMetaData} - /// only once. + private Lazy _lazyEvidenceKeyFilter; + + /// + /// Responsible for initializing IReadOnlyDictionary{string, ProductMetaData} + /// only once. /// - private Lazy> - _lazyPublicProperties; - + private Lazy> + _lazyPublicProperties; + /// /// A filter object that indicates the evidence keys that can be used /// by this engine. This will vary based on the supplied resource key /// so will be populated after a call to the cloud service as part of /// object initialization. - /// - /// - /// Thrown if there is an error from the cloud service or - /// there is no data in the response. + /// + /// + /// Thrown if there is an error from the cloud service or + /// there is no data in the response. /// /// /// @@ -356,30 +356,30 @@ private Lazy> /// public override IEvidenceKeyFilter EvidenceKeyFilter { - get - { - try - { - if (_lazyEvidenceKeyFilter.IsValueCreated) - { - return _lazyEvidenceKeyFilter.Value; - } - lock (_lazyEvidenceKeyFilter) - { - return _lazyEvidenceKeyFilter.Value; - } - } - catch (AggregateException ex) - { - Logger?.LogWarning( - "Could not fetch evidence key filter from '{0}'", - _endpointsAndKeys.EvidenceKeysEndpoint); - if (ex.InnerException is CloudRequestException cloudException) - { - throw ResurfaceCloudException(cloudException, ex); - } - throw; - } + get + { + try + { + if (_lazyEvidenceKeyFilter.IsValueCreated) + { + return _lazyEvidenceKeyFilter.Value; + } + lock (_lazyEvidenceKeyFilter) + { + return _lazyEvidenceKeyFilter.Value; + } + } + catch (AggregateException ex) + { + Logger?.LogWarning( + "Could not fetch evidence key filter from '{0}'", + _endpointsAndKeys.EvidenceKeysEndpoint); + if (ex.InnerException is CloudRequestException cloudException) + { + throw ResurfaceCloudException(cloudException, ex); + } + throw; + } } } @@ -387,10 +387,10 @@ public override IEvidenceKeyFilter EvidenceKeyFilter /// A collection of the properties that the cloud service can /// populate in the JSON response. /// Keyed on property name. - /// - /// - /// Thrown if there is an error from the cloud service or - /// there is no data in the response. + /// + /// + /// Thrown if there is an error from the cloud service or + /// there is no data in the response. /// /// /// @@ -398,30 +398,30 @@ public override IEvidenceKeyFilter EvidenceKeyFilter /// due to recent error. /// public IReadOnlyDictionary PublicProperties { - get - { - try - { - if (_lazyPublicProperties.IsValueCreated) - { - return _lazyPublicProperties.Value; - } - lock (_lazyPublicProperties) - { - return _lazyPublicProperties.Value; - } - } - catch (AggregateException ex) - { - Logger?.LogWarning( - "Could not fetch public properties from '{0}'", - _endpointsAndKeys.PropertiesEndpoint); - if (ex.InnerException is CloudRequestException cloudException) - { - throw ResurfaceCloudException(cloudException, ex); - } - throw; - } + get + { + try + { + if (_lazyPublicProperties.IsValueCreated) + { + return _lazyPublicProperties.Value; + } + lock (_lazyPublicProperties) + { + return _lazyPublicProperties.Value; + } + } + catch (AggregateException ex) + { + Logger?.LogWarning( + "Could not fetch public properties from '{0}'", + _endpointsAndKeys.PropertiesEndpoint); + if (ex.InnerException is CloudRequestException cloudException) + { + throw ResurfaceCloudException(cloudException, ex); + } + throw; + } } } @@ -432,10 +432,10 @@ public IReadOnlyDictionary PublicProperties { /// /// /// Thrown if a required parameter is null. - /// - /// - /// Thrown if there is an error from the cloud service or - /// there is no data in the response. + /// + /// + /// Thrown if there is an error from the cloud service or + /// there is no data in the response. /// /// /// @@ -446,19 +446,19 @@ protected override void ProcessEngine( IFlowData data, CloudRequestData aspectData) { - if (data == null) - { - throw new ArgumentNullException(nameof(data)); + if (data == null) + { + throw new ArgumentNullException(nameof(data)); } - if (aspectData == null) - { - throw new ArgumentNullException(nameof(aspectData)); + if (aspectData == null) + { + throw new ArgumentNullException(nameof(aspectData)); } aspectData.ProcessStarted = true; - string jsonResult = string.Empty; + string jsonResult = string.Empty; ThrowIfStillRecovering(); using (var content = GetContent(data)) @@ -478,79 +478,79 @@ protected override void ProcessEngine( } aspectData.JsonResponse = jsonResult; - } - - private void ThrowIfStillRecovering() - { - try - { - _failHandler.ThrowIfStillRecovering(); - } - catch (Exception ex) - { - throw new CloudRequestEngineTemporarilyUnavailableException( - "Sending requests to cloud server" - + " is temporarily restricted" - + " due to recent failures.", ex); - } - } - - private string AmendSendAndProcess( + } + + private void ThrowIfStillRecovering() + { + try + { + _failHandler.ThrowIfStillRecovering(); + } + catch (Exception ex) + { + throw new CloudRequestEngineTemporarilyUnavailableException( + "Sending requests to cloud server" + + " is temporarily restricted" + + " due to recent failures.", ex); + } + } + + private string AmendSendAndProcess( HttpRequestMessage request, - bool checkForErrorMessages) - { - try - { - using (var requestScope = _failHandler.MakeAttemptScope()) - { - try - { - return ProcessResponse( - AddCommonHeadersAndSend( - request), - checkForErrorMessages); - } - catch (Exception ex) - when (!(ex is CloudRequestEngineTemporarilyUnavailableException)) - { - requestScope.RecordFailure(ex); - throw; - } - } - } - catch (AggregateException ex) - when (ex.InnerException is CloudRequestException originalCloudException) - { - throw ResurfaceCloudException(originalCloudException, ex); - } - } - - private CloudRequestException ResurfaceCloudException( - CloudRequestException cloudException, - Exception newInnerException) - { - Dictionary responseHeaders; - if (cloudException.ResponseHeaders is IDictionary oldHeaders) - { - if (oldHeaders is Dictionary oldHeadersDic) - { - responseHeaders = oldHeadersDic; - } - else - { - responseHeaders = new Dictionary(oldHeaders); - } - } else - { - responseHeaders = null; - } - return new CloudRequestException( - cloudException.Message, - cloudException.HttpStatusCode, - responseHeaders, - newInnerException); - } - + bool checkForErrorMessages) + { + try + { + using (var requestScope = _failHandler.MakeAttemptScope()) + { + try + { + return ProcessResponse( + AddCommonHeadersAndSend( + request), + checkForErrorMessages); + } + catch (Exception ex) + when (!(ex is CloudRequestEngineTemporarilyUnavailableException)) + { + requestScope.RecordFailure(ex); + throw; + } + } + } + catch (AggregateException ex) + when (ex.InnerException is CloudRequestException originalCloudException) + { + throw ResurfaceCloudException(originalCloudException, ex); + } + } + + private CloudRequestException ResurfaceCloudException( + CloudRequestException cloudException, + Exception newInnerException) + { + Dictionary responseHeaders; + if (cloudException.ResponseHeaders is IDictionary oldHeaders) + { + if (oldHeaders is Dictionary oldHeadersDic) + { + responseHeaders = oldHeadersDic; + } + else + { + responseHeaders = new Dictionary(oldHeaders); + } + } else + { + responseHeaders = null; + } + return new CloudRequestException( + cloudException.Message, + cloudException.HttpStatusCode, + responseHeaders, + newInnerException); + } + /// /// Validate the JSON response from the cloud service. /// An exception will be throw if any type of error has @@ -575,26 +575,26 @@ private string ProcessResponse( var hasData = string.IsNullOrEmpty(jsonResult) == false; List messages = new List(); - Func> GetHeaders = () => + Func> GetHeaders = () => { - // Get the response headers. + // Get the response headers. return response.Headers.ToDictionary( kvp => kvp.Key, - kvp => string.Join(", ", kvp.Value)); + kvp => string.Join(", ", kvp.Value)); }; if (hasData && checkForErrorMessages) { JObject jObj; - try - { - jObj = JObject.Parse(jsonResult); + try + { + jObj = JObject.Parse(jsonResult); } - catch (JsonReaderException ex) - { - throw new CloudRequestException( - "Failed to parse server's response as JSON", - (int)response.StatusCode, GetHeaders(), ex); + catch (JsonReaderException ex) + { + throw new CloudRequestException( + "Failed to parse server's response as JSON", + (int)response.StatusCode, GetHeaders(), ex); } var hasErrors = jObj.ContainsKey("errors"); hasData = hasErrors ? @@ -653,8 +653,8 @@ private string ProcessResponse( Messages.ExceptionCloudErrorsMultiple, messages.Select(m => new CloudRequestException(m, (int)response.StatusCode, headers))); - throw new CloudRequestException( - Messages.ExceptionCloudErrorsMultiple, + throw new CloudRequestException( + Messages.ExceptionCloudErrorsMultiple, (int)response.StatusCode, GetHeaders(), aggregated); } else if (messages.Count == 1) @@ -694,7 +694,7 @@ private FormUrlEncodedContent GetContent(IFlowData data) var evidence = data.GetEvidence().AsDictionary(); - // Add evidence in reverse alphabetical order, excluding special + // Add evidence in reverse alphabetical order, excluding special // keys. AddQueryData(queryData, evidence, evidence .Where(e => @@ -766,7 +766,7 @@ private void AddQueryData( else { // Get the conflicting pieces of evidence and then log a - // warning, if the evidence prefix is not query. Otherwise + // warning, if the evidence prefix is not query. Otherwise // a warning is not needed as query evidence is expected // to overwrite any existing evidence with the same suffix. if (prefix.Equals(Core.Constants.EVIDENCE_QUERY_PREFIX, @@ -801,10 +801,10 @@ protected override void UnmanagedResourcesCleanup() /// /// /// The value to be saved into - /// - /// - /// Thrown if there is an error from the cloud service or - /// there is no data in the response. + /// + /// + /// Thrown if there is an error from the cloud service or + /// there is no data in the response. /// private IReadOnlyDictionary GetCloudProperties() { @@ -818,16 +818,16 @@ private IReadOnlyDictionary GetCloudProperties() using (var requestMessage = new HttpRequestMessage(HttpMethod.Get, url)) { - try - { - jsonResult = AmendSendAndProcess( - requestMessage, - checkForErrorMessages: true); + try + { + jsonResult = AmendSendAndProcess( + requestMessage, + checkForErrorMessages: true); } - catch (Exception ex) - { - Logger?.LogError(ErrorMessage(), ex); - throw; + catch (Exception ex) + { + Logger?.LogError(ErrorMessage(), ex); + throw; } } @@ -849,7 +849,7 @@ private IReadOnlyDictionary GetCloudProperties() /// /// /// The value to be saved into . - /// + /// private IEvidenceKeyFilter GetCloudEvidenceKeys() { ThrowIfStillRecovering(); @@ -861,18 +861,18 @@ private IEvidenceKeyFilter GetCloudEvidenceKeys() using (var requestMessage = new HttpRequestMessage(HttpMethod.Get, _endpointsAndKeys.EvidenceKeysEndpoint)) { - try - { - // Note - Don't check for error messages in the response - // as it is a flat JSON array. - jsonResult = AmendSendAndProcess( - requestMessage, - checkForErrorMessages: false); + try + { + // Note - Don't check for error messages in the response + // as it is a flat JSON array. + jsonResult = AmendSendAndProcess( + requestMessage, + checkForErrorMessages: false); } - catch (Exception ex) - { - Logger?.LogError(ErrorMessage(), ex); - throw; + catch (Exception ex) + { + Logger?.LogError(ErrorMessage(), ex); + throw; } } @@ -924,17 +924,17 @@ private HttpResponseMessage AddCommonHeadersAndSend( private HttpResponseMessage SendRequestAsync( HttpRequestMessage request) { - try - { - var task = Task.Run(() => _httpClient.SendAsync(request)); - task.Wait(); - return task.Result; + try + { + var task = Task.Run(() => _httpClient.SendAsync(request)); + task.Wait(); + return task.Result; } - catch (AggregateException httpException) - { - throw new CloudRequestException( - Messages.ExceptionCloudResponseFailure, - httpException); + catch (AggregateException httpException) + { + throw new CloudRequestException( + Messages.ExceptionCloudResponseFailure, + httpException); } } } diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngineBuilder.cs b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngineBuilder.cs index af052dc3e..2b0ea212c 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngineBuilder.cs +++ b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngineBuilder.cs @@ -21,8 +21,8 @@ * ********************************************************************* */ using FiftyOne.Pipeline.CloudRequestEngine.Data; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Facade; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery; +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using FiftyOne.Pipeline.Core.Attributes; using FiftyOne.Pipeline.Core.Exceptions; using FiftyOne.Pipeline.Core.FlowElements; @@ -63,6 +63,10 @@ public class CloudRequestEngineBuilder : private int _failuresToEnterRecovery = Constants.CLOUD_REQUEST_FAILURES_TO_ENTER_RECOVERY_DEFAULT; private int _failuresWindowSeconds = Constants.CLOUD_REQUEST_FAILURES_WINDOW_SECONDS_DEFAULT; private double _recoverySeconds = Constants.CLOUD_REQUEST_RECOVERY_SECONDS_DEFAULT; + private bool _useExponentialBackoff = false; + private double _exponentialBackoffInitialDelay = ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT; + private double _exponentialBackoffMaxDelay = ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT; + private double _exponentialBackoffMultiplier = ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT; #endregion @@ -249,6 +253,57 @@ public CloudRequestEngineBuilder SetRecoverySeconds(double recoverySeconds) return this; } + /// + /// Enable exponential backoff recovery strategy instead of simple recovery. + /// + /// True to use exponential backoff, false for simple recovery + /// This builder instance + [DefaultValue(false)] + public CloudRequestEngineBuilder SetUseExponentialBackoff(bool useExponentialBackoff) + { + _useExponentialBackoff = useExponentialBackoff; + return this; + } + + /// + /// Set the initial delay in seconds for exponential backoff recovery strategy. + /// Only used when exponential backoff is enabled. + /// + /// Initial delay in seconds + /// This builder instance + [DefaultValue(ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT)] + public CloudRequestEngineBuilder SetExponentialBackoffInitialDelay(double initialDelaySeconds) + { + _exponentialBackoffInitialDelay = initialDelaySeconds; + return this; + } + + /// + /// Set the maximum delay in seconds for exponential backoff recovery strategy. + /// Only used when exponential backoff is enabled. + /// + /// Maximum delay in seconds + /// This builder instance + [DefaultValue(ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT)] + public CloudRequestEngineBuilder SetExponentialBackoffMaxDelay(double maxDelaySeconds) + { + _exponentialBackoffMaxDelay = maxDelaySeconds; + return this; + } + + /// + /// Set the multiplier for exponential backoff recovery strategy. + /// Only used when exponential backoff is enabled. + /// + /// Exponential multiplier (typically 2.0 for doubling) + /// This builder instance + [DefaultValue(ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT)] + public CloudRequestEngineBuilder SetExponentialBackoffMultiplier(double multiplier) + { + _exponentialBackoffMultiplier = multiplier; + return this; + } + /// /// The value to set for the Origin header when making requests /// to the cloud service. @@ -314,6 +369,20 @@ private CloudRequestData CreateAspectData(IPipeline pipeline, (IAspectEngine)engine); } + /// + /// Create the appropriate recovery strategy based on current configuration. + /// + /// The configured recovery strategy + private IRecoveryStrategy CreateRecoveryStrategy() + { + return RecoveryStrategyFactory.Create( + _useExponentialBackoff, + _recoverySeconds, + _exponentialBackoffInitialDelay, + _exponentialBackoffMaxDelay, + _exponentialBackoffMultiplier); + } + /// /// Create a new engine using the current configuration. /// @@ -331,10 +400,7 @@ protected override CloudRequestEngine NewEngine(List properties) Messages.ExceptionResourceKeyNeeded); } - var failThrottlingStrategy - = (_recoverySeconds > 0) - ? new SimpleRecoveryStrategy(_recoverySeconds) - : (IRecoveryStrategy)new InstantRecoveryStrategy(); + var failThrottlingStrategy = CreateRecoveryStrategy(); return new CloudRequestEngine( _loggerFactory.CreateLogger(), diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/ExceptionCaching/CachedException.cs b/FiftyOne.Pipeline.Core/FailHandling/ExceptionCaching/CachedException.cs similarity index 96% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/ExceptionCaching/CachedException.cs rename to FiftyOne.Pipeline.Core/FailHandling/ExceptionCaching/CachedException.cs index d32bed564..5fcf6188b 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/ExceptionCaching/CachedException.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/ExceptionCaching/CachedException.cs @@ -24,7 +24,7 @@ using System.Collections.Generic; using System.Text; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching +namespace FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching { /// /// Links the exception and the timestamp. diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/IFailHandler.cs b/FiftyOne.Pipeline.Core/FailHandling/Facade/IFailHandler.cs similarity index 77% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/IFailHandler.cs rename to FiftyOne.Pipeline.Core/FailHandling/Facade/IFailHandler.cs index 92935c168..3ab3e81d9 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/IFailHandler.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Facade/IFailHandler.cs @@ -20,12 +20,13 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Scope; +using FiftyOne.Pipeline.Core.Exceptions; +using FiftyOne.Pipeline.Core.FailHandling.Scope; using System; using System.Collections.Generic; using System.Text; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Facade +namespace FiftyOne.Pipeline.Core.FailHandling.Facade { /// /// Tracks failures and throttles requests. @@ -36,10 +37,19 @@ public interface IFailHandler /// Throws if the strategy indicates that /// requests may not be sent now. /// - /// + /// /// void ThrowIfStillRecovering(); + /// + /// Checks if requests may be sent now without throwing exceptions. + /// Use this for non-critical operations that should silently skip when unavailable. + /// + /// + /// True if requests may be sent, false if still in recovery mode. + /// + bool IsAvailable(); + /// /// Lets a consumer to wrap an attempt in `using` scope /// to implicitly report success diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/SimpleFailHandler.cs b/FiftyOne.Pipeline.Core/FailHandling/Facade/SimpleFailHandler.cs similarity index 78% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/SimpleFailHandler.cs rename to FiftyOne.Pipeline.Core/FailHandling/Facade/SimpleFailHandler.cs index 56e31091b..3b413125d 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/SimpleFailHandler.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Facade/SimpleFailHandler.cs @@ -20,14 +20,15 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Scope; +using FiftyOne.Pipeline.Core.Exceptions; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; +using FiftyOne.Pipeline.Core.FailHandling.Scope; using System; using System.Collections.Generic; using System.Text; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Facade +namespace FiftyOne.Pipeline.Core.FailHandling.Facade { /// /// Tracks failures and throttles requests. @@ -52,18 +53,30 @@ public SimpleFailHandler(IRecoveryStrategy recoveryStrategy) /// Throws if the strategy indicates that /// requests may not be sent now. /// - /// + /// /// public void ThrowIfStillRecovering() { if (!_recoveryStrategy.MayTryNow(out var cachedException)) { - throw new Exception( + throw new PipelineTemporarilyUnavailableException( $"Recovered exception from {(DateTime.Now - cachedException.DateTime).TotalSeconds}s ago.", cachedException.Exception); } } + /// + /// Checks if requests may be sent now without throwing exceptions. + /// Use this for non-critical operations that should silently skip when unavailable. + /// + /// + /// True if requests may be sent, false if still in recovery mode. + /// + public bool IsAvailable() + { + return _recoveryStrategy.MayTryNow(out var _); + } + /// /// Lets a consumer to wrap an attempt in `using` scope /// to implicitly report success diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/WindowedFailHandler.cs b/FiftyOne.Pipeline.Core/FailHandling/Facade/WindowedFailHandler.cs similarity index 94% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/WindowedFailHandler.cs rename to FiftyOne.Pipeline.Core/FailHandling/Facade/WindowedFailHandler.cs index a74cc3333..595469d99 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Facade/WindowedFailHandler.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Facade/WindowedFailHandler.cs @@ -20,14 +20,15 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Scope; +using FiftyOne.Pipeline.Core.Exceptions; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; +using FiftyOne.Pipeline.Core.FailHandling.Scope; using System; using System.Collections.Generic; using System.Diagnostics; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Facade +namespace FiftyOne.Pipeline.Core.FailHandling.Facade { /// /// Tracks failures and throttles requests. @@ -149,7 +150,7 @@ public WindowedFailHandler( /// Throws if the strategy indicates that /// requests may not be sent now. /// - /// + /// /// public void ThrowIfStillRecovering() { @@ -201,7 +202,7 @@ public void ThrowIfStillRecovering() break; } } - throw new Exception( + throw new PipelineTemporarilyUnavailableException( $"Recovered exception from {(DateTime.Now - cachedException.DateTime).TotalSeconds}s ago.", cachedException.Exception); } @@ -235,6 +236,18 @@ public void ThrowIfStillRecovering() } } + /// + /// Checks if requests may be sent now without throwing exceptions. + /// Use this for non-critical operations that should silently skip when unavailable. + /// + /// + /// True if requests may be sent, false if still in recovery mode. + /// + public bool IsAvailable() + { + return _recoveryStrategy.MayTryNow(out var _); + } + /// /// Lets a consumer to wrap an attempt in `using` scope /// to implicitly report success diff --git a/FiftyOne.Pipeline.Core/FailHandling/Recovery/ExponentialBackoffRecoveryStrategy.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/ExponentialBackoffRecoveryStrategy.cs new file mode 100644 index 000000000..265a3da95 --- /dev/null +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/ExponentialBackoffRecoveryStrategy.cs @@ -0,0 +1,189 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2023 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; +using System; + +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery +{ + /// + /// Implements exponential backoff where delay doubles after each consecutive failure. + /// First failure: wait initialDelay seconds + /// Second failure: wait initialDelay * multiplier seconds + /// Third failure: wait initialDelay * multiplier^2 seconds + /// And so on, up to maxDelaySeconds. + /// + public class ExponentialBackoffRecoveryStrategy : IRecoveryStrategy + { + /// + /// Default initial delay in seconds for exponential backoff recovery. + /// + public const double INITIAL_DELAY_SECONDS_DEFAULT = 2.0; + + /// + /// Default maximum delay in seconds for exponential backoff recovery. + /// + public const double MAX_DELAY_SECONDS_DEFAULT = 300.0; + + /// + /// Default multiplier for exponential backoff recovery. + /// + public const double MULTIPLIER_DEFAULT = 2.0; + /// + /// Initial delay in seconds for the first failure. + /// + public readonly double InitialDelaySeconds; + + /// + /// Maximum delay in seconds to cap the exponential growth. + /// + public readonly double MaxDelaySeconds; + + /// + /// Multiplier for exponential backoff (typically 2.0 for doubling). + /// + public readonly double Multiplier; + + /// + /// Current delay in seconds based on consecutive failures. + /// + public double CurrentDelaySeconds { get; private set; } + + private CachedException _exception = null; + private DateTime _recoveryDateTime = DateTime.MinValue; + private int _consecutiveFailures = 0; + private readonly object _lock = new object(); + + /// + /// Constructor with default exponential backoff parameters. + /// + /// + /// Initial delay in seconds (default: INITIAL_DELAY_SECONDS_DEFAULT). + /// + /// + /// Maximum delay in seconds to cap growth (default: MAX_DELAY_SECONDS_DEFAULT). + /// + /// + /// Exponential multiplier (default: MULTIPLIER_DEFAULT for doubling). + /// + public ExponentialBackoffRecoveryStrategy( + double initialDelaySeconds = INITIAL_DELAY_SECONDS_DEFAULT, + double maxDelaySeconds = MAX_DELAY_SECONDS_DEFAULT, + double multiplier = MULTIPLIER_DEFAULT) + { + if (initialDelaySeconds <= 0) + throw new ArgumentException("Initial delay must be positive", nameof(initialDelaySeconds)); + if (maxDelaySeconds <= 0) + throw new ArgumentException("Max delay must be positive", nameof(maxDelaySeconds)); + if (multiplier <= 1.0) + throw new ArgumentException("Multiplier must be greater than 1.0", nameof(multiplier)); + + InitialDelaySeconds = initialDelaySeconds; + MaxDelaySeconds = maxDelaySeconds; + Multiplier = multiplier; + CurrentDelaySeconds = initialDelaySeconds; + } + + /// + /// Called when querying the server failed. + /// Calculates the next delay using exponential backoff. + /// + /// + /// Timestamped exception. + /// + public void RecordFailure(CachedException cachedException) + { + lock (_lock) + { + // Only increment failure count if this is a new failure, not a concurrent + // recording of the same failure event. We consider it a new failure if: + // 1. This is the first failure (_exception is null), or + // 2. The new failure occurred after the current recovery time would have ended + bool isNewFailure = _exception == null || + cachedException.DateTime >= _recoveryDateTime; + + if (isNewFailure) + { + _consecutiveFailures++; + } + + // Calculate new delay: initialDelay * multiplier^(failures-1) + // For failures=1: initialDelay * multiplier^0 = initialDelay + // For failures=2: initialDelay * multiplier^1 = initialDelay * multiplier + // For failures=3: initialDelay * multiplier^2, etc. + CurrentDelaySeconds = Math.Min( + InitialDelaySeconds * Math.Pow(Multiplier, _consecutiveFailures - 1), + MaxDelaySeconds); + + var newRecoveryTime = cachedException.DateTime.AddSeconds(CurrentDelaySeconds); + + _exception = cachedException; + _recoveryDateTime = newRecoveryTime; + } + } + + /// + /// Whether the new request may be sent already. + /// + /// + /// Timestamped exception that prevents new requests. + /// + /// true -- send, false -- skip + public bool MayTryNow(out CachedException cachedException) + { + DateTime recoveryDateTime; + CachedException lastCachedException; + + lock (_lock) + { + recoveryDateTime = _recoveryDateTime; + lastCachedException = _exception; + } + + if (recoveryDateTime < DateTime.Now) + { + cachedException = null; + return true; + } + else + { + cachedException = lastCachedException; + return false; + } + } + + /// + /// Called once the request succeeds (after recovery). + /// Resets consecutive failures and delay back to initial value. + /// + public void Reset() + { + lock (_lock) + { + _consecutiveFailures = 0; + CurrentDelaySeconds = InitialDelaySeconds; + _exception = null; + _recoveryDateTime = DateTime.MinValue; + } + } + } +} \ No newline at end of file diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/IRecoveryStrategy.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/IRecoveryStrategy.cs similarity index 93% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/IRecoveryStrategy.cs rename to FiftyOne.Pipeline.Core/FailHandling/Recovery/IRecoveryStrategy.cs index 056f59c22..41737aaf7 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/IRecoveryStrategy.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/IRecoveryStrategy.cs @@ -20,9 +20,9 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery { /// /// Controls when to suspend requests diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/InstantRecoveryStrategy.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/InstantRecoveryStrategy.cs similarity index 94% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/InstantRecoveryStrategy.cs rename to FiftyOne.Pipeline.Core/FailHandling/Recovery/InstantRecoveryStrategy.cs index 814fe9b8a..8eaac8842 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/InstantRecoveryStrategy.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/InstantRecoveryStrategy.cs @@ -20,9 +20,9 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery { /// /// Always allows to make new server call diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/NoRecoveryStrategy.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/NoRecoveryStrategy.cs similarity index 94% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/NoRecoveryStrategy.cs rename to FiftyOne.Pipeline.Core/FailHandling/Recovery/NoRecoveryStrategy.cs index 8f9788968..457293344 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/NoRecoveryStrategy.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/NoRecoveryStrategy.cs @@ -20,9 +20,9 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery { /// /// Drops all server calls after first failure. diff --git a/FiftyOne.Pipeline.Core/FailHandling/Recovery/RecoveryStrategyFactory.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/RecoveryStrategyFactory.cs new file mode 100644 index 000000000..7d18b1830 --- /dev/null +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/RecoveryStrategyFactory.cs @@ -0,0 +1,94 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2023 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery +{ + /// + /// Factory for creating recovery strategy instances. + /// + public static class RecoveryStrategyFactory + { + /// + /// Create an exponential backoff recovery strategy with the specified parameters. + /// + /// Initial delay in seconds + /// Maximum delay in seconds + /// Exponential multiplier + /// A new ExponentialBackoffRecoveryStrategy instance + public static IRecoveryStrategy CreateExponentialBackoff( + double initialDelaySeconds = ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT, + double maxDelaySeconds = ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT, + double multiplier = ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT) + { + return new ExponentialBackoffRecoveryStrategy(initialDelaySeconds, maxDelaySeconds, multiplier); + } + + /// + /// Create a simple recovery strategy with the specified delay. + /// + /// Recovery delay in seconds + /// A new SimpleRecoveryStrategy instance + public static IRecoveryStrategy CreateSimple(double recoverySeconds) + { + return new SimpleRecoveryStrategy(recoverySeconds); + } + + /// + /// Create an instant recovery strategy (no delay). + /// + /// A new InstantRecoveryStrategy instance + public static IRecoveryStrategy CreateInstant() + { + return new InstantRecoveryStrategy(); + } + + /// + /// Create the appropriate recovery strategy based on configuration parameters. + /// + /// Whether to use exponential backoff + /// Simple recovery delay (used if exponential backoff is false) + /// Initial delay for exponential backoff + /// Maximum delay for exponential backoff + /// Multiplier for exponential backoff + /// The appropriate recovery strategy instance + public static IRecoveryStrategy Create( + bool useExponentialBackoff = false, + double recoverySeconds = 60.0, + double initialDelaySeconds = ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT, + double maxDelaySeconds = ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT, + double multiplier = ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT) + { + if (useExponentialBackoff) + { + return CreateExponentialBackoff(initialDelaySeconds, maxDelaySeconds, multiplier); + } + else if (recoverySeconds > 0) + { + return CreateSimple(recoverySeconds); + } + else + { + return CreateInstant(); + } + } + } +} \ No newline at end of file diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/SimpleRecoveryStrategy.cs b/FiftyOne.Pipeline.Core/FailHandling/Recovery/SimpleRecoveryStrategy.cs similarity index 96% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/SimpleRecoveryStrategy.cs rename to FiftyOne.Pipeline.Core/FailHandling/Recovery/SimpleRecoveryStrategy.cs index ec1d98abf..de003b30d 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Recovery/SimpleRecoveryStrategy.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Recovery/SimpleRecoveryStrategy.cs @@ -20,10 +20,10 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; using System; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery +namespace FiftyOne.Pipeline.Core.FailHandling.Recovery { /// /// Disallows calling the server for diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/AttemptScope.cs b/FiftyOne.Pipeline.Core/FailHandling/Scope/AttemptScope.cs similarity index 96% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/AttemptScope.cs rename to FiftyOne.Pipeline.Core/FailHandling/Scope/AttemptScope.cs index 93e350b6d..96fe50a77 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/AttemptScope.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Scope/AttemptScope.cs @@ -20,12 +20,12 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; using System; using System.Collections.Generic; using System.Text; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Scope +namespace FiftyOne.Pipeline.Core.FailHandling.Scope { /// /// A scope within which an attempt will be made. diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/IAttemptScope.cs b/FiftyOne.Pipeline.Core/FailHandling/Scope/IAttemptScope.cs similarity index 96% rename from FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/IAttemptScope.cs rename to FiftyOne.Pipeline.Core/FailHandling/Scope/IAttemptScope.cs index 3044ffc85..b4b0545e6 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FailHandling/Scope/IAttemptScope.cs +++ b/FiftyOne.Pipeline.Core/FailHandling/Scope/IAttemptScope.cs @@ -24,7 +24,7 @@ using System.Collections.Generic; using System.Text; -namespace FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Scope +namespace FiftyOne.Pipeline.Core.FailHandling.Scope { /// /// A scope within which an attempt will be made. diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs index 11065b8ea..f0853ab6c 100644 --- a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs +++ b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs @@ -23,6 +23,8 @@ using FiftyOne.Caching; using FiftyOne.Pipeline.Core.Data; using FiftyOne.Pipeline.Core.Exceptions; +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using FiftyOne.Pipeline.Core.FlowElements; using FiftyOne.Pipeline.Engines.Configuration; using FiftyOne.Pipeline.Engines.FiftyOne.Data; @@ -66,6 +68,11 @@ public abstract class ShareUsageBase : /// protected HttpClient HttpClient => _httpClient; + /// + /// Fail handler for managing failed attempts to add data to the queue. + /// + private readonly IFailHandler _failHandler; + /// /// Inner class that is used to store details of data in memory /// prior to it being sent to 51Degrees. @@ -413,7 +420,8 @@ protected ShareUsageBase( ignoreDataEvidenceFilter, aspSessionCookieName, null, - false) + false, + null) { } @@ -504,7 +512,8 @@ protected ShareUsageBase( ignoreDataEvidenceFilter, aspSessionCookieName, tracker, - false) + false, + null) { } @@ -569,6 +578,10 @@ protected ShareUsageBase( /// the blockedHttpHeaders, includedQueryStringParameters and /// ignoreDataEvidenceFilter parameters will be ignored. /// + /// + /// The fail handler to use when failures occur while sending data. + /// If null, a default WindowedFailHandler with ExponentialBackoffRecoveryStrategy will be created. + /// /// /// Thrown if certain arguments are null. /// @@ -591,7 +604,8 @@ protected ShareUsageBase( List> ignoreDataEvidenceFilter, string aspSessionCookieName, ITracker tracker, - bool shareAllEvidence) + bool shareAllEvidence, + IFailHandler failHandler = null) : base(logger) { if (blockedHttpHeaders == null) @@ -610,6 +624,10 @@ protected ShareUsageBase( } _httpClient = httpClient; + _failHandler = failHandler ?? new WindowedFailHandler( + new ExponentialBackoffRecoveryStrategy(), + 1, + TimeSpan.FromSeconds(10)); EvidenceCollection = new BlockingCollection(maximumQueueSize); @@ -775,6 +793,13 @@ private static bool IsLocalHost(IPAddress address) /// private void ProcessData(IFlowData data) { + // Check if we're still in recovery period + // Use IsAvailable for non-critical operations that should silently skip + if (!_failHandler.IsAvailable()) + { + return; + } + if (_rng.NextDouble() <= _sharePercentage) { // Check if the tracker will allow sharing of this data @@ -795,8 +820,10 @@ private void ProcessData(IFlowData data) } else { + var exception = new InvalidOperationException("Failed to add data to usage sharing queue"); + // For queue failures, we simply disable sharing rather than using fail handler IsCanceled = true; - Logger.LogError(Messages.MessageShareUsageFailedToAddData); + Logger.LogWarning(Messages.MessageShareUsageFailedToAddData); } } diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilder.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilder.cs index 3d6c84778..47c570adb 100644 --- a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilder.cs +++ b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilder.cs @@ -20,7 +20,10 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using Microsoft.Extensions.Logging; +using System; using System.Net.Http; using System.Runtime.CompilerServices; @@ -83,6 +86,16 @@ public ShareUsageBuilder( /// public override ShareUsageElement Build() { + var recoveryStrategy = RecoveryStrategyFactory.CreateExponentialBackoff( + InitialRecoveryDelaySeconds, + MaxRecoveryDelaySeconds, + RecoveryMultiplier); + + var failHandler = new WindowedFailHandler( + recoveryStrategy, + FailuresToEnterRecovery, + TimeSpan.FromSeconds(FailuresWindowSeconds)); + return new ShareUsageElement( LoggerFactory.CreateLogger(), _httpClient, @@ -99,7 +112,8 @@ public override ShareUsageElement Build() IgnoreDataEvidenceFilter, AspSessionCookieName, null, - ShareAllEvidence); + ShareAllEvidence, + failHandler); } } } diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilderBase.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilderBase.cs index f8557c8d9..5b1edf6dd 100644 --- a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilderBase.cs +++ b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBuilderBase.cs @@ -21,6 +21,8 @@ * ********************************************************************* */ using FiftyOne.Pipeline.Core.Attributes; +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; @@ -140,6 +142,31 @@ private set /// protected bool ShareAllEvidence { get; private set; } = Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_EVIDENCE; + /// + /// Number of failures required to enter recovery mode. + /// + protected int FailuresToEnterRecovery { get; private set; } = 1; + + /// + /// Time window in seconds for tracking failures. + /// + protected int FailuresWindowSeconds { get; private set; } = 10; + + /// + /// Initial delay in seconds for exponential backoff recovery strategy. + /// + protected double InitialRecoveryDelaySeconds { get; private set; } = ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT; + + /// + /// Maximum delay in seconds for exponential backoff recovery strategy. + /// + protected double MaxRecoveryDelaySeconds { get; private set; } = ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT; + + /// + /// Multiplier for exponential backoff recovery strategy. + /// + protected double RecoveryMultiplier { get; private set; } = ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT; + /// /// Constructor /// @@ -509,6 +536,76 @@ public ShareUsageBuilderBase SetTrackSession(bool track) return this; } + /// + /// Set the number of failures required to enter recovery mode. + /// + /// + /// Number of failures within the window to trigger recovery. + /// + /// This builder instance. + [DefaultValue(1)] + public ShareUsageBuilderBase SetFailuresToEnterRecovery(int failuresToEnterRecovery) + { + FailuresToEnterRecovery = failuresToEnterRecovery; + return this; + } + + /// + /// Set the time window in seconds for tracking failures. + /// + /// + /// Time window in seconds for failure tracking. + /// + /// This builder instance. + [DefaultValue(10)] + public ShareUsageBuilderBase SetFailuresWindowSeconds(int failuresWindowSeconds) + { + FailuresWindowSeconds = failuresWindowSeconds; + return this; + } + + /// + /// Set the initial delay in seconds for exponential backoff recovery strategy. + /// + /// + /// Initial delay in seconds for the first failure. + /// + /// This builder instance. + [DefaultValue(ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT)] + public ShareUsageBuilderBase SetInitialRecoveryDelaySeconds(double initialDelaySeconds) + { + InitialRecoveryDelaySeconds = initialDelaySeconds; + return this; + } + + /// + /// Set the maximum delay in seconds for exponential backoff recovery strategy. + /// + /// + /// Maximum delay in seconds to cap the exponential growth. + /// + /// This builder instance. + [DefaultValue(ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT)] + public ShareUsageBuilderBase SetMaxRecoveryDelaySeconds(double maxDelaySeconds) + { + MaxRecoveryDelaySeconds = maxDelaySeconds; + return this; + } + + /// + /// Set the multiplier for exponential backoff recovery strategy. + /// + /// + /// Exponential multiplier (typically 2.0 for doubling). + /// + /// This builder instance. + [DefaultValue(ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT)] + public ShareUsageBuilderBase SetRecoveryMultiplier(double multiplier) + { + RecoveryMultiplier = multiplier; + return this; + } + /// /// Create the /// diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs index 8ef96ee2e..e8022dddd 100644 --- a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs +++ b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs @@ -20,6 +20,8 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ +using FiftyOne.Pipeline.Core.FailHandling.Facade; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using FiftyOne.Pipeline.Engines.Trackers; using Microsoft.Extensions.Logging; using System; @@ -62,7 +64,8 @@ internal ShareUsageElement( List> ignoreDataEvidenceFilter, string aspSessionCookieName, ITracker tracker, - bool shareAllEvidence) + bool shareAllEvidence, + IFailHandler failHandler = null) : base(logger, httpClient, sharePercentage, @@ -78,7 +81,8 @@ internal ShareUsageElement( ignoreDataEvidenceFilter, aspSessionCookieName, tracker, - shareAllEvidence) + shareAllEvidence, + failHandler) { } diff --git a/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/RecoveryStrategyTests.cs b/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/RecoveryStrategyTests.cs index b32ade054..575bac98e 100644 --- a/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/RecoveryStrategyTests.cs +++ b/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/RecoveryStrategyTests.cs @@ -21,10 +21,12 @@ * ********************************************************************* */ -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.ExceptionCaching; -using FiftyOne.Pipeline.CloudRequestEngine.FailHandling.Recovery; +using FiftyOne.Pipeline.Core.FailHandling.ExceptionCaching; +using FiftyOne.Pipeline.Core.FailHandling.Recovery; using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; using System.Threading; +using System.Threading.Tasks; namespace FiftyOne.Pipeline.CloudRequestEngine.Tests { @@ -153,5 +155,315 @@ IRecoveryStrategy strategy } #endregion + + #region ExponentialBackoffRecoveryStrategy + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldReturnTrue() + { + IRecoveryStrategy strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 1.0, + maxDelaySeconds: 60.0, + multiplier: 2.0); + + Assert.IsTrue(strategy.MayTryNow(out var cachedException), + $"{nameof(ExponentialBackoffRecoveryStrategy)}.{nameof(IRecoveryStrategy.MayTryNow)}" + + " should return true initially."); + Assert.IsNull(cachedException, + $"{nameof(cachedException)} should be null initially."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldReturnFalseAfterFailure() + { + IRecoveryStrategy strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 5.0, + maxDelaySeconds: 60.0, + multiplier: 2.0); + + var ex = new CachedException(new System.Exception("dummy exception")); + strategy.RecordFailure(ex); + + Assert.IsFalse(strategy.MayTryNow(out var ex2), + $"{nameof(ExponentialBackoffRecoveryStrategy)}.{nameof(IRecoveryStrategy.MayTryNow)}" + + " should return false after failure."); + Assert.AreSame(ex, ex2, + "The returned exception should be the same object."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldReturnTrueAfterRecovery() + { + IRecoveryStrategy strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 0.1, + maxDelaySeconds: 60.0, + multiplier: 2.0); + + var ex = new CachedException(new System.Exception("dummy exception")); + strategy.RecordFailure(ex); + + Assert.IsFalse(strategy.MayTryNow(out var ex2), + $"{nameof(ExponentialBackoffRecoveryStrategy)}.{nameof(IRecoveryStrategy.MayTryNow)}" + + " should return false immediately after failure."); + Assert.AreSame(ex, ex2, + "The returned exception should be the same object."); + + Thread.Sleep(millisecondsTimeout: 200); + + Assert.IsTrue(strategy.MayTryNow(out var ex3), + $"{nameof(ExponentialBackoffRecoveryStrategy)}.{nameof(IRecoveryStrategy.MayTryNow)}" + + " should return true after recovery."); + Assert.IsNull(ex3, + $"{nameof(ex3)} should be null after recovery."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldDoubleDelayOnConsecutiveFailures() + { + IRecoveryStrategy strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 0.1, + maxDelaySeconds: 10.0, + multiplier: 2.0); + + // First failure - should delay for 0.1 seconds + var ex1 = new CachedException(new System.Exception("failure 1")); + strategy.RecordFailure(ex1); + + Assert.IsFalse(strategy.MayTryNow(out _), + "Should be in recovery after first failure."); + + // Wait for first recovery + Thread.Sleep(millisecondsTimeout: 150); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should be available after first recovery period."); + + // Second consecutive failure - should delay for 0.2 seconds + var ex2 = new CachedException(new System.Exception("failure 2")); + strategy.RecordFailure(ex2); + + Assert.IsFalse(strategy.MayTryNow(out _), + "Should be in recovery after second failure."); + + // Should still be in recovery after first delay period + Thread.Sleep(millisecondsTimeout: 150); + Assert.IsFalse(strategy.MayTryNow(out _), + "Should still be in recovery after 0.15 seconds (delay should be 0.2s)."); + + // Should be available after full second delay period + Thread.Sleep(millisecondsTimeout: 100); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should be available after full second delay period."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldRespectMaxDelay() + { + IRecoveryStrategy strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 1.0, + maxDelaySeconds: 2.0, // Cap at 2 seconds + multiplier: 10.0); // Large multiplier to test capping + + // First failure - 1 second delay + var ex1 = new CachedException(new System.Exception("failure 1")); + strategy.RecordFailure(ex1); + Thread.Sleep(millisecondsTimeout: 1100); + Assert.IsTrue(strategy.MayTryNow(out _), "Should recover after 1 second."); + + // Second failure - should be capped at 2 seconds, not 10 seconds + var ex2 = new CachedException(new System.Exception("failure 2")); + strategy.RecordFailure(ex2); + Thread.Sleep(millisecondsTimeout: 2100); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should recover after 2 seconds (capped), not 10 seconds."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldUseConstants() + { + // Test that constants are properly set + Assert.AreEqual(2.0, ExponentialBackoffRecoveryStrategy.INITIAL_DELAY_SECONDS_DEFAULT); + Assert.AreEqual(300.0, ExponentialBackoffRecoveryStrategy.MAX_DELAY_SECONDS_DEFAULT); + Assert.AreEqual(2.0, ExponentialBackoffRecoveryStrategy.MULTIPLIER_DEFAULT); + + // Test default constructor uses constants + var strategy = new ExponentialBackoffRecoveryStrategy(); + + // Should work with default values + Assert.IsTrue(strategy.MayTryNow(out _), + "Default strategy should allow initial attempts."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldHandleConcurrentFailures() + { + var strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 0.5, + maxDelaySeconds: 10.0, + multiplier: 2.0); + + var ex1 = new CachedException(new System.Exception("failure 1")); + var ex2 = new CachedException(new System.Exception("failure 2")); + var ex3 = new CachedException(new System.Exception("failure 3")); + + // Record multiple failures concurrently + var tasks = new[] + { + Task.Run(() => strategy.RecordFailure(ex1)), + Task.Run(() => strategy.RecordFailure(ex2)), + Task.Run(() => strategy.RecordFailure(ex3)) + }; + + Task.WaitAll(tasks); + + // Should still be in recovery mode + Assert.IsFalse(strategy.MayTryNow(out _), + "Should be in recovery after concurrent failures."); + + // Should eventually recover + Thread.Sleep(millisecondsTimeout: 1000); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should eventually recover from concurrent failures."); + } + + [TestMethod] + public void ExponentialBackoffRecoveryStrategyShouldPreventRaceConditionSkipping() + { + // This test verifies the fix for the race condition where multiple threads + // calling RecordFailure simultaneously could cause unexpected skips of exponential stages + + var strategy = new ExponentialBackoffRecoveryStrategy( + initialDelaySeconds: 0.2, + maxDelaySeconds: 10.0, + multiplier: 2.0); + + // All threads get MayTryNow = true at the same time + Assert.IsTrue(strategy.MayTryNow(out _), "All threads should initially be allowed."); + + // Simulate the race condition scenario described in the PR: + // [00:10.50] (4x threads) MayTryNow returns true + // [00:11.03] (thread A) RecordFailure -- should be stage 1 (0.2s recovery) + // [00:11.05] (thread B) RecordFailure -- should NOT increment to stage 2 if it's the same failure event + // [00:11.06] (thread C) RecordFailure -- should NOT increment to stage 3 if it's the same failure event + // [00:11.08] (thread D) RecordFailure -- should NOT increment to stage 4 if it's the same failure event + + var ex1 = new CachedException(new System.Exception("failure 1")); + var ex2 = new CachedException(new System.Exception("failure 2")); + var ex3 = new CachedException(new System.Exception("failure 3")); + var ex4 = new CachedException(new System.Exception("failure 4")); + + // Record failures in rapid succession (simulating concurrent access) + // The fix should prevent unconditional increment of consecutive failures + strategy.RecordFailure(ex1); // Should set stage 1 + strategy.RecordFailure(ex2); // Should NOT increment (same failure window due to race condition fix) + strategy.RecordFailure(ex3); // Should NOT increment (same failure window due to race condition fix) + strategy.RecordFailure(ex4); // Should NOT increment (same failure window due to race condition fix) + + // Should be in recovery for stage 1 (0.2 seconds), NOT stage 4 (1.6 seconds) + Assert.IsFalse(strategy.MayTryNow(out _), + "Should be in recovery after recording failures."); + + // Should recover after stage 1 delay (~0.2s), not stage 4 delay (~1.6s) + Thread.Sleep(millisecondsTimeout: 300); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should recover after stage 1 delay, proving race condition was prevented."); + + // Now record a genuinely new failure after recovery window + Thread.Sleep(millisecondsTimeout: 50); + var ex5 = new CachedException(new System.Exception("failure 5")); + strategy.RecordFailure(ex5); + + // This should now be stage 2 (0.4 seconds) + Assert.IsFalse(strategy.MayTryNow(out _), + "Should be in recovery for stage 2."); + + // Should still be in recovery after stage 1 delay + Thread.Sleep(millisecondsTimeout: 250); + Assert.IsFalse(strategy.MayTryNow(out _), + "Should still be in recovery after 0.25s (stage 2 delay is 0.4s)."); + + // Should recover after stage 2 delay + Thread.Sleep(millisecondsTimeout: 200); + Assert.IsTrue(strategy.MayTryNow(out _), + "Should recover after stage 2 delay."); + } + + #endregion + + #region RecoveryStrategyFactory + + [TestMethod] + public void RecoveryStrategyFactoryCreateExponentialBackoffShouldWork() + { + var strategy = RecoveryStrategyFactory.CreateExponentialBackoff(1.0, 10.0, 2.0); + + Assert.IsNotNull(strategy, "Factory should create strategy."); + Assert.IsInstanceOfType(strategy, typeof(ExponentialBackoffRecoveryStrategy), + "Factory should create ExponentialBackoffRecoveryStrategy."); + + // Test it works + Assert.IsTrue(strategy.MayTryNow(out _), "Created strategy should work."); + } + + [TestMethod] + public void RecoveryStrategyFactoryCreateExponentialBackoffShouldUseDefaults() + { + var strategy = RecoveryStrategyFactory.CreateExponentialBackoff(); + + Assert.IsNotNull(strategy, "Factory should create strategy with defaults."); + Assert.IsTrue(strategy.MayTryNow(out _), "Created strategy should work with defaults."); + } + + [TestMethod] + public void RecoveryStrategyFactoryCreateSimpleShouldWork() + { + var strategy = RecoveryStrategyFactory.CreateSimple(1.0); + + Assert.IsNotNull(strategy, "Factory should create simple strategy."); + Assert.IsInstanceOfType(strategy, typeof(SimpleRecoveryStrategy), + "Factory should create SimpleRecoveryStrategy."); + } + + [TestMethod] + public void RecoveryStrategyFactoryCreateInstantShouldWork() + { + var strategy = RecoveryStrategyFactory.CreateInstant(); + + Assert.IsNotNull(strategy, "Factory should create instant strategy."); + Assert.IsInstanceOfType(strategy, typeof(InstantRecoveryStrategy), + "Factory should create InstantRecoveryStrategy."); + } + + [TestMethod] + public void RecoveryStrategyFactoryCreateShouldChooseCorrectStrategy() + { + // Test exponential backoff selection + var exponentialStrategy = RecoveryStrategyFactory.Create( + useExponentialBackoff: true, + recoverySeconds: 60.0, + initialDelaySeconds: 1.0, + maxDelaySeconds: 10.0, + multiplier: 2.0); + + Assert.IsInstanceOfType(exponentialStrategy, typeof(ExponentialBackoffRecoveryStrategy), + "Should create ExponentialBackoffRecoveryStrategy when useExponentialBackoff=true."); + + // Test simple recovery selection + var simpleStrategy = RecoveryStrategyFactory.Create( + useExponentialBackoff: false, + recoverySeconds: 5.0); + + Assert.IsInstanceOfType(simpleStrategy, typeof(SimpleRecoveryStrategy), + "Should create SimpleRecoveryStrategy when useExponentialBackoff=false and recoverySeconds>0."); + + // Test instant recovery selection + var instantStrategy = RecoveryStrategyFactory.Create( + useExponentialBackoff: false, + recoverySeconds: 0.0); + + Assert.IsInstanceOfType(instantStrategy, typeof(InstantRecoveryStrategy), + "Should create InstantRecoveryStrategy when useExponentialBackoff=false and recoverySeconds<=0."); + } + + #endregion } }