1

This is my Polly implementation, it has two policy, one timeout and one retry. The idea is that when sql time out, the timeout span will become longer, so sql server got more time to do the work.

However, when using a sp that takes minutes to finish to simulate timeout, I do not see the timeout policy fired 3 times(either attach a debugger or just search the output log). It fired once and then TimeoutRejectedException will be thrown.

var timeoutPerTry = Policy
    .TimeoutAsync(context =>
    {
        ////enlarge timeout every time it happens

        taskTimeoutInSeconds = (int)(timeoutMs / 1000);

        Log.LogVerbose(
            $"log something");
        return TimeSpan.FromMilliseconds(timeoutMs);
    }, TimeoutStrategy.Optimistic);

// retry SqlException up to MaxRetries
var retryPolicy = Policy
    .Handle<SqlException>()
    .RetryAsync(Constants.MaxRetries,
        (response, calculatedWaitDuration, context) =>
        {
            Log.LogError(
                $"Failed dynamic execution attempt. Retrying. {response.Message} - {response.StackTrace}");
        });

try
{
    ////combine timeout policy and retry policy
    var combinedPolicy = retryPolicy.WrapAsync(timeoutPerTry);
    // ReSharper disable once AccessToDisposedClosure
    var results =
        await combinedPolicy.ExecuteAsync<IEnumerable<T>>(async () => {

            var connectionString = ConnectionStringHelper.GetConnectionString(warehouseId);
            using (var connection = new SqlConnection(connectionString))  // assumed no need for using block as closed by caller
            {
                await connection.OpenAsync();
                using (var cmd = new SqlCommand
                {
                    CommandType = commandType,
                    CommandTimeout = taskTimeoutInSeconds, // in secs
                    CommandText = "JerrySimulateSlowSp"
                })
                {
                    cmd.Parameters.AddRange(parameters.ToArray());
                    cmd.Connection = connection;

                    using (var reader = await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection))
                    {
                        return mapper.Map<IDataReader, IEnumerable<T>>(reader);
                    }
                }
            }
        });
    return results;
    //cmd.Connection = null;        
}
catch (SqlException ex) when (ex.Number == -2)  // -2 is a sql timeout
{
    throw new ThunderTimeoutException(Constants.HttpResponseTimeoutSql);
}
catch (TimeoutRejectedException)
{
    throw new ThunderTimeoutException(Constants.HttpResponseTimeoutTask);
}
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
daxu
  • 3,514
  • 5
  • 38
  • 76

1 Answers1

2

Polly's timeout policy supports two types of operation:

  • Optimistic: The decorated method can co-op with a CancellationToken
  • Pessimistic: The decorated method can NOT co-op with a CancellationToken

Fortunately the ExecuteReaderAsync does support CancellationToken, so we can use optimistic timeout policy here. The trick is that you have you use a different overload of ExecuteAsync

.ExecuteAsync(async ct => 
{
   ...
   var reader = await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection, ct);
   ...
}, CancellationToken.None); 

In this case the ExecuteReaderAsync will use the timeout's CancellationToken. If you have another CancellationToken (for instance to allow user interaction based cancellation) then you can combine that with the timeout's one by passing that token instead of CancellationToken.None

.ExecuteAsync(async combinedToken => 
{
   ...
   var reader = await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection, combinedToken);
   ...
}, userCancellationToken); 

Side-note: Please prefer PolicyWrap over WrapAsync

var combinedPolicy = PolicyWrap.WrapAsync(retryPolicy, timeoutPerTry);

Related SO topics: 1, 2, 3

Peter Csala
  • 17,736
  • 16
  • 35
  • 75
  • hi, Sorry. I didn't really understand your reply. Your method is certainly much better than our one, but I still not sure why the timeout wasn't fired – daxu Jul 06 '22 at 09:57
  • @daxu It is all about the following. Change this line: await combinedPolicy.ExecuteAsync>(async () => { to this: await combinedPolicy.ExecuteAsync>(async ct => { – Peter Csala Jul 06 '22 at 10:30
  • And at the closing bracket change this : }); to this: }, CancellationToken.None); – Peter Csala Jul 06 '22 at 10:32
  • @daxu Which part of my post needs further explanation? – Peter Csala Jul 07 '22 at 08:26
  • think I need to re-read the document and play with the code – daxu Jul 07 '22 at 14:27