Transient Fault Retry logic best practices

Question

Friends, I have a question about implementing a simple retry policy around the execution of the SQL command.

My question is: should the retry loop encapsulate the construction of the connection and transaction, or should it live inside the connection.

For example:

private void RetryLogSave(DynamicParameters parameters, int retries = 3)
{    
    int tries = 0;

    using (var connection = new SqlConnection(_connectionString))
    {
        connection.Open();

        using (var transaction = connection.BeginTransaction())
        {
            var logItemCommand = new CommandDefinition(commandText: Constants.InsertLogItem,
                parameters: parameters, transaction: transaction, commandType: System.Data.CommandType.Text);

            do
            {
                try
                {
                    tries++;
                    connection.Execute(logItemCommand);
                    transaction.Commit();
                    break;
                }
                catch (Exception exc)
                {
                    if (tries == retries)
                    {
                        transaction.Rollback();
                        throw exc;
                    }
                    Task.Delay(100 * tries).Wait();
                }
            }
            while (true);
        }
}
}

Is what I've done here appropriate and acceptable, or should the retry logic live on the outside of the SqlConnection construction?

I usually use [Polly](https://github.com/App-vNext/Polly) for this. Transaction should be inside retry block. Try to keep your transactions / connections as small as possible — Dmitry Pavliv, Jun 02 '18 at 12:50
@DmitryPavliv I've heard good things about Polly and may try it today. So, your suggestion is that the transaction should be inside the block. What about the connection instantiation? Also, that would mean rolling back on each retry, correct? — Vic F, Jun 02 '18 at 12:55
Connection should be opened for as small amount of time as possible. Ideally you have connection pooling enabled in your odbc driver (this is usually by default). That means once you close connection, it is actually goes back to pool and some another method can reuse it. Having opened connection during your retries attempts may force system to create too many physical connection (as you do not release them back to connection pool) and exhaust your sql server — Dmitry Pavliv, Jun 02 '18 at 12:59

Dmitry Pavliv · Accepted Answer · 2018-06-02T13:47:14.207

Formalizing my comments as an answer.

should the retry logic live on the outside of the SqlConnection construction?

Yes. If doing retry logic with keeping connection opened you're wasting resources. Someone else may use it while you're waiting N seconds for re-try. Opening/closing connections is usually (for most ODBC drivers) implemented on top of Connection Pooling mechanism. You do not actually close it - you allow connection to go back in pool to be reused by someone else. Keeping connections opened during re-try will force system to create more and more new physical connections (because they are not returned to the pool) and eventually your SQL Server will be exhausted.

Regarding re-try mechanism - to not reinvent the wheel, I usually use Polly library.

You can define somewhere static class with list of your polices:

public static class MyPolices
{
    // Retry, waiting a specified duration between each retry
    public static Policy RetryPolicy = Policy
       .Handle<Exception>() // can be more specific like SqlException
       .WaitAndRetry(new[]
       {
          TimeSpan.FromSeconds(1),
          TimeSpan.FromSeconds(2),
          TimeSpan.FromSeconds(3)
       });
}

Then, simplify your method to

private void LogSave(DynamicParameters parameters)
{    
    using (var connection = new SqlConnection(_connectionString))
    {
        connection.Open();

        using (var transaction = connection.BeginTransaction())
        {
            // make sure to not forget to dispose your command
            var logItemCommand = new CommandDefinition(commandText: Constants.InsertLogItem,
                parameters: parameters, transaction: transaction, commandType: System.Data.CommandType.Text);

            try
            {
                // not sure if conn.Execute is your extension method?
                connection.Execute(logItemCommand);
                transaction.Commit();
            }
            catch (Exception exc)
            {
                transaction.Rollback();
                throw;
            }
        }
    }
}

and call it like this

MyPolices.RetryPolicy.Execute(() => LogSave(parameters));

This approach will make your code more SOLID keeping retry logic in isolation.

Transient Fault Retry logic best practices

1 Answers1