Refactor for modular connection pool #3199

mdaigle · 2025-03-05T00:37:00Z

Introduces an abstract class to capture the API of the current connection pool implementation. Gathers pooling related classes into a single namespace (Microsoft.Data.SqlClient.ConnectionPool) and folder. These changes set the stage for swappable connection pool implementations. Targeting main because these are good standalone changes for code cleanliness. Future changes will target a feature branch.

This work is tracking against #25

PoolBlockingPeriod is not moved because it is public. Moving it's location/namespace would be a breaking API change. It may still make sense to switch its namespace if this can go in with a major version bump.

…ction-pool

mdaigle · 2025-03-05T00:42:01Z

/azp run

azure-pipelines · 2025-03-05T00:42:14Z

Azure Pipelines successfully started running 1 pipeline(s).

codecov · 2025-03-06T01:23:31Z

Codecov Report

Attention: Patch coverage is 84.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 65.96%. Comparing base (3f4e486) to head (b7a321e).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...lient/ConnectionPool/WaitHandleDbConnectionPool.cs	82.35%	12 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (3f4e486) and HEAD (b7a321e). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (3f4e486) HEAD (b7a321e)

addons 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3199      +/-   ##
==========================================
- Coverage   72.89%   65.96%   -6.94%     
==========================================
  Files         288      283       -5     
  Lines       59246    59209      -37     
==========================================
- Hits        43190    39055    -4135     
- Misses      16056    20154    +4098

Flag	Coverage Δ
addons	`?`
netcore	`68.82% <83.09%> (-6.69%)`	⬇️
netfx	`64.87% <84.00%> (-6.41%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

PR Overview

This PR refactors the connection pool implementation to be more modular by introducing an abstract DbConnectionPool class and moving pooling-related classes into the Microsoft.Data.SqlClient.ConnectionPool namespace.

Introduces a new abstract class (DbConnectionPool) that captures the current connection pool API.
Migrates several classes from Microsoft.Data.ProviderBase to Microsoft.Data.SqlClient.ConnectionPool.
Updates pool instantiation in DbConnectionPoolGroup to use WaitHandleDbConnectionPool, supporting modular and swappable implementations.

Reviewed Changes

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPool.cs	Adds a new abstract base class for connection pooling API.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolCounters.netfx.cs	Updates the namespace to the new connection pool namespace.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs	Adapts references from the old provider base pool group to the new connection pool group.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolIdentity.Unix.cs	Updates namespace from ProviderBase to SqlClient.ConnectionPool.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolAuthenticationContextKey.cs	Updates namespace from ProviderBase to SqlClient.ConnectionPool.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroupProviderInfo.cs	Updates namespace from ProviderBase to SqlClient.ConnectionPool.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolAuthenticationContext.cs	Updates namespace from ProviderBase to SqlClient.ConnectionPool.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs	Modifies pool instantiation to use WaitHandleDbConnectionPool.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs	Incorporates the new connection pool namespace in factory logic.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs	Updates pool group references to use the new connection pool namespace.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs	Updates pool group references to use the new connection pool namespace.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs	Adapts using statements to include the new connection pool namespace.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs	Updates namespace references accordingly.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs	Updates namespace to use the new connection pool namespace.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs	Updates namespace to include the new connection pool namespace.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs	Updates namespace imports accordingly.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs	Updates namespace imports accordingly.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs	Updates namespace imports accordingly.

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPoolGroup.cs:190

Please ensure that the new WaitHandleDbConnectionPool implementation is thoroughly tested to validate its initialization, cleanup, and overall behavior in the connection pool lifecycle.

DbConnectionPool newPool = new WaitHandleDbConnectionPool(connectionFactory, this, currentIdentity, connectionPoolProviderInfo);

benrr101

Overall I like these changes. Mostly stylistic changes I'm requesting, a couple bigger questions around what the longer term plan is.

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionHelper.cs

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/DbConnectionPool.cs

...oft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

...soft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionPoolHelper.cs

benrr101

Looks much better now, thanks!

mdaigle added 8 commits March 4, 2025 11:49

Add abstract class to define pool interface.

eb423fd

Code cleanup

306efa5

Refactoring pool state and cross-instance access.

e2b9615

Rename files

ad1fa75

Rename abstract base back to DbConnectionPool

2cd3a58

Reorganize pool files.

a99ca38

Fix imports

7cbb506

Merge branch 'main' of github.com:dotnet/SqlClient into modular-conne…

3d34293

…ction-pool

mdaigle changed the title ~~Modular connection pool~~ Refactor for modular connection pool Mar 5, 2025

Fix pooling test reflection

3f434bf

mdaigle changed the base branch from main to dev/mdaigle/connection-pool-perf March 6, 2025 19:45

mdaigle marked this pull request as ready for review March 6, 2025 19:45

mdaigle changed the base branch from dev/mdaigle/connection-pool-perf to main March 6, 2025 19:47

mdaigle requested a review from Copilot March 6, 2025 20:06

Copilot AI reviewed Mar 6, 2025

View reviewed changes

benrr101 requested changes Mar 6, 2025

View reviewed changes

Address review comments

b7a321e

mdaigle requested a review from a team March 6, 2025 23:12

benrr101 approved these changes Mar 7, 2025

View reviewed changes

cheenamalhotra approved these changes Mar 7, 2025

View reviewed changes

mdaigle added this to the 7.0-preview1 milestone Mar 7, 2025

mdaigle merged commit 91f238c into dotnet:main Mar 7, 2025
123 checks passed

mdaigle deleted the modular-connection-pool branch March 7, 2025 17:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor for modular connection pool #3199

Refactor for modular connection pool #3199

Uh oh!

mdaigle commented Mar 5, 2025 •

edited

Loading

Uh oh!

mdaigle commented Mar 5, 2025

Uh oh!

azure-pipelines bot commented Mar 5, 2025

Uh oh!

codecov bot commented Mar 6, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

benrr101 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benrr101 left a comment

Uh oh!

Uh oh!

Uh oh!

Refactor for modular connection pool #3199

Refactor for modular connection pool #3199

Uh oh!

Conversation

mdaigle commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdaigle commented Mar 5, 2025

Uh oh!

azure-pipelines bot commented Mar 5, 2025

Uh oh!

codecov bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdaigle commented Mar 5, 2025 •

edited

Loading

codecov bot commented Mar 6, 2025 •

edited

Loading