Add configurable idle connection timeout (ADO #39970)#4295
Add configurable idle connection timeout (ADO #39970)#4295priyankatiwari08 wants to merge 2 commits into
Conversation
Implements spec User Stories 1, 2, 4 + FR-009 of US3 from specs/003-pool-idle-timeout/spec.md. Adds 'Connection Idle Timeout' keyword (synonym: 'Pool Idle Timeout') exposed via SqlConnectionStringBuilder.IdleTimeout. When > 0, connections that have sat idle in the pool longer than the configured number of seconds are discarded on retrieval and a fresh connection is returned. Default 0 (disabled) matches the existing convention used by LoadBalanceTimeout and ConnectionLifetime. Covers both pool designs (ChannelDbConnectionPool, WaitHandleDbConnectionPool). Deferred to follow-up: proactive timer sweep (FR-008, FR-010) which the spec assumes is built on top of the pruning feature (#37338).
There was a problem hiding this comment.
Pull request overview
Adds a new pooling-related connection-string keyword, Connection Idle Timeout (synonym: Pool Idle Timeout), enabling lazy eviction of pooled connections that have sat idle longer than the configured number of seconds. This targets stale/half-open pooled connections (e.g., behind firewalls/load balancers) by discarding expired idle connections on the retrieval path in both pool implementations.
Changes:
- Introduces
Connection Idle Timeoutparsing +SqlConnectionStringBuilder.IdleTimeoutproperty (default0disables). - Tracks per-connection idle timestamp (
DbConnectionInternal.IdleSinceUtc) and stamps it on return-to-pool; evicts idle-expired connections on retrieval in both pool designs. - Adds unit/functional tests for the new builder keyword/property and Channel pool idle-expiry behavior.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml | Adds XML doc snippet for SqlConnectionStringBuilder.IdleTimeout. |
| src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.cs | Updates reference assembly surface with IdleTimeout property. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionStringKeywords.cs | Adds canonical keyword string Connection Idle Timeout. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionStringDefaults.cs | Adds default IdleTimeout = 0. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionStringSynonyms.cs | Adds synonym pool idle timeout. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | Adds IdleSinceUtc tracking + MarkPooledIdle() stamping helper. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Stamps idle time on return and adds idle-expiry check in IsLiveConnection. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolOptions.cs | Adds IdleTimeout option (as TimeSpan) to pool group options. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | Stamps idle time on return and adds IsIdleExpired check at retrieval sites. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Wires parsed idle-timeout option into pool group options creation. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionOptions.cs | Parses/validates idle-timeout integer from connection string. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs | Adds IdleTimeout keyword/property support + synonym mapping. |
| src/Microsoft.Data.SqlClient/src/Resources/Strings.resx | Adds localized description string for the new keyword/property. |
| src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs | Regenerates resource accessor for DbConnectionString_IdleTimeout. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | Adds unit tests for Channel pool idle-timeout behavior and stamping. |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs | Adds functional tests for keyword parsing, round-trip, default, and invalid values. |
Files not reviewed (1)
- src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:445
- The comment says that when
IdleSinceUtcis the default (DateTime.MinValue) the idle-timeout check is a no-op and indicates the connection has never been pooled. In this PRIdleSinceUtcis initialized toCreateTimeinDbConnectionInternal, so it will not beDateTime.MinValue, and even if it were, the current comparison would not be a no-op. Please update/remove this comment to match the actual semantics (e.g., describe the create-time initialization and that the check applies to any connection read from the idle channel).
// Connection has been sitting idle longer than the configured idle timeout.
// IdleSinceUtc is stamped by ReturnInternalConnection on each return; if it is the default
// (DateTime.MinValue), the connection has never been pooled yet and the check is a no-op.
TimeSpan idleTimeout = PoolGroupOptions.IdleTimeout;
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs:1351
- Idle-timeout enforcement was added to the legacy
WaitHandleDbConnectionPool(newIsIdleExpiredcheck +MarkPooledIdlestamping), but there doesn’t appear to be any unit test coverage exercising this path (existing idle-timeout unit tests are only forChannelDbConnectionPool). Please add at least one focused test validating that an idle-expired connection is discarded and replaced inWaitHandleDbConnectionPool, and that IdleTimeout==0 leaves behavior unchanged.
/// <summary>
/// Returns true when the supplied connection has been sitting idle in the pool longer than the
/// configured <see cref="DbConnectionPoolGroupOptions.IdleTimeout"/>. Returns false when idle timeout
/// is disabled (zero).
/// </summary>
private bool IsIdleExpired(DbConnectionInternal obj)
{
TimeSpan idleTimeout = PoolGroupOptions.IdleTimeout;
return idleTimeout != TimeSpan.Zero && DateTime.UtcNow > obj.IdleSinceUtc + idleTimeout;
}
| { | ||
| // Stamp the idle-since timestamp immediately before putting the connection back in the | ||
| // pool so that IsLiveConnection can later evict it if it sits idle past the configured limit. | ||
| connection.MarkPooledIdle(); |
|
|
||
| // Stamp the idle-since timestamp immediately before placing the connection on the idle stack | ||
| // so that idle-expiry checks on later retrieval can decide whether it has sat unused too long. | ||
| obj.MarkPooledIdle(); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4295 +/- ##
==========================================
- Coverage 66.04% 65.89% -0.16%
==========================================
Files 275 272 -3
Lines 42976 66035 +23059
==========================================
+ Hits 28383 43512 +15129
- Misses 14593 22523 +7930
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Fix stale comment in ChannelDbConnectionPool.IsLiveConnection: IdleSinceUtc is initialized to CreateTime, not DateTime.MinValue. - Add WaitHandleDbConnectionPoolIdleTimeoutTest mirroring the existing channel-pool idle-timeout coverage (stamp on return, zero disables expiry, expired connection is replaced, fresh connection is reused).
This pull request adds a new
Connection Idle Timeoutconnection-string keyword to Microsoft.Data.SqlClient, allowing users to control how long a pooled connection may remain idle before it is discarded on the next retrieval. This addresses #343, where firewalls or load balancers silently kill idle connections and applications then pull dead connections from the pool. Tracks ADO #39970.The implementation is based on the Idle Connection Timeout spec.
Configuration surface:
Connection Idle Timeout(in seconds), withPool Idle Timeoutas a synonym.IdleTimeoutproperty onSqlConnectionStringBuilder(int, seconds, default0). Negative values throw at parse time. A value of0disables idle expiration.Behavior:
ChannelDbConnectionPoolandWaitHandleDbConnectionPool).MinPoolSize: a caller always receives a working connection.Design decisions taken:
Connection Idle Timeoutis the canonical keyword;Pool Idle Timeoutis registered as a synonym.SqlConnection.IdleTimeoutproperty — the keyword is exposed only onSqlConnectionStringBuilder.0(idle expiration disabled).Out of scope / deferred to a follow-up PR:
MinPoolSizefloor on proactive removal. The spec requires this to share a timer with the pruning feature (#37338), which is not yet landed;ChannelDbConnectionPoolalso has no periodic timer today. Deferred to a follow-up that builds on the pruning work.Tests:
ChannelDbConnectionPoolTest.cscovering zero-disables, expiry on retrieval, recently-returned-not-expired, stamping on return, initial value, and "checked-out connections aren't affected".SqlConnectionStringBuilderTest.cscovering default value, canonical keyword parsing, synonym parsing, round-trip through the builder, and negative-value rejection. Mirrors the existingLoadBalanceTimeout/MaxPoolSize/MinPoolSizetest pattern.ChannelDbConnectionPoolwork itself was shipped.net9.0; clean build.Links: