Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature suggestion: In-box DecorrelatedJitter #526

Closed
grant-d opened this issue Nov 5, 2018 · 4 comments
Closed

Feature suggestion: In-box DecorrelatedJitter #526

grant-d opened this issue Nov 5, 2018 · 4 comments

Comments

@grant-d
Copy link

grant-d commented Nov 5, 2018

If DecorrelatedJitter is a common-enough requirement, could we turn it into a first class Polly citizen.
I am happy to submit a PR. Otherwise, proposed solution is below.
This is a little different to what's shown in the wiki; it's an encapsulated and testable class.

[Edited to fix bug]

    // Background: https://www.awsarchitectureblog.com/2015/03/backoff.html
    public sealed class DecorrelatedJitter
    {
        [ThreadStatic]
        private static readonly Random s_random = new Random(); // Default ctor uses a random seed
        
        private readonly Random _random;
        public int RetryCount { get; }
        public TimeSpan MinDelay { get; }
        public TimeSpan MaxDelay { get; }

        public DecorrelatedJitter(int retryCount, TimeSpan minDelay, TimeSpan maxDelay, Random random = null)
        {
            if (retryCount < 0) throw new ArgumentOutOfRangeException(nameof(retryCount));
            if (minDelay < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(minDelay));
            if (maxDelay < minDelay) throw new ArgumentOutOfRangeException(nameof(maxDelay));
            
            _random = random ?? s_random; // Custom Random enables unit-testing
            RetryCount = retryCount;
            MinDelay = minDelay;
            MaxDelay = maxDelay;
        }

        public IEnumerable<TimeSpan> Generate()
        {
            var delays = new TimeSpan[RetryCount];

            double range = MaxDelay.TotalMilliseconds - MinDelay.TotalMilliseconds;  // Range

            for (var i = 0; i < delays.Length; i++)
            {
                var ms = range * _random.NextDouble(); // Ceiling
                ms += MinDelay.TotalMilliseconds; // Floor

                delays[i] = TimeSpan.FromMilliseconds(ms);
            }

            return delays;
        }
    }

Here's an example of how to call it. Note that the class is instantiated as a singleton (directly or via DI) and each execution uses the .Generate().

private static readonly DecorrelatedJitter s_jitterer = new DecorrelatedJitter(RetryCount, MinDelay, MaxDelay);

return Policy<IRestResponse<T>>
    .Handle<HttpRequestException>()
    .OrResult(resultPredicate)
    .WaitAndRetryAsync(s_jitterer.Generate()) // <-- note usage
    .ExecuteAsync(() => client.ExecuteTaskAsync<T>(request, cancellationToken));

This is a nice-to-have, certainly doesn't have to be in-box but it would remove the duplication in custom code.

@grant-d grant-d changed the title In-box DecorrelatedJitter Feature suggestion: In-box DecorrelatedJitter Nov 5, 2018
@reisenberger
Copy link
Member

Thanks @grant-d for taking the time to package this a step further than our quick sample.

Pending this while we have discussion on #530.

Note for later: If we do this, it would be make sense to (trivially) do some of the other IEnumerable patterns (like straight ExponentialBackoff) as convenience helpers too.

@grant-d
Copy link
Author

grant-d commented Nov 10, 2018

That's a good idea, I will wrap some of the other strategies too

@grant-d
Copy link
Author

grant-d commented Nov 11, 2018

I had a thought. If we add more 'sleep duration providers', then we could formalize the pattern as follows. The benefit is that we can then provide a host of duration providers, and the pattern protects the user from calling '.Create' at the wrong time.
It is also amenable to unit-testing and DI (thus, policy libraries)

    public interface ISleepDurationStrategy // naming TBD
    {
        int RetryCount { get; }

        // For when IEnumerable<TimeSpan> is needed
        IEnumerable<TimeSpan> Create(Context context = null);       

        // For when Func<int, Context, TimeSpan> is needed
        TimeSpan Next(int i, Context context = null);

        // etc
    }

    public sealed class DecorrelatedJitter : ISleepDurationStrategy
    {
        // Implementations per initial post above
    }

    public sealed class ExponentialBackoffProvider : ISleepDurationStrategy { }
    public sealed class ConstantBackoffProvider : ISleepDurationStrategy { }
    public sealed class MyCustomProvider : ISleepDurationStrategy { }
    public sealed class NoOpProvider : ISleepDurationStrategy { }

    // Additional overloads/extensions on Policy/Builder (only showing two)

    public static RetryPolicy<TResult> WaitAndRetryAsync<TResult>(
        this PolicyBuilder<TResult> policyBuilder,
        ISleepDurationStrategy sleepDurationStrategy) // Note param
    {
        var sleepDurations = sleepDurationStrategy.Generate(); // Note call to .Generate

        return policyBuilder.WaitAndRetryAsync(sleepDurations); // Existing IEnumerable<TimeSpan> overload
    }

    public static RetryPolicy<TResult> WaitAndRetry<TResult>(this PolicyBuilder<TResult> policyBuilder, int retryCount, ISleepDurationStrategy sleepDurationProvider)
    {
        Action<DelegateResult<TResult>, TimeSpan, int, Context> doNothing = (_, __, ___, ____) => { };

       return policyBuilder.WaitAndRetry(retryCount, sleepDurationProvider.Next, doNothing); // Existing Func<int, Context, TimeSpan> overload
    }

@grant-d
Copy link
Author

grant-d commented Nov 14, 2018

If we do this, it would be make sense to (trivially) do some of the other IEnumerable patterns (like straight ExponentialBackoff) as convenience helpers too

I have created a PR #536, please see latest code there

@reisenberger reisenberger added this to the v6.2.0 milestone Nov 18, 2018
@reisenberger reisenberger modified the milestones: v7.1.0, v7.2.0 Mar 10, 2019
@grant-d grant-d closed this as completed Apr 26, 2019
@reisenberger reisenberger removed this from the v7.1.1 or v7.2.0 milestone Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants