Skip to content

Commit fbe03e5

Browse files
naxing123Aniruddh25anushakolan
authored
Use cancellation token in DbCommand.ExecuteReaderAsync call in DAB layer (#3300)
This is to address #3301 Use cancellation token in DbCommand.ExecuteReaderAsync call in DAB layer Problem: RequestTimeoutAttribute only works during graphql workload execution, it doesn’t take effect during query execution as the logic to use cancellation token to DAB layer is NOT implemented, even though the code to pass cancellation token to DAB is already implemented. Fix: Use cancellation token to DAB layer in DbCommand.ExecuteReaderAsync call ## Why make this change? - This is the 3rd task to address multiple GraphQL timeout related issues. Discussion: https://microsoft.sharepoint.com/:w:/r/teams/dbsaaspillar/_layouts/15/doc2.aspx?sourcedoc=%7B230dcf09-f556-4071-877a-8294b7df7338%7D&action=edit&wdPid=62b0c291&share=IQEJzw0jVvVxQId6gpS333M4AS2c9wqqYSgUlkTBDxQSCEw ## What is this change? - Summary of how your changes work to give reviewers context of your intent. - Use cancellation token(if any) in DbCommand.ExecuteReaderAsync call in DAB layer ## How was this tested? - [ ] Integration Tests - [X] Unit Tests - Also did manual test with long running(3 minutes) SQL query from graphql and cancellation token with 30 seconds timeout, from debugger I can see code goes to DbCommand.ExecuteReaderAsync call in DAB layer and times out after 30 seconds. --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> Co-authored-by: Anusha Kolan <anushakolan10@gmail.com>
1 parent 29a1b7d commit fbe03e5

2 files changed

Lines changed: 209 additions & 2 deletions

File tree

src/Core/Resolvers/QueryExecutor.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,11 @@ public virtual TConnection CreateConnection(string dataSourceName)
293293
TResult? result = default(TResult);
294294
try
295295
{
296-
using DbDataReader dbDataReader = ConfigProvider.GetConfig().MaxResponseSizeLogicEnabled() ?
297-
await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess) : await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection);
296+
CommandBehavior commandBehavior = ConfigProvider.GetConfig().MaxResponseSizeLogicEnabled() ? CommandBehavior.SequentialAccess : CommandBehavior.CloseConnection;
297+
// CancellationToken is passed to ExecuteReaderAsync to ensure that if the client times out while the query is executing, the execution will be cancelled and resources will be freed up.
298+
CancellationToken cancellationToken = httpContext?.RequestAborted ?? CancellationToken.None;
299+
using DbDataReader dbDataReader = await cmd.ExecuteReaderAsync(commandBehavior, cancellationToken);
300+
298301
if (dataReaderHandler is not null && dbDataReader is not null)
299302
{
300303
result = await dataReaderHandler(dbDataReader, args);

src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Linq;
1212
using System.Net;
1313
using System.Reflection;
14+
using System.Threading;
1415
using System.Threading.Tasks;
1516
using Azure.Core;
1617
using Azure.DataApiBuilder.Config;
@@ -1015,6 +1016,209 @@ private static Mock<IHttpContextAccessor> CreateHttpContextAccessorWithAuthentic
10151016
#endregion
10161017

10171018
/// <summary>
1019+
/// Validates that when the CancellationToken from httpContext.RequestAborted times out
1020+
/// during a long-running query execution (simulating ExecuteReaderAsync being interrupted
1021+
/// by a token timeout), the resulting TaskCanceledException propagates through the Polly
1022+
/// retry policy without any retry attempts.
1023+
/// Unlike TestCancellationExceptionIsNotRetriedByRetryPolicy which throws immediately,
1024+
/// this test simulates a real timeout where the cancellation occurs asynchronously
1025+
/// after a delay.
1026+
/// </summary>
1027+
[TestMethod, TestCategory(TestCategory.MSSQL)]
1028+
public async Task TestCancellationTokenTimeoutDuringQueryExecutionAsync()
1029+
{
1030+
RuntimeConfig mockConfig = new(
1031+
Schema: "",
1032+
DataSource: new(DatabaseType.MSSQL, "", new()),
1033+
Runtime: new(
1034+
Rest: new(),
1035+
GraphQL: new(),
1036+
Mcp: new(),
1037+
Host: new(null, null)
1038+
),
1039+
Entities: new(new Dictionary<string, Entity>())
1040+
);
1041+
1042+
MockFileSystem fileSystem = new();
1043+
fileSystem.AddFile(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, new MockFileData(mockConfig.ToJson()));
1044+
FileSystemRuntimeConfigLoader loader = new(fileSystem);
1045+
RuntimeConfigProvider provider = new(loader)
1046+
{
1047+
IsLateConfigured = true
1048+
};
1049+
1050+
Mock<ILogger<QueryExecutor<SqlConnection>>> queryExecutorLogger = new();
1051+
Mock<IHttpContextAccessor> httpContextAccessor = new();
1052+
HttpContext context = new DefaultHttpContext();
1053+
httpContextAccessor.Setup(x => x.HttpContext).Returns(context);
1054+
DbExceptionParser dbExceptionParser = new MsSqlDbExceptionParser(provider);
1055+
Mock<MsSqlQueryExecutor> queryExecutor
1056+
= new(provider, dbExceptionParser, queryExecutorLogger.Object, httpContextAccessor.Object, null, null);
1057+
1058+
queryExecutor.Setup(x => x.ConnectionStringBuilders).Returns(new Dictionary<string, DbConnectionStringBuilder>());
1059+
1060+
queryExecutor.Setup(x => x.CreateConnection(
1061+
It.IsAny<string>())).CallBase();
1062+
1063+
// Set up a CancellationTokenSource that times out after a short delay,
1064+
// simulating httpContext.RequestAborted firing due to a client timeout.
1065+
CancellationTokenSource cts = new();
1066+
cts.CancelAfter(TimeSpan.FromMilliseconds(100));
1067+
context.RequestAborted = cts.Token;
1068+
1069+
// Mock ExecuteQueryAgainstDbAsync to simulate a long-running database query
1070+
// that is interrupted when the CancellationToken times out.
1071+
// Task.Delay with the cancellation token throws TaskCanceledException when the
1072+
// token fires, mimicking cmd.ExecuteReaderAsync being cancelled by a timed-out token.
1073+
// The Stopwatch + finally block mirrors the real ExecuteQueryAgainstDbAsync to verify
1074+
// that execution time is recorded even when a timeout occurs.
1075+
queryExecutor.Setup(x => x.ExecuteQueryAgainstDbAsync(
1076+
It.IsAny<SqlConnection>(),
1077+
It.IsAny<string>(),
1078+
It.IsAny<IDictionary<string, DbConnectionParam>>(),
1079+
It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
1080+
It.IsAny<HttpContext>(),
1081+
provider.GetConfig().DefaultDataSourceName,
1082+
It.IsAny<List<string>>()))
1083+
.Returns(async () =>
1084+
{
1085+
Stopwatch timer = Stopwatch.StartNew();
1086+
try
1087+
{
1088+
// Simulate a long-running query interrupted by token timeout.
1089+
// Timeout.Infinite (-1) means "wait forever" — the only way this
1090+
// completes is when cts.Token fires after ~100 ms, which causes
1091+
// Task.Delay to throw TaskCanceledException.
1092+
await Task.Delay(Timeout.Infinite, cts.Token);
1093+
return (object)null;
1094+
}
1095+
finally
1096+
{
1097+
timer.Stop();
1098+
queryExecutor.Object.AddDbExecutionTimeToMiddlewareContext(timer.ElapsedMilliseconds);
1099+
}
1100+
});
1101+
1102+
// Call the actual ExecuteQueryAsync method (includes Polly retry policy).
1103+
queryExecutor.Setup(x => x.ExecuteQueryAsync(
1104+
It.IsAny<string>(),
1105+
It.IsAny<IDictionary<string, DbConnectionParam>>(),
1106+
It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
1107+
It.IsAny<string>(),
1108+
It.IsAny<HttpContext>(),
1109+
It.IsAny<List<string>>())).CallBase();
1110+
1111+
// Act & Assert: TaskCanceledException should propagate without retries.
1112+
await Assert.ThrowsExceptionAsync<TaskCanceledException>(async () =>
1113+
{
1114+
await queryExecutor.Object.ExecuteQueryAsync<object>(
1115+
sqltext: string.Empty,
1116+
parameters: new Dictionary<string, DbConnectionParam>(),
1117+
dataReaderHandler: null,
1118+
dataSourceName: String.Empty,
1119+
httpContext: context,
1120+
args: null);
1121+
});
1122+
1123+
// Verify that the underlying database execution is invoked exactly once,
1124+
// confirming that Polly does not perform any retries for TaskCanceledException.
1125+
queryExecutor.Verify(q => q.ExecuteQueryAgainstDbAsync(
1126+
It.IsAny<SqlConnection>(),
1127+
It.IsAny<string>(),
1128+
It.IsAny<IDictionary<string, DbConnectionParam>>(),
1129+
It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
1130+
It.IsAny<HttpContext>(),
1131+
provider.GetConfig().DefaultDataSourceName,
1132+
It.IsAny<List<string>>()),
1133+
Times.Once);
1134+
1135+
// Verify the finally block recorded execution time even though the token timed out.
1136+
Assert.IsTrue(
1137+
context.Items.ContainsKey(TOTAL_DB_EXECUTION_TIME),
1138+
"HttpContext must contain the total db execution time even when the request is cancelled.");
1139+
}
1140+
1141+
/// <summary>
1142+
/// Validates that when ExecuteQueryAgainstDbAsync throws OperationCanceledException
1143+
/// (e.g., due to client disconnect via httpContext.RequestAborted cancellation token),
1144+
/// the Polly retry policy does NOT retry and the exception propagates to the caller.
1145+
/// The retry policy is configured to only handle DbException, so OperationCanceledException
1146+
/// should be immediately re-thrown without any retry attempts.
1147+
/// </summary>
1148+
[TestMethod, TestCategory(TestCategory.MSSQL)]
1149+
public async Task TestCancellationExceptionIsNotRetriedByRetryPolicy()
1150+
{
1151+
RuntimeConfig mockConfig = new(
1152+
Schema: "",
1153+
DataSource: new(DatabaseType.MSSQL, "", new()),
1154+
Runtime: new(
1155+
Rest: new(),
1156+
GraphQL: new(),
1157+
Mcp: new(),
1158+
Host: new(null, null)
1159+
),
1160+
Entities: new(new Dictionary<string, Entity>())
1161+
);
1162+
1163+
MockFileSystem fileSystem = new();
1164+
fileSystem.AddFile(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, new MockFileData(mockConfig.ToJson()));
1165+
FileSystemRuntimeConfigLoader loader = new(fileSystem);
1166+
RuntimeConfigProvider provider = new(loader)
1167+
{
1168+
IsLateConfigured = true
1169+
};
1170+
1171+
Mock<ILogger<QueryExecutor<SqlConnection>>> queryExecutorLogger = new();
1172+
Mock<IHttpContextAccessor> httpContextAccessor = new();
1173+
DbExceptionParser dbExceptionParser = new MsSqlDbExceptionParser(provider);
1174+
Mock<MsSqlQueryExecutor> queryExecutor
1175+
= new(provider, dbExceptionParser, queryExecutorLogger.Object, httpContextAccessor.Object, null, null);
1176+
1177+
queryExecutor.Setup(x => x.ConnectionStringBuilders).Returns(new Dictionary<string, DbConnectionStringBuilder>());
1178+
1179+
queryExecutor.Setup(x => x.CreateConnection(
1180+
It.IsAny<string>())).CallBase();
1181+
1182+
// Mock ExecuteQueryAgainstDbAsync to throw OperationCanceledException,
1183+
// simulating a cancelled CancellationToken from httpContext.RequestAborted.
1184+
queryExecutor.Setup(x => x.ExecuteQueryAgainstDbAsync(
1185+
It.IsAny<SqlConnection>(),
1186+
It.IsAny<string>(),
1187+
It.IsAny<IDictionary<string, DbConnectionParam>>(),
1188+
It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
1189+
It.IsAny<HttpContext>(),
1190+
provider.GetConfig().DefaultDataSourceName,
1191+
It.IsAny<List<string>>()))
1192+
.ThrowsAsync(new OperationCanceledException("The operation was canceled."));
1193+
1194+
// Call the actual ExecuteQueryAsync method.
1195+
queryExecutor.Setup(x => x.ExecuteQueryAsync(
1196+
It.IsAny<string>(),
1197+
It.IsAny<IDictionary<string, DbConnectionParam>>(),
1198+
It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
1199+
It.IsAny<string>(),
1200+
It.IsAny<HttpContext>(),
1201+
It.IsAny<List<string>>())).CallBase();
1202+
1203+
// Act & Assert: OperationCanceledException should propagate without retries.
1204+
await Assert.ThrowsExceptionAsync<OperationCanceledException>(async () =>
1205+
{
1206+
await queryExecutor.Object.ExecuteQueryAsync<object>(
1207+
sqltext: string.Empty,
1208+
parameters: new Dictionary<string, DbConnectionParam>(),
1209+
dataReaderHandler: null,
1210+
dataSourceName: String.Empty,
1211+
httpContext: null,
1212+
args: null);
1213+
});
1214+
1215+
// Verify no retry log messages were emitted. Since IsLateConfigured is true,
1216+
// the debug log is skipped, and since Polly doesn't handle OperationCanceledException,
1217+
// no retry occurs → zero logger invocations.
1218+
Assert.AreEqual(0, queryExecutorLogger.Invocations.Count);
1219+
}
1220+
1221+
/// <summary>
10181222
/// Validates that GetSessionParamsQuery includes all observability values:
10191223
/// - OpenTelemetry correlation values (dab.trace_id, dab.span_id) when an Activity is present
10201224
/// - OBO observability values (dab.auth_type, dab.user_id, dab.tenant_id) when user-delegated auth is enabled

0 commit comments

Comments
 (0)