General Retry Strategy

Question

Let’s say we copy some file using retry strategy (it might be blocked, etc.):

class Processor
{
    public void CopyData() =>
        CopyData(Try.Slow);

    public void CopyData(Try loop) =>
        loop.Execute(() => 
            File.Copy(@"c\a.txt", @"c:\b.txt"));
}

What do you think about names chosen for the following library code identifiers? Would you name them differently?

public abstract class Try
{
    public static Try Repeat(params int[] delays) => new Repeat(delays);
    public static readonly Try Never = Repeat();
    public static readonly Try Once = Repeat(0);
    public static readonly Try Slow = Repeat(0, 500, 1500, 4500, 12000);
    public static readonly Try Fast = Repeat(0, 50, 150, 450, 1200);
    public abstract void Execute(Action action);
}

class Repeat : Try
{
    IReadOnlyList<int> Delays { get; }

    public Repeat(params int[] delays)
    {
        Delays = delays;
    }

    public override void Execute(Action action)
    {
        for(int i=0; i< Delays.Count; i++)
            try
            {
                Thread.Sleep(Delays[i]);
                action();
                return;
            }
            catch
            {
                if (i == Delays.Count - 1)
                    throw;
            }
    }
}

Answer 1

I think this would be more useful if the user could specify an interval and how many times he wants to retry like:

public static Try Repeat(int dalay, int count) 
    => new Repeat(Enumerable.Repeat(delay, count));

or if he could specify a count and the increment function:

public static Try Repeat(int count, Func<int, int> increment) 
    => new Repeat(Enumerable.Range(1, count + 1).Select(x => increment(x));

where the increment could be:

x => x * 20

---

I wouldn't provide such members as Fast or Slow because they are very subjective and what for you currently is slow might be in my application still too fast.

Never does not make any sense ;-) Why should I want to never try to execute something? I might as well not write the code at all if it shouldn't run :-P

---

One more thoght. How about specifying the retry strategy via a generic argument:

class SlowTry : Repeat
{
    public SlowTry() : base(0, 500, 1500, 4500, 12000) { }
}

the Try becomes this:

public abstract class Try
{
    ... stays the same

    public static void Execute<TStrategy>(Action action) where TStrategy : Try, new()
    {
        new TStrategy().Execute(action);
    }
}

use:

Try.Execute<SlowTry>(() => File.Copy(@"c\a.txt", @"c:\b.txt"));

This way the user can easier specify his strategy.

Answer 2

I think you need to implement a cancellation mechanism. In worst case scenario you will have to wait for almost 20 seconds for CopyData() to return with no way to interrupt this operation, except for maybe aborting the thread (which is hardly an option). This might not be as important for server-side application, but it is extremely important for clients, who should always have the ability to close the application or cancel pending operation. In general, I think using Thread.Sleep is almost always a poor design decision, and it can always be replaced with a wait handle of choice.

Edit: here is a simple example of cancellation, which uses tasks:

public LoopHandle Execute(Action action)
{
    return Execute(ct => action());
}

public LoopHandle Execute(Action<CancellationToken> action)
{
    //You might want to properly dispose CancellationTokenSource later on. 
    //Or you can let GC call its finalizer later. Same goes for tasks.
    var cts = new CancellationTokenSource();
    var token = cts.Token;
    var task = Task.Factory.StartNew(() =>
    {
        for (int i = 0; i < Delays.Count; i++)
        {
            try
            {
                if (token.WaitHandle.WaitOne(Delays[i])) return;
                action(token);
                return;
            }
            catch
            {
                if (i == Delays.Count - 1) throw;
            }
        }
    }, token);
    return new LoopHandle(task, cts);
}

public class LoopHandle
{
    public LoopHandle(Task task, CancellationTokenSource tokenSource)
    {
        _task = task;
        _tokenSource = tokenSource;
    }

    //or you can just expose the task itself
    public void Wait()
    {
        //this will throw if Execute threw
        _task.Wait();
    }

    public void Cancel()
    {
        _tokenSource.Cancel();
    }

    private readonly Task _task;
    private readonly CancellationTokenSource _tokenSource;
}

You can still run it synchronously by calling loop.Execute(...).Wait(). I think this approach is more universal, if the goal is to create general retry strategy, but you can do the same thing without tasks, if you already have the background thread which executes this long-running method.

Answer 3

I think it could be:

public abstract class Try
{
    public static Try After(params int[] delays) => new Repeat(delays);
    public static readonly Try Never = After();
    public static readonly Try Once = After(0);
    public static readonly Try Slow = After(0, 500, 1500, 4500, 12000);
    public static readonly Try Fast = After(0, 50, 150, 450, 1200);
    public static readonly Try InASecond = After(1000);
    public static readonly Try InAMinute = After(60000);
    public abstract void Execute(Action action);

    public Try FailFast() => FailFast(0);
    public Try FailFast(int timeout) => new Breaker(this, timeout);
}

where delays are provided by:

class Repeat : Try
{
    IReadOnlyList<int> Delays { get; }

    public Repeat(params int[] delays)
    {
        Delays = delays;
    }

    public override void Execute(Action action)
    {
        for(int i=0; i< Delays.Count; i++)
            try
            {
                Thread.Sleep(Delays[i]);
                action();
                return;
            }
            catch
            {
                if (i == Delays.Count - 1)
                    throw;
            }
    }
}

and circuit breaker pattern is implemented as:

class Breaker : Try
{
    Try Loop { get; }
    TimeSpan Timeout { get; }
    DateTime Ready { get; set; }

    public Breaker(Try loop, int timeout)
        : this(loop, TimeSpan.FromMilliseconds(timeout))
    {
    }

    public Breaker(Try loop, TimeSpan timeout)
    {
        Loop = loop;
        Timeout = timeout;
        Ready = DateTime.Now;
    }

    public override void Execute(Action action)
    {
        if (Ready > DateTime.Now)
            throw new OperationCanceledException();

        try
        {
            Loop.Execute(action);
        }
        catch
        {
            if (Timeout == TimeSpan.Zero)
                Ready = DateTime.MaxValue;
            else
                Ready = DateTime.Now + Timeout;

            throw;
        }   
    }
}