[Design reference — not for merge] Pump-based, fast-path-free ChannelDbConnectionPool

[mdaigle]

Status: design reference, not intended to merge

This PR is kept open as a documented design alternative for the rate-limiting work (originally explored for #4396). It implements a pump-based, fast-path-free connection pool. After evaluation we decided not to adopt it, because the rate limiting we actually need is a concurrency limiter, which does not have the problem this design solves. See Why it isn't suitable below. Reference this PR from the rate-limiting PR to show what was considered and why it was rejected.

What it does

Reworks ChannelDbConnectionPool so that every request queues on the idle/creation channel — there is no optimistic fast path. Physical connection creation is decoupled from the requesting caller and runs on background pump tasks that grow the pool as demand and capacity allow. Creation results travel over the channel via a new CreateOutcome payload (connection | error | bare wake).

• New CreateOutcome payload carried over the channel. default is a bare wake that nudges a parked waiter to re-evaluate.
• IdleConnectionChannel widened to carry CreateOutcome; its Count tracks connection-bearing outcomes only.
• New accounting (_waiterCount, _pendingCreates) plus TryPumpCreate (cheap synchronous demand+capacity gate) and LaunchCreate (background Task.Run create that publishes its result to the channel).
• GetInternalConnection rewritten: a warm TryRead still serves buffered connections fast; on a miss the request registers demand (_waiterCount++), pumps, then blocks on the channel until an outcome arrives.

Why this design exists

It targets a generic / time-based rate limiter (e.g. token bucket). With a time-based limiter, tokens replenish on a timer, so there is no event to signal a parked waiter when capacity becomes available. The existing pool relies on writing a bare wake to the channel whenever a slot frees (connection returned/removed); a timer-replenished limiter has no such moment, so a waiter can starve (a lost-wakeup problem). The pump solves this by continuously re-driving demand against capacity from background tasks and a _waiterCount/_pendingCreates gate, independent of any per-connection event.

Why it isn't suitable

The rate limiting we actually need (pooling against on-prem SQL Server) is a concurrency limiter, not a time-based one. A concurrency limiter does not have the lost-wakeup problem: token availability is lease-release, which is a concrete event. That maps directly onto the pool's existing "write a wake to the channel when a slot frees" model — the limiter simply writes a wake on lease-release. So the central problem this PR solves does not exist for our use case, and the pump's machinery buys us nothing on that axis.

Meanwhile, adopting the pump has real costs relative to the current (caller-creates-inline) model:

1. Error precision is downgraded. Today the caller that triggers a create receives its own creation error. The pump delivers a captured error to whichever waiter is at the FIFO head — "a caller", not necessarily "the caller". Failed creates with no waiter are trace-logged and swallowed. This makes diagnosing a specific failed open harder.
2. Sync path regresses. Today a sync caller that must create a connection does so inline on its own thread. Under the pump, sync opens are routed through ReadChannelSyncOverAsync (sync-over-async, with a thread-pool hop) — strictly more overhead for the pure-sync path.
3. Significant added complexity on a hot path. ~440 lines including _waiterCount/_pendingCreates accounting and a subtle lost-wakeup ordering argument (waiter increments _waiterCount before parking; slot-freeing events free the slot before reading _waiterCount). This is a lot of concurrency-critical surface area to maintain.
4. Its independent upsides aren't wanted. The two benefits the pump offers beyond rate limiting are strict FIFO fairness and parallel connection creation. For our use case, best-effort ordering is sufficient (strict FIFO not required) and warmup is intentionally serial (parallel creation is explicitly not wanted). So neither upside applies.

Conclusion

For a concurrency limiter, integrate AcquireAsync/lease-release directly into the existing pool's create path and write a wake to the channel on lease-release — no pump required. This preserves per-caller error precision and the inline sync path, and avoids the added complexity. This PR remains as a reference for what a time-based-limiter design would have looked like, should that need ever arise.

Testing (for the record)

• ChannelDbConnectionPoolTest / IdleConnectionChannelTest updated for CreateOutcome; added pump-behavior tests (parallel creates up to max, sync + async creation-error delivery, doomed-return unblocks waiter).
• All 240 ConnectionPool unit tests pass. The only failing unit tests in the suite are pre-existing, environment-specific ones (AlwaysEncrypted cert store + integrated auth) that also fail on a clean checkout.

Note: CHANGELOG.md intentionally not edited. No public API changes.

[Copilot]

Pull request overview

This PR refactors ChannelDbConnectionPool to a pump-based model where all connection requests queue through a single idle/creation channel, enabling strict FIFO ordering for contended waiters and moving physical connection creation onto background pump tasks.

Changes:

• Introduces CreateOutcome (connection | error | bare wake) and widens the idle channel to carry outcomes instead of nullable connections.
• Reworks ChannelDbConnectionPool.GetInternalConnection to pump connection creation on background tasks and deliver results/errors via the channel.
• Updates/extends connection pool unit tests to validate pump behavior and CreateOutcome semantics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Implements the pump-based queuing/creation model and error delivery via CreateOutcome.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs	Changes the channel payload to CreateOutcome and updates count semantics to track only connection-bearing outcomes.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/CreateOutcome.cs	Adds the new outcome payload type used to convey connection/error/wake signals through the channel.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs	Adds pump-behavior tests (parallel create, error delivery, doomed-return wakeup) and updates timeout expectations for pump creation.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/IdleConnectionChannelTest.cs	Updates idle channel tests to use CreateOutcome and validate count behavior across outcomes.