Implement ChannelDbConnectionPool.Clear#4194
Conversation
There was a problem hiding this comment.
Pull request overview
Adds “pool clear” behavior for the new channel-based connection pool implementation (pool v2) so that SqlConnection.ClearPool() / SqlConnection.ClearAllPools() can invalidate pooled connections using a generation-counter approach.
Changes:
- Implemented
ChannelDbConnectionPool.Clear()and added per-connectionClearGenerationstamping/validation to lazily invalidate busy connections. - Enabled pool v2 creation via
DbConnectionPoolGroupand updated pooled-connection creation to usepool.PoolGroupmetadata. - Added/updated unit + manual tests and introduced a draft spec describing expected clear semantics.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | Replaces “Clear not implemented” test with a suite validating clear + generation behavior. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs | Extends ClearAllPoolsTest to run under both pool v1 and v2 via an AppContext switch helper. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs | Updates reflection helpers to recognize channel pool and use interface-based Count. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Simplifies pooled connection creation to use pool.PoolGroup options/key. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | Updates call to CreatePooledConnection to match new signature. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs | Switches pool v2 branch from throwing to instantiating ChannelDbConnectionPool. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Implements Clear() and enforces generation checks in liveliness evaluation + new connection stamping. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | Adds ClearGeneration property used by channel pool to detect stale connections. |
| specs/001-pool-clear/spec.md | Adds a draft feature spec documenting scenarios/requirements for pool clear. |
| specs/001-pool-clear/checklists/requirements.md | Adds a requirements checklist for the spec. |
There was a problem hiding this comment.
Pull request overview
Implements pool clearing for the channel-based connection pool (pool v2) so that SqlConnection.ClearPool() / SqlConnection.ClearAllPools() can invalidate pooled connections when UseConnectionPoolV2 is enabled.
Changes:
- Added a generation-based
Clear()implementation toChannelDbConnectionPool, plus idle-connection tracking via a newIdleConnectionChannel. - Extended
IDbConnectionPoolwithIdleCountand implemented it inWaitHandleDbConnectionPoolandChannelDbConnectionPool. - Updated/add tests and manual test plumbing to validate clear semantics and to support running ClearAllPools tests against both pool implementations.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs | Updates test mock to satisfy the extended IDbConnectionPool interface (IdleCount). |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleChannelTest.cs | Adds unit tests validating idle-channel counting and read/write behavior. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | Updates expectations (e.g., ErrorOccurred) and adds clear-focused unit tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs | Runs ClearAllPools coverage for both pool v1 and pool v2 via an AppContext switch helper. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs | Updates reflection helpers to use interface properties (Count/IdleCount) and handle channel pool cleanup differences. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Simplifies pooled-connection creation to use pool.PoolGroup instead of passing redundant options/key args. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | Implements IdleCount and updates internal usage to use the new property. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleChannel.cs | Introduces IdleConnectionChannel to wrap an unbounded channel and track non-null idle count. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs | Adds IdleCount to the pool interface. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs | Enables constructing ChannelDbConnectionPool when UseConnectionPoolV2 is enabled. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Implements Clear() with a generation counter; adds idle tracking and clears stale connections on return/retrieval. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | Adds ClearGeneration tracking on internal connections for pool v2 stale detection. |
| specs/001-pool-clear/spec.md | Adds a feature spec for pool clear behavior and acceptance scenarios. |
| specs/001-pool-clear/checklists/requirements.md | Adds a spec quality checklist for the new feature spec. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4194 +/- ##
==========================================
- Coverage 65.99% 64.41% -1.58%
==========================================
Files 276 272 -4
Lines 42951 65812 +22861
==========================================
+ Hits 28344 42395 +14051
- Misses 14607 23417 +8810
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:
|
paulmedynski
left a comment
There was a problem hiding this comment.
Looks good overall, with mostly questions to be answered.
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Implements IDbConnectionPool.Clear() for the channel-based pooling implementation (pool v2) using a generation-counter approach, and updates tests/docs to validate pool clearing behavior.
Changes:
- Add
Clear()implementation toChannelDbConnectionPooland introduceIdleConnectionChannelto track idle-connection counts for an unbounded channel. - Extend
IDbConnectionPoolwithIdleCountand update v1 pool + test infrastructure accordingly. - Add/adjust unit tests and wire
ClearAllPoolsmanual test to run under both pool versions; update docs and add a feature spec.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Implements Clear() via generation counter; integrates IdleConnectionChannel; adjusts pool properties/behavior. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs | New helper wrapping Channel<T> with an atomic non-null idle count. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs | Adds IdleCount and expands Clear() remarks. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs | Implements IdleCount; uses new SqlConnectionFactory.CreatePooledConnection signature. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs | Enables creating ChannelDbConnectionPool when UseConnectionPoolV2 is set. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs | Simplifies CreatePooledConnection to use pool group state from the pool instance. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs | Adds ClearGeneration property used by pool v2 to detect stale connections after clears. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | Updates tests for ErrorOccurred and adds coverage for pool v2 Clear() behavior. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleConnectionChannelTest.cs | New unit tests validating IdleConnectionChannel count semantics (sync/async/concurrent). |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/TransactedConnectionPoolTest.cs | Updates IDbConnectionPool mock to include IdleCount. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs | Runs ClearAllPoolsTest under both pool v1 and v2 via LocalAppContextSwitchesHelper. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs | Switches “free connection count” reflection to use IDbConnectionPool.IdleCount and handles v2 cleanup differences. |
| doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml | Improves ClearAllPools/ClearPool remarks and adds a caution note. |
| specs/001-pool-clear/spec.md | Adds a draft feature spec describing pool-clear behavior and acceptance scenarios. |
benrr101
left a comment
There was a problem hiding this comment.
Couple nits, just one question
Description
Adds an implementation for
IDbConnectionPool.Clear()toChannelDbConnectionPool.Testing
Adds new unit tests and hooks into existing manual tests.