dotnet / dotnet/SqlClient

Configurable retry logic can retry SqlCommands that have a transaction

Open
#4,309 1 comment 0 reactions 0 assignees View on GitHub
Repro Available :heavy_check_mark:
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.