Skip to content

Commit ef3b678

Browse files
authored
Update the retry policy for tsql query execution (#2285)
## Changes 1. Updates retry policy for QueryExecutor to attempt 3 times instead of 5 times as per guidance 2. Remove sql error number 18456 from the list of transient exceptions which would result in retries. 18456 is a fatal error. ## Why make this change? Sql Error 18456 occurs when using invalid authentication methods such as invalid audience or expired token or credentials. There is a permanent error hence no benefit from retrying. Ref: https://learn.microsoft.com/en-us/sql/relational-databases/errors-events/mssqlserver-18456-database-engine-error?view=sql-server-ver16 Additionally, the number of retries of 5 means that it would take considerable for user to see the error returned. This does not bode well for GraphQL queries which are an interactive scenario. Ref: https://learn.microsoft.com/en-us/azure/architecture/best-practices/retry-service-specific#retry-usage-guidance-3 ## How was this tested? - Manual testing to ensure retry attempts down to 2 down from 5 via breakpoints - Manual testing to reproduce error 18456 and ensure no retries via breakpoints - Unit tests
1 parent 73098a9 commit ef3b678

File tree

3 files changed

+3
-7
lines changed

3 files changed

+3
-7
lines changed

src/Core/Resolvers/MsSqlDbExceptionParser.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ public MsSqlDbExceptionParser(RuntimeConfigProvider configProvider) : base(confi
4848

4949
// These errors mainly occur when the SQL Server client can't connect to the server.
5050
// This may happen when the client cannot resolve the name of the server or the name of the server is incorrect.
51-
"53", "11001",
52-
53-
// Transient error codes compiled from:
54-
// https://docs.microsoft.com/en-us/sql/relational-databases/errors-events/database-engine-events-and-errors?view=sql-server-ver16
55-
"18456"
51+
"53", "11001"
5652
});
5753

5854
ConflictExceptionCodes.UnionWith(new List<string>

src/Core/Resolvers/QueryExecutor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class QueryExecutor<TConnection> : IQueryExecutor
3030

3131
// The maximum number of attempts that can be made to execute the query successfully in addition to the first attempt.
3232
// So to say in case of transient exceptions, the query will be executed (_maxRetryCount + 1) times at max.
33-
private static int _maxRetryCount = 5;
33+
private static int _maxRetryCount = 2;
3434

3535
private AsyncRetryPolicy _retryPolicyAsync;
3636

src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ await provider.Initialize(
140140
[TestMethod, TestCategory(TestCategory.MSSQL)]
141141
public async Task TestRetryPolicyExhaustingMaxAttempts()
142142
{
143-
int maxRetries = 5;
143+
int maxRetries = 2;
144144
int maxAttempts = maxRetries + 1; // 1 represents the original attempt to execute the query in addition to retries.
145145
RuntimeConfig mockConfig = new(
146146
Schema: "",

0 commit comments

Comments
 (0)