Cleanup | Split out TransactedConnectionPool

[mdaigle]

Description

Moves the TransactedConnectionPool class out from WaitHandleDbConnectionPool. Modernizes the class, adds doc comments, and adds unit test coverage. These tests highlight some deficiencies with how transactions are currently managed in the WaitHandleDbConnectionPool. I comment on these issues, but do not address them in this PR.

No logic changes are made to TransactedConnectionPool in this PR.

Changes should be reviewed commit by commit.

Testing

Adds unit testing.

[Copilot]

Pull Request Overview

This PR refactors the connection pool infrastructure by extracting the TransactedConnectionPool class from WaitHandleDbConnectionPool.cs into a standalone file and adds comprehensive unit test coverage for three connection pool components:

• TransactedConnectionPool: Extracted to a separate file with enhanced documentation
• ConnectionPoolSlots: New unit tests for capacity management
• ChannelDbConnectionPool: New unit tests for connection pooling behavior

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
TransactedConnectionPool.cs	Extracted from WaitHandleDbConnectionPool.cs with improved XML documentation, nullable annotations, and modernized property syntax
WaitHandleDbConnectionPool.cs	Removed TransactedConnectionPool and TransactedConnectionList nested classes (244 lines removed)
netfx/src/Microsoft.Data.SqlClient.csproj	Added reference to new TransactedConnectionPool.cs file
netcore/src/Microsoft.Data.SqlClient.csproj	Added reference to new TransactedConnectionPool.cs file
TransactedConnectionPoolTest.cs	Added 762 lines of comprehensive unit tests covering constructor, property, method, and integration scenarios
ConnectionPoolSlotsTest.cs	Added 481 lines of unit tests for capacity and reservation management
ChannelDbConnectionPoolTest.cs	Added 912 lines of unit tests for connection lifecycle and pooling behavior

[edwardneal]

No objections to the cleanup, (apart from one missing debug assertion) but I've got a few comments for the pile when you start looking at the overall design.

[paulmedynski]

Resolved some comments, others remain.

[mdaigle]

@paulmedynski just pushed changes addressing your comments

[paulmedynski]

One comment left!

[Copilot]

Pull Request Overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.

[codecov]

Codecov Report

❌ Patch coverage is 94.18605% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.57%. Comparing base (a19cec4) to head (a52142b).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...lClient/ConnectionPool/TransactedConnectionPool.cs	94.18%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3746      +/-   ##
==========================================
- Coverage   76.70%   76.57%   -0.14%     
==========================================
  Files         272      273       +1     
  Lines       44177    44180       +3     
==========================================
- Hits        33885    33829      -56     
- Misses      10292    10351      +59     

Flag	Coverage Δ
addons	90.82% <ø> (ø)
netcore	76.60% <94.18%> (-0.18%)	⬇️
netfx	76.29% <94.18%> (-0.06%)	⬇️

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.