Additional ExecutionStrategy extension point: OnOperationRetrying(Async)

[paulirwin]

In ExecutionStrategy, there is a convenient template method OnRetry that will be called after an operation failure. However, there are a few limitations to this extension point:

1. It is called before the delay, so you cannot do something after the delay (i.e. allowing the database time to recover)
2. Any exceptions thrown by OnRetry are not caught and handled via ShouldRetryOn like operation exceptions are
3. It does not have an async version

I'd like to propose an additional extension point, which for the sake of this issue I'll call OnOperationRetrying (feel free to rename). My primary use case is to be able to execute some SQL queries before the operation in the case of a retry, but I could imagine other use cases such as logging or consistency validation.

It would be a similar template-method signature to OnRetry but with an additional async version:

protected virtual void OnOperationRetrying() 
{
}

protected virtual Task OnOperationRetryingAsync(CancellationToken cancellationToken)
{
    return Task.CompletedTask;
}

This would be called from ExecuteImplementation and ExecuteImplementationAsync inside the try block, immediately before the call to operation. For example:

        private TResult ExecuteImplementation<TState, TResult>(
            Func<DbContext, TState, TResult> operation,
            Func<DbContext, TState, ExecutionResult<TResult>> verifySucceeded,
            TState state)
        {
            bool isRetrying = false; // This is new
            while (true)
            {
                TimeSpan? delay;
                try
                {
                    Suspended = true;
                    if (isRetrying) {
                        OnOperationRetrying(); // new extension point
                    }
                    var result = operation(Dependencies.CurrentDbContext.Context, state);
                    Suspended = false;
                    return result;
                }
                catch (Exception ex)
                {
                    ... // existing catch code unmodified

                    OnRetry(); // this will still exist
                    isRetrying = true; // this is new
                }

                using (var waitEvent = new ManualResetEventSlim(false))
                {
                    waitEvent.WaitHandle.WaitOne(delay.Value);
                }
            }
        }

And of course there would be similar logic in ExecuteImplementationAsync just using await OnOperationRetryingAsync() instead.

This addresses all 3 of the limitations above:

1. It is called right before the next operation, so it is after the delay
2. It is inside the try block, so any exceptions that occur that ShouldRetryOn says should be retried will be retried
3. There is an async version

In addition, this should not impose a significant performance penalty in the common (non-retry) case, as only an if check of a boolean variable is added on the first execution of the loop. And because of the delay logic, the additional cost of a non-overridden virtual method call on a retry is negligible.

The only caveat that would be needed for the documentation would be that if OnOperationRetrying throws, that would count as a retry and operation would not be called. But by its nature, that is the expected behavior. (If OnOperationRetrying fails, the expectation is that operation would likely fail anyways.)

I'd be happy to submit a PR for this, but I wanted to get feedback on it first to see if it's acceptable, or if someone has an alternative of a better way of doing this (without having to copy/paste reimplement ExecutionStrategy).

[AndriySvyryd]

@paulirwin General approach looks good, we'll comment on the details in the PR.