Support for SqlRetryLogic

[vakuras]

Hello,

Is there a plan to support the new SqlRetryLogic in dapper?
A way to pass the retry logic to the SqlCommand object?

Thanks

[mgravell]

My limited understanding (from here is that .RetryLogicProvider can be specified at the connection level; so for that: no changes would be required. At the command level: no, this is not currently supported

[vakuras]

Is there a plan to support it?

[mgravell]

Honestly, today is the first time I've heard of it. Typically, we prefer to target the primary ADO.NET API surface (since we don't want to force any provider-specific dependency trees), and this is very specifically a Microsoft.Data.SqlClient feature, which makes it a little awkward to support. We'd need to think about it, basically.

[billrob]

@vakuras I have had good luck with Polly to easily wrap Sql retry logic. It's syntax is a little awkward to start, but eventually grew on me.

[fsbflavio]

It would be advantageous to include a retry on fail mechanism in the built-in methods, eliminating the need to create wrappers for all Dapper's methods to handle the errors.

Currently, I am doing something like this:

public new IEnumerable<T> Query<T>(string sql, object param = null)
    {
        var retries = 0;
        while (true)
        {
            try
            {
                // Try to execute the query
                return connection.Query<T>(sql, param); //connection is IDbConnection
            }
            catch (Exception ex)
            {
                // Check if the exception is a network error (replace ORA-12570 with the actual error code)
                if (ex.Number == 'ORA-12570' ) //a list of transient errors
                {
                    if (++retries == _retryCount)
                    {
                        throw; // If all retries failed, propagate the exception.
                    }
                    Thread.Sleep(TimeSpan.FromSeconds(Math.Pow(2, retries))); // Wait before retrying.
                }
                else
                {
                    throw; // If it's not a network error, propagate the exception.
                }
            }
        }
    }
    ```

[billrob]

As you can see from even your own example, there isn't a good way to know when to bubble up and when to retry, then we have a max retryCount to content with.

A DbConcurrencyException will need additional cleanup code before retrying, a SqlException with message of timeout is another behavior.

You should be wrapping all Dapper calls with your own layer nevertheless.

[fsbflavio]

That's true.
But, it would be useful for Dapper to allow configuring a list of transient error codes/exceptions to retry on, for each database it supports. This could be done in a fluent style, similar to Entity Framework. For example:

public class RetryOnErrorStrategyForOracle : OracleRetryingExecutionStrategy // in Dapper
{
    private static readonly IList<int> ErrorNumbersToRetry = new List<int>()
    {
        12570, 12571, 03135 //lost connection errors
    };

    public SomeRetryStrategy()
        : base(DefaultMaxRetryCount, ErrorNumbersToRetry)
    {
    }
}


//configure Dapper to use the custom strategy in the methods
connection.Query("select * from ...", new RetryOnErrorStrategyForOracle())

Now Dapper could use the configured retry strategy. To keep it simple, it could just use the sample error code list provided here for the OracleRetryingExecutionStrategy implementation.

This would help retry when connection errors occur - I often run into errors 12570, 12571, 03135 when the connection is lost.

OK, Wrapping all the Dapper methods to handle errors could work as a workaround, but modifying Dapper itself to support configurable retry strategies would be a more elegant solution.

Thanks!

[billrob]

I don't want to step on @mgravell and his thoughts for this library. Dapper should do Dapper things, if we want retry features: errors, exceptions, we should extend Dapper with a nuget package of Dapper.Extensions.Retry or something of the sort.

Dapper.dll, shouldn't be littered with DatabaseProviderRetryStrategy code. I like where your head is at for ease of adding retry logic, but not in the Dapper core library.

[mgravell]

I agree with @billrob's comments above; additionally, a low effort implementation is probably more harmful than helpful. If we you transient retry and resilience - that sounds like a job for Polly, no? Let Dapper do what it knows, and let Polly do what it knows. IMO.

[fsbflavio]

I understand.
I like the idea of extending Dapper with this option.
An extension library like Dapper.Extensions.Retry would allow implementing retries while keeping Dapper simple.
Polly really works but it is a little awkward to add its logic to all my methods, I end up having my own library that does it.

Thanks for providing that insight

[RyanThomas73]

Came across this issue while trying to find a way to use SqlRetryLogic. Support for the ability to use the out-of-the-box retries provided by Microsoft.Data.SqlClient would be a big win for me and my team over having to write our own wrapper around dapper and being forced to define the retry policy logic instead of just using the microsoft provided default.

@mgravell
At the core of my request, rather than try to implement provider specific features, can you simply give us an extension point around the command creation (that still adheres to just the abstract ADO.NET API sruface)?

For example, a hypothetical Action<IDbConnection, IDbCommand>? configureCommand = null parameter passed through to the command setup:

var cmd = cnn.CreateCommand();
configureCommand?.Invoke(cnn, cmd);
var init = GetInit(cmd.getType());
... rest of code ...

[billrob]

@RyanThomas73 Doesn't the SqlConnection object allows this already?

https://learn.microsoft.com/en-us/sql/connect/ado-net/configurable-retry-logic-sqlclient-introduction?view=sql-server-ver16

// Assumes that connection is a valid SqlConnection object 

// Set the retry logic provider on the connection instance
connection.RetryLogicProvider = provider;

// Establishing the connection will retry if a transient failure occurs.
connection.Open();

//now make Dapper calls.

[RyanThomas73]

@RyanThomas73 Doesn't the SqlConnection object allows this already?

Yes the connection does, but as mentioned in the above that only helps with connection.Open() call. My request is for an extension point to configure the SqlCommand objects so that we can also use the out-of-the-box retry provider for command execution.

For example, if the CommandDefinition supported an extension point such as an optional action for configuring the DbCommand object as suggested I could simply pass in a delegate, like this:

Action<IDbConnection, IDbCommand> configureCommand = (dbConnection, dbCommand) => {
    ((SqlCommand)dbCommand).RetryLogicProvider = ((SqlConnection)dbConnection).RetryLogicProvider;
}

Such an extension point would
a. Allow us to fully utilize the out-of-the-box features of the Microsoft.Data.SqlClient library
b. Avoid the need for the Dapper library to take on provider specific dependency logic branches
c. Allow other consumers of the Dapper library the ability to use that extension point to leverage other non sql server provider specific functionality that needs to be configured at the DbCommand level

[billrob]

Yes, you are correct. I would have predicted it would have used the retry logic on the connection if it wasn't overwritten on the command. I can see their point for not doing it.

Although I propose to @mgravell Func<IDbConnection, IDbCommand> createCommand the caller would now be in full control of the creation of the IDbCommand. It would be a easyish change to have a default implementation. I can picture other scenarios where more complex IDbCommand creations might come up with the increasing number of db providers out there.

internal static _defaultCreateCommand = new Func<IDbConnection, IDbCommand>((IDbConnection con)
   => con.CreateCommand());

It looks like all code paths end up on

internal IDbCommand SetupCommand(IDbConnection cnn, Action<IDbCommand, object?>? paramReader)

so it would just be some monkey work to add the optional parameter in everywhere.

Would that be more useful?

I suppose the only confusing might be if the caller sets their own command.xxx properties and Dapper's internal implementation changes some of that. That would be frustrating and confusing.

[RyanThomas73]

so it would just be some monkey work to add the optional parameter in everywhere.

The SqlMapper extension methods already have overloads that take the CommandDefinition directly, correct?

Since this is a fairly advanced use case I would argue that it would be sufficient to add such a Func<IDbConnection, IDbCommand>? as a CreateCommand property of the CommandDefinition but leave off adding optional parameters everywhere to minimize additional code the Dapper library needs to maintain.

Consumers that need to leverage this extension point could then create the CommandDefinition directly. e.g.

public async Task DoSomethingAsync(CancellationToken cancellationToken)
{
    using var sqlConnection = ... create and open sql connection ...
    var commandDefinition = new CommandDefinition(commandText, ... other parameters ..., createCommand: CustomCreateCommandDelegate, cancellationToken: cancellationToken);
    var results = await sqlConnection.ExecuteAsync(commandDefinition);
}