Skip to content

Commit 6878b3f

Browse files
Kaur-ParminderKaur-Parminder
committed
Fix multiple retry issue on ExecuteScalarAsync + add missing ConfigureAwait calls (Backport dotnet#1120)
1 parent 8908b92 commit 6878b3f

File tree

5 files changed

+218
-25
lines changed

5 files changed

+218
-25
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,7 +2517,10 @@ protected override Task<DbDataReader> ExecuteDbDataReaderAsync(CommandBehavior b
25172517
throw result.Exception.InnerException;
25182518
}
25192519
return result.Result;
2520-
}, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.NotOnCanceled, TaskScheduler.Default);
2520+
},
2521+
CancellationToken.None,
2522+
TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.NotOnCanceled,
2523+
TaskScheduler.Default);
25212524
}
25222525

25232526
/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/ExecuteReaderAsync[@name="default"]/*'/>
@@ -2617,13 +2620,9 @@ private void SetCachedCommandExecuteReaderAsyncContext(ExecuteReaderAsyncCallCon
26172620
}
26182621

26192622
/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/ExecuteScalarAsync[@name="CancellationToken"]/*'/>
2620-
public override Task<object> ExecuteScalarAsync(CancellationToken cancellationToken)
2621-
=> IsRetryEnabled ?
2622-
InternalExecuteScalarWithRetryAsync(cancellationToken) :
2623-
InternalExecuteScalarAsync(cancellationToken);
2624-
2625-
private Task<object> InternalExecuteScalarWithRetryAsync(CancellationToken cancellationToken)
2626-
=> RetryLogicProvider.ExecuteAsync(this, () => InternalExecuteScalarAsync(cancellationToken), cancellationToken);
2623+
public override Task<object> ExecuteScalarAsync(CancellationToken cancellationToken) =>
2624+
// Do not use retry logic here as internal call to ExecuteReaderAsync handles retry logic.
2625+
InternalExecuteScalarAsync(cancellationToken);
26272626

26282627
private Task<object> InternalExecuteScalarAsync(CancellationToken cancellationToken)
26292628
{

‎src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3071,14 +3071,10 @@ private Task<SqlDataReader> InternalExecuteReaderAsync(CommandBehavior behavior,
30713071
return returnedTask;
30723072
}
30733073

3074-
private Task<object> InternalExecuteScalarWithRetryAsync(CancellationToken cancellationToken)
3075-
=> RetryLogicProvider.ExecuteAsync(this, () => InternalExecuteScalarAsync(cancellationToken), cancellationToken);
3076-
30773074
/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/ExecuteScalarAsync[@name="CancellationToken"]/*'/>
3078-
public override Task<object> ExecuteScalarAsync(CancellationToken cancellationToken)
3079-
=> IsRetryEnabled ?
3080-
InternalExecuteScalarWithRetryAsync(cancellationToken) :
3081-
InternalExecuteScalarAsync(cancellationToken);
3075+
public override Task<object> ExecuteScalarAsync(CancellationToken cancellationToken) =>
3076+
// Do not use retry logic here as internal call to ExecuteReaderAsync handles retry logic.
3077+
InternalExecuteScalarAsync(cancellationToken);
30823078

30833079
private Task<object> InternalExecuteScalarAsync(CancellationToken cancellationToken)
30843080
{

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Reliability/Common/SqlRetryLogicProvider.cs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ internal class SqlRetryLogicProvider : SqlRetryLogicBaseProvider
1717
{
1818
private const string TypeName = nameof(SqlRetryLogicProvider);
1919
// keeps free RetryLogic objects
20-
private readonly ConcurrentBag<SqlRetryLogicBase> _retryLogicPool = new ConcurrentBag<SqlRetryLogicBase>();
20+
private readonly ConcurrentBag<SqlRetryLogicBase> _retryLogicPool = new();
2121

2222
/// <summary>Creates an instance of this type.</summary>
2323
public SqlRetryLogicProvider(SqlRetryLogicBase retryLogic)
@@ -28,8 +28,7 @@ public SqlRetryLogicProvider(SqlRetryLogicBase retryLogic)
2828

2929
private SqlRetryLogicBase GetRetryLogic()
3030
{
31-
SqlRetryLogicBase retryLogic = null;
32-
if (!_retryLogicPool.TryTake(out retryLogic))
31+
if (!_retryLogicPool.TryTake(out SqlRetryLogicBase retryLogic))
3332
{
3433
retryLogic = RetryLogic.Clone() as SqlRetryLogicBase;
3534
}
@@ -69,7 +68,7 @@ public override TResult Execute<TResult>(object sender, Func<TResult> function)
6968
{
7069
if (RetryLogic.RetryCondition(sender) && RetryLogic.TransientPredicate(e))
7170
{
72-
retryLogic = retryLogic ?? GetRetryLogic();
71+
retryLogic ??= GetRetryLogic();
7372
SqlClientEventSource.Log.TryTraceEvent("<sc.{0}.Execute<TResult>|INFO> Found an action eligible for the retry policy (retried attempts = {1}).",
7473
TypeName, retryLogic.Current);
7574
exceptions.Add(e);
@@ -107,15 +106,15 @@ public override async Task<TResult> ExecuteAsync<TResult>(object sender, Func<Ta
107106
retry:
108107
try
109108
{
110-
TResult result = await function.Invoke();
109+
TResult result = await function.Invoke().ConfigureAwait(false);
111110
RetryLogicPoolAdd(retryLogic);
112111
return result;
113112
}
114113
catch (Exception e)
115114
{
116115
if (RetryLogic.RetryCondition(sender) && RetryLogic.TransientPredicate(e))
117116
{
118-
retryLogic = retryLogic ?? GetRetryLogic();
117+
retryLogic ??= GetRetryLogic();
119118
SqlClientEventSource.Log.TryTraceEvent("<sc.{0}.ExecuteAsync<TResult>|INFO> Found an action eligible for the retry policy (retried attempts = {1}).",
120119
TypeName, retryLogic.Current);
121120
exceptions.Add(e);
@@ -124,7 +123,7 @@ public override async Task<TResult> ExecuteAsync<TResult>(object sender, Func<Ta
124123
// The retrying event raises on each retry.
125124
ApplyRetryingEvent(sender, retryLogic, intervalTime, exceptions, e);
126125

127-
await Task.Delay(intervalTime, cancellationToken);
126+
await Task.Delay(intervalTime, cancellationToken).ConfigureAwait(false);
128127
goto retry;
129128
}
130129
else
@@ -153,14 +152,14 @@ public override async Task ExecuteAsync(object sender, Func<Task> function, Canc
153152
retry:
154153
try
155154
{
156-
await function.Invoke();
155+
await function.Invoke().ConfigureAwait(false);
157156
RetryLogicPoolAdd(retryLogic);
158157
}
159158
catch (Exception e)
160159
{
161160
if (RetryLogic.RetryCondition(sender) && RetryLogic.TransientPredicate(e))
162161
{
163-
retryLogic = retryLogic ?? GetRetryLogic();
162+
retryLogic ??= GetRetryLogic();
164163
SqlClientEventSource.Log.TryTraceEvent("<sc.{0}.ExecuteAsync|INFO> Found an action eligible for the retry policy (retried attempts = {1}).",
165164
TypeName, retryLogic.Current);
166165
exceptions.Add(e);
@@ -169,7 +168,7 @@ public override async Task ExecuteAsync(object sender, Func<Task> function, Canc
169168
// The retrying event raises on each retry.
170169
ApplyRetryingEvent(sender, retryLogic, intervalTime, exceptions, e);
171170

172-
await Task.Delay(intervalTime, cancellationToken);
171+
await Task.Delay(intervalTime, cancellationToken).ConfigureAwait(false);
173172
goto retry;
174173
}
175174
else

‎src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
</Compile>
7171
<Compile Include="DataCommon\AADUtility.cs" />
7272
<Compile Include="SQL\ConfigurableIpPreferenceTest\ConfigurableIpPreferenceTest.cs" />
73+
<Compile Include="SQL\RetryLogic\RetryLogicCounterTest.cs" />
7374
<Compile Include="SQL\RetryLogic\RetryLogicConfigHelper.cs" />
7475
<Compile Include="SQL\RetryLogic\RetryLogicTestHelper.cs" />
7576
<Compile Include="SQL\RetryLogic\SqlCommandReliabilityTest.cs" />

‎src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/RetryLogicCounterTest.cs‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Xunit;
9+
10+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests
11+
{
12+
public class RetryLogicCounterTest
13+
{
14+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
15+
[InlineData("ExecuteScalarAsync", 3)]
16+
[InlineData("ExecuteReaderAsync", 3)]
17+
[InlineData("ExecuteXmlReaderAsync", 3)]
18+
[InlineData("ExecuteNonQueryAsync", 3)]
19+
public async void ValidateRetryCount_SqlCommand_Async(string methodName, int numOfTries)
20+
{
21+
ErrorInfoRetryLogicProvider _errorInfoRetryProvider = new(
22+
SqlConfigurableRetryFactory.CreateFixedRetryProvider(new SqlRetryLogicOption()
23+
{ NumberOfTries = numOfTries, TransientErrors = new[] { 50000 } }));
24+
25+
try
26+
{
27+
RetryLogicTestHelper.SetRetrySwitch(true);
28+
29+
using var connection = new SqlConnection(DataTestUtility.TCPConnectionString);
30+
connection.Open();
31+
32+
using SqlCommand cmd = connection.CreateCommand();
33+
cmd.RetryLogicProvider = _errorInfoRetryProvider;
34+
cmd.CommandText = "THROW 50000,'Error',0";
35+
36+
_errorInfoRetryProvider.CallCounter = 0;
37+
switch (methodName)
38+
{
39+
case "ExecuteScalarAsync":
40+
await cmd.ExecuteScalarAsync();
41+
break;
42+
case "ExecuteReaderAsync":
43+
{
44+
using SqlDataReader _ = await cmd.ExecuteReaderAsync();
45+
break;
46+
}
47+
case "ExecuteXmlReaderAsync":
48+
{
49+
using System.Xml.XmlReader _ = await cmd.ExecuteXmlReaderAsync();
50+
break;
51+
}
52+
case "ExecuteNonQueryAsync":
53+
await cmd.ExecuteNonQueryAsync();
54+
break;
55+
default:
56+
break;
57+
}
58+
Assert.False(true, "Exception did not occur.");
59+
}
60+
catch
61+
{
62+
Assert.Equal(numOfTries, _errorInfoRetryProvider.CallCounter);
63+
}
64+
finally
65+
{
66+
RetryLogicTestHelper.SetRetrySwitch(false);
67+
}
68+
}
69+
70+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
71+
[InlineData("ExecuteScalar", 3)]
72+
[InlineData("ExecuteReader", 3)]
73+
[InlineData("ExecuteXmlReader", 3)]
74+
[InlineData("ExecuteNonQuery", 3)]
75+
public void ValidateRetryCount_SqlCommand_Sync(string methodName, int numOfTries)
76+
{
77+
ErrorInfoRetryLogicProvider _errorInfoRetryProvider = new(
78+
SqlConfigurableRetryFactory.CreateFixedRetryProvider(new SqlRetryLogicOption()
79+
{ NumberOfTries = numOfTries, TransientErrors = new[] { 50000 } }));
80+
81+
try
82+
{
83+
RetryLogicTestHelper.SetRetrySwitch(true);
84+
85+
using var connection = new SqlConnection(DataTestUtility.TCPConnectionString);
86+
connection.Open();
87+
88+
using SqlCommand cmd = connection.CreateCommand();
89+
cmd.RetryLogicProvider = _errorInfoRetryProvider;
90+
cmd.CommandText = "THROW 50000,'Error',0";
91+
92+
_errorInfoRetryProvider.CallCounter = 0;
93+
switch (methodName)
94+
{
95+
case "ExecuteScalar":
96+
cmd.ExecuteScalar();
97+
break;
98+
case "ExecuteReader":
99+
{
100+
using SqlDataReader _ = cmd.ExecuteReader();
101+
break;
102+
}
103+
case "ExecuteXmlReader":
104+
{
105+
using System.Xml.XmlReader _ = cmd.ExecuteXmlReader();
106+
break;
107+
}
108+
case "ExecuteNonQuery":
109+
cmd.ExecuteNonQuery();
110+
break;
111+
default:
112+
break;
113+
}
114+
Assert.False(true, "Exception did not occur.");
115+
}
116+
catch
117+
{
118+
Assert.Equal(numOfTries, _errorInfoRetryProvider.CallCounter);
119+
}
120+
finally
121+
{
122+
RetryLogicTestHelper.SetRetrySwitch(false);
123+
}
124+
}
125+
126+
public class ErrorInfoRetryLogicProvider : SqlRetryLogicBaseProvider
127+
{
128+
public SqlRetryLogicBaseProvider InnerProvider { get; }
129+
130+
public ErrorInfoRetryLogicProvider(SqlRetryLogicBaseProvider innerProvider)
131+
{
132+
InnerProvider = innerProvider;
133+
}
134+
135+
readonly AsyncLocal<int> _depth = new();
136+
public int CallCounter = 0;
137+
138+
TResult LogCalls<TResult>(Func<TResult> function)
139+
{
140+
CallCounter++;
141+
return function();
142+
}
143+
144+
public override TResult Execute<TResult>(object sender, Func<TResult> function)
145+
{
146+
_depth.Value++;
147+
try
148+
{
149+
return InnerProvider.Execute(sender, () => LogCalls(function));
150+
}
151+
finally
152+
{
153+
_depth.Value--;
154+
}
155+
}
156+
157+
public async Task<TResult> LogCallsAsync<TResult>(Func<Task<TResult>> function)
158+
{
159+
CallCounter++;
160+
return await function();
161+
}
162+
163+
public override async Task<TResult> ExecuteAsync<TResult>(object sender, Func<Task<TResult>> function,
164+
CancellationToken cancellationToken = default)
165+
{
166+
_depth.Value++;
167+
try
168+
{
169+
return await InnerProvider.ExecuteAsync(sender, () => LogCallsAsync(function), cancellationToken);
170+
}
171+
finally
172+
{
173+
_depth.Value--;
174+
}
175+
}
176+
177+
public async Task LogCallsAsync(Func<Task> function)
178+
{
179+
CallCounter++;
180+
await function();
181+
}
182+
183+
public override async Task ExecuteAsync(object sender, Func<Task> function,
184+
CancellationToken cancellationToken = default)
185+
{
186+
_depth.Value++;
187+
try
188+
{
189+
await InnerProvider.ExecuteAsync(sender, () => LogCallsAsync(function), cancellationToken);
190+
}
191+
finally
192+
{
193+
_depth.Value--;
194+
}
195+
}
196+
}
197+
}
198+
}

0 commit comments

Comments
 (0)