Configurable retry logic can retry SqlCommands that have a transaction
- Dominant language
- C#
- Stars
- 989
- Forks
- 340
- Avg merge
- 4d 19h
- Merged PRs (30d)
- 72
Description
### Describe the bug
When applied to a `SqlCommand` the `RetryCondition` within `SqlRetryLogic` checks the `Transaction` property on the `SqlCommand`, and returns false if the `Transaction` is null, indicating that the command *shouldn't* be retried:
https://github.com/dotnet/SqlClient/blob/v7.0.1/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/Common/SqlRetryLogic.cs#L88
However, this seems to fall foul of the issue mentioned in #1509, that is that the `Transaction` property gets cleared if the error causes the transaction to abort, meaning that a `SqlCommand` with a transaction can be retried *without* a transaction.
This can mean that commands that were intented to run within a transaction (e.g. may need to be rolled back) can be executed outside of any transaction in case of a transient error such as a deadlock.
### To reproduce
```c#
using Microsoft.Data.SqlClient;
namespace SqlCommandRetryProviderTest;
internal class Program {
static async Task Main(string[] args) {
const string baseConnectionString =
@"Data Source=(localdb)\MSSQLLocalDB;Integrated Security=True;";
const string databaseName = "TestDb";
CreateDatabaseAndSchema(baseConnectionString + "Initial Catalog=master;", databaseName);
await CreateDeadlock(baseConnectionString + $"Initial Catalog={databaseName};");
}
private static async Task CreateDeadlock(string connectionString) {
SqlConnection conn1 = new SqlConnection(connectionString);
conn1.Open();
SqlConnection conn2 = new SqlConnection(connectionString);
conn2.Open();
using SqlTransaction txn1 = conn1.BeginTransaction();
using SqlTransaction txn2 = conn2.BeginTransaction();
var commandRetryProvider = SqlConfigurableRetryFactory
.CreateExponentialRetryProvider(new SqlRetryLogicOption {
NumberOfTries = 2,
});
commandRetryProvider.Retrying += (sender, eventArgs) => {
SqlCommand cmd = (SqlCommand)sender!;
Console.WriteLine("SqlCommand " + cmd.GetHashCode() + " has transaction: " + cmd.Transaction);
throw new InvalidOperationException("This shouldn't happen!");
};
async Task ExecuteNonQueryAsync(SqlConnection conn, SqlTransaction txn, string commandText) {
using SqlCommand cmd = new SqlCommand(commandText, conn, txn);
Console.WriteLine("SqlCommand " + cmd.GetHashCode() + " has transaction: " + cmd.Transaction);
cmd.RetryLogicProvider = commandRetryProvider;
await cmd.ExecuteNonQueryAsync();
return;
}
await ExecuteNonQueryAsync(conn1, txn1, "INSERT INTO [Table1] ([Id], [Value]) VALUES (1, 1)");
await ExecuteNonQueryAsync(conn2, txn2, "INSERT INTO [Table1] ([Id], [Value]) VALUES (2, 2)");
Task t1 = ExecuteNonQueryAsync(conn1, txn1, "UPDATE [Table1] SET [Value] = 2 WHERE [Id] = 1");
Task t2 = ExecuteNonQueryAsync(conn2, txn2, "UPDATE [Table1] SET [Value] = 1 WHERE [Id] = 2");
await Task.WhenAll(t1, t2);
}
private static void CreateDatabaseAndSchema(string connectionString, string dbName) {
using SqlConnection conn = new SqlConnection(connectionString);
conn.Open();
void ExecuteNonQuery(string commandText) {
using SqlCommand cmd = new SqlCommand(commandText, conn);
cmd.ExecuteNonQuery();
}
ExecuteNonQuery($"DROP DATABASE IF EXISTS [{dbName}]");
ExecuteNonQuery($"CREATE DATABASE [{dbName}]");
ExecuteNonQuery($"CREATE TABLE [{dbName}].[dbo].[Table1] ([Id] int, [Value] int)");
}
}
```
### Expected behavior
`SqlCommand`s that have a `Transaction` set at the time of first execution should not be retried. For example, in the code above, the `throw new InvalidOperationException` line should not be hit.
### Further technical details
Microsoft.Data.SqlClient version: 7.0.1
.NET target: 10.0.8
SQL Server version: Local DB 17.0.4025.3 (also recently observed on Azure SQL and SQL Server 2022)
Operating system: Windows 11 24H2
Contributor guide
Assessment
This issue has not been assessed yet.