Skip to content

Commit 1e42c7e

Browse files
vkuttypclaude
andcommitted
fix(MsSql): default-validate is now IsOpen, no per-acquire SELECT 1 round-trip
The previous DefaultValidateConnectionAsync ran "SELECT 1" against every pooled connection on every acquire. Under cancellation pressure (HTTP server forwarding requests whose CT can fire mid-flight), that round-trip could stall on a connection whose previous response hadn't been fully drained — taking seconds and inviting the CT to cancel mid-validation. That was wedging CosmoS3 PUTs through cosmoproxy. Drop validation to a cheap !IsOpen-or-true check and let WithRetryAsync's existing dead-connection retry path detect actually-broken connections when the real query throws. Operators who want a wire-level health probe on every acquire can still pass their own validateConnection delegate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0aed7c1 commit 1e42c7e

1 file changed

Lines changed: 17 additions & 18 deletions

File tree

src/CosmoSQLClient.MsSql/MsSqlConnectionPool.cs

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -331,24 +331,23 @@ private async Task<T> WithRetryAsync<T>(Func<MsSqlConnection, Task<T>> op, Cance
331331
}
332332
}
333333

334-
private async ValueTask<bool> DefaultValidateConnectionAsync(MsSqlConnection conn, CancellationToken ct)
335-
{
336-
if (!conn.IsOpen)
337-
return false;
338-
try
339-
{
340-
await conn.QueryAsync("SELECT 1", ct: ct).ConfigureAwait(false);
341-
return true;
342-
}
343-
catch (OperationCanceledException)
344-
{
345-
throw;
346-
}
347-
catch
348-
{
349-
return false;
350-
}
351-
}
334+
// Default validation is intentionally lightweight: just check IsOpen.
335+
// We DO NOT run a "SELECT 1" round-trip on every acquire because
336+
// — when the pool is under cancellation pressure (e.g. an HTTP server
337+
// forwarding requests whose CT can fire at any moment) — that round-trip
338+
// can stall on a connection whose previous response wasn't fully drained,
339+
// leading to a request-time hang of seconds before the CT eventually
340+
// cancels validation. WithRetryAsync already retries on dead-connection
341+
// exceptions thrown by the real query (IOException, SocketException,
342+
// SqlException with connection-error kind, etc.), so a stale-but-IsOpen
343+
// connection will still recover transparently — it just defers the
344+
// detection to the actual statement instead of spending a network
345+
// round-trip on every acquire.
346+
//
347+
// Callers that need stronger validation can supply their own
348+
// validateConnection delegate via the ctor.
349+
private static ValueTask<bool> DefaultValidateConnectionAsync(MsSqlConnection conn, CancellationToken ct)
350+
=> new ValueTask<bool>(conn.IsOpen);
352351

353352
// ── ISqlDatabase ───────────────────────────────────────────────────────────
354353

0 commit comments

Comments
 (0)