-
Notifications
You must be signed in to change notification settings - Fork 331
Add connection-creation rate limiting to ChannelDbConnectionPool #4396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d3e2fe7
Add connection-creation rate limiting to ChannelDbConnectionPool
mdaigle 4396432
Narrow pool rate limiter to ConcurrencyLimiter
mdaigle 86b4b2a
Replace rate-limit TODOs with rationale comment
mdaigle 144a324
Remove rate-limit TODOs from AttemptAcquire call
mdaigle 029ce7e
Merge remote-tracking branch 'origin/main' into dev/mdaigle/pool-chan…
mdaigle 662b36d
Inline leaseAcquired into lease.IsAcquired checks
mdaigle 87c8631
Add test that successful create releases its rate-limiter lease
mdaigle 19948c2
Add test for lease-release wake path (FR-004)
mdaigle 0d60cd4
Address Copilot review: OCE handling, redundant wake, docs, test disp…
mdaigle 36e0a51
Merge remote-tracking branch 'origin/main' into dev/mdaigle/pool-chan…
mdaigle 000531b
Address Paul's review: doc/comment clarity and test consolidation
mdaigle 3e82378
Rewrite pool recovery test using TimeProvider seam
mdaigle 08ad22e
Clarify rate-limiter is scoped to a single pool
mdaigle a804da8
Capture lease.IsAcquired before disposing the lease
mdaigle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Rate limiting comparison | ||
|
|
||
| ## Existing rate limiting | ||
|
|
||
| ```mermaid | ||
| flowchart TD | ||
| Start([Open request]) --> WaitAny["WaitHandle.WaitAny<br/>(blocking, no queue)"] | ||
|
|
||
| WaitAny -->|idle available| S0["PoolSemaphore<br/>Semaphore 0..MAX"] | ||
| WaitAny -->|error state| S1["ErrorEvent<br/>ManualResetEvent"] | ||
| WaitAny -->|permit to open one conn| S2["CreationSemaphore<br/>Semaphore 1,1"] | ||
|
|
||
| S0 -->|got connection| Done([Return connection]) | ||
| S2 --> Open["Open physical connection"] | ||
| Open --> Release["Semaphore.Release 1"] | ||
| Release -->|got connection| Done | ||
|
|
||
| classDef prim fill:#bfdbfe,stroke:#1e3a8a,color:#111 | ||
| class WaitAny,S0,S1,S2,Open,Release prim | ||
| ``` | ||
|
|
||
| ## New rate limiting | ||
|
|
||
| ```mermaid | ||
| flowchart TD | ||
| Start([Open request]) --> Idle["Idle channel<br/>TryRead<br/>(non-blocking)"] | ||
|
|
||
| Idle -->|got connection| Done([Return connection]) | ||
| Idle -->|empty| Limiter["ConcurrencyLimiter<br/>AttemptAcquire 1<br/>(non-blocking)"] | ||
|
|
||
| Limiter -->|acquired lease| Open["Open physical connection"] | ||
| Limiter -->|not acquired| Channel["Idle channel<br/>await ReadAsync <br/>(FIFO queued)"] | ||
|
|
||
| Open --> Lease["RateLimitLease.Dispose"] | ||
| Lease --> |got connection| Done | ||
| Channel -->|loop on wake signal| Idle | ||
| Channel --> |got connection| Done | ||
|
|
||
| classDef prim fill:#bfdbfe,stroke:#1e3a8a,color:#111 | ||
| class Idle,Limiter,Open,Lease,Channel prim | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| # Feature Specification: Pool Rate Limiting and Blocking Period | ||
|
|
||
| **Feature Branch**: `dev/mdaigle/pool-rate-limit` | ||
| **Created**: 2026-05-19 | ||
| **Status**: Draft | ||
| **Input**: ADO Work Item 37824 — "Implement connection open rate limiting" | ||
|
|
||
| ## Description | ||
|
|
||
| Add rate limiting to `ChannelDbConnectionPool` to control how many physical connections can be | ||
| created concurrently. Without throttling, a burst of concurrent requests can trigger a login | ||
| storm against SQL Server. The implementation uses | ||
| `System.Threading.RateLimiting.ConcurrencyLimiter` from the BCL — no custom rate limiting | ||
| primitives are defined. | ||
|
|
||
| This feature also adds the `PoolBlockingPeriod` error state (fast-fail after a connection | ||
| creation failure) with exponential backoff recovery, matching the existing | ||
| `WaitHandleDbConnectionPool` behavior. | ||
|
|
||
| > Time spent waiting for the rate limiter counts against the caller's overall `ConnectTimeout` | ||
| > budget. `ReplaceConnection` (when implemented) MUST bypass the rate limiter: it already holds | ||
| > a pool slot and must not deadlock. | ||
|
|
||
| ## User Scenarios & Testing | ||
|
|
||
| ### User Story 1 — Throttled Connection Creation Under Burst Demand (P1) | ||
|
|
||
| The pool limits the number of simultaneous physical connection creation attempts. Callers that | ||
| cannot immediately create a connection do not queue on the limiter; they fall back to waiting on | ||
| the idle channel, where they are satisfied either by a returned connection or by a best-effort | ||
| wake when another caller releases its permit. The idle channel preserves FIFO order for returned | ||
| connections, but rate-limit retries are best-effort rather than strictly ordered. All waiting is | ||
| subject to the caller's `ConnectTimeout`. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** the pool has no idle connections and many callers request connections simultaneously, | ||
| **When** the concurrency limit is reached, **Then** additional callers wait until an in-flight | ||
| creation completes before starting their own. | ||
| 2. **Given** a caller is waiting for the rate limiter, **When** its `ConnectTimeout` elapses, | ||
| **Then** the caller receives a timeout error without ever attempting to create a connection. | ||
| 3. **Given** the rate limiter has available capacity, **When** a caller requests a new connection, | ||
| **Then** the create proceeds immediately with no added latency. | ||
| 4. **Given** a connection creation completes (success or failure), **When** the `RateLimitLease` | ||
| is disposed, **Then** the next waiting caller is allowed to proceed. | ||
|
|
||
| --- | ||
|
|
||
| ### User Story 2 — Blocking Period Fast-Fail on Connection Failure (P1) | ||
|
|
||
| When a connection creation attempt fails because the server is unreachable, the pool enters an | ||
| error state and immediately fails subsequent requests for a limited period, returning the cached | ||
| error. This prevents cascading timeouts when the server is down. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** a creation failure has occurred and blocking period is enabled, **When** a new | ||
| connection is requested within the blocking window, **Then** the request fails immediately | ||
| with the cached error. | ||
| 2. **Given** a creation failure has occurred and blocking period is enabled, **When** the | ||
| blocking window expires, **Then** the next request attempts fresh connection creation. | ||
| 3. **Given** `PoolBlockingPeriod=NeverBlock`, **When** a creation failure occurs, **Then** each | ||
| subsequent request independently attempts creation (no fast-fail). | ||
| 4. **Given** `PoolBlockingPeriod=Auto` connecting to an Azure SQL endpoint and a failure occurs, | ||
| **Then** no blocking period is applied (same as `NeverBlock`). | ||
| 5. **Given** `PoolBlockingPeriod=Auto` connecting to an on-premises SQL Server and a failure | ||
| occurs, **Then** the blocking period is applied (same as `AlwaysBlock`). | ||
|
|
||
| --- | ||
|
|
||
| ### User Story 3 — Error State Recovery with Exponential Backoff (P2) | ||
|
|
||
| While in the error state the pool waits using exponential backoff (5s → 10s → 20s → 30s → 60s | ||
| cap) before allowing the next attempt. Once an attempt after the backoff succeeds, the error | ||
| state clears and backoff resets. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** the pool is in error state, **When** the backoff timer fires and the next caller's | ||
| attempt succeeds, **Then** the error state is cleared and subsequent requests attempt normal | ||
| creation. | ||
| 2. **Given** the pool is in error state, **When** the backoff timer fires and the next caller's | ||
| attempt fails, **Then** the backoff interval increases (up to the 60s cap) and the pool | ||
| re-enters the error state. | ||
| 3. **Given** the pool is in error state, **When** the error is cleared, **Then** the cached | ||
| exception, the error flag, and the backoff interval are all reset. | ||
|
|
||
| --- | ||
|
|
||
| ### User Story 4 — Rate Limiting Counts Against Connection Timeout (P2) | ||
|
|
||
| Time spent waiting for rate limiter capacity counts against the caller's overall | ||
| `ConnectTimeout` budget. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** a caller's timeout is 15s and the caller waits 10s for rate limiting, **When** the | ||
| rate limiter releases, **Then** the remaining budget for connection creation is 5s. | ||
| 2. **Given** a caller's timeout expires while waiting for the rate limiter, **When** the timeout | ||
| fires, **Then** the caller receives a timeout error and is removed from the limiter queue. | ||
|
|
||
| --- | ||
|
|
||
| ### User Story 5 — Rate Limiting Built on a Concurrency Limiter (P3) | ||
|
|
||
| The pool supports an optional `System.Threading.RateLimiting.ConcurrencyLimiter` to throttle | ||
| concurrent physical connection creation. This is the only limiter type the pool currently needs | ||
| (pooling against on-prem SQL Server), so the pool takes a concrete `ConcurrencyLimiter?` rather | ||
| than the abstract `RateLimiter` base. Support for other limiter types can be added later if a | ||
| concrete need arises. When no limiter is supplied (`null`), no rate limiting is applied. | ||
|
|
||
| **Acceptance Scenarios**: | ||
|
|
||
| 1. **Given** the pool is configured with a `ConcurrencyLimiter`, **When** connections | ||
| are created, **Then** the limiter throttles concurrent creation to the configured maximum. | ||
| 2. **Given** no limiter is supplied (`null`), **When** connections are created, **Then** the | ||
| pool applies no rate limiting. | ||
|
|
||
| --- | ||
|
|
||
| ## Functional Requirements | ||
|
|
||
| - **FR-001**: The pool MUST limit the number of concurrent physical connection creation attempts | ||
| to a configurable maximum. | ||
| - **FR-002**: Callers that cannot immediately create a connection due to rate limiting MUST fall | ||
| back to waiting on the idle channel until capacity becomes available (via a returned connection | ||
| or a best-effort wake when a permit is released) or their timeout expires. Rate-limit retries are | ||
| best-effort and not strictly FIFO-ordered. | ||
| - **FR-003**: Time spent waiting for rate limiter capacity MUST count against the caller's | ||
| overall connection timeout budget. | ||
| - **FR-004**: When a connection creation attempt completes (success or failure), the | ||
| `RateLimitLease` MUST be disposed so the next waiting caller can proceed. | ||
| - **FR-005**: The pool MUST support three `PoolBlockingPeriod` modes: `Auto`, `AlwaysBlock`, and | ||
| `NeverBlock`. | ||
| - **FR-006**: When the blocking period is enabled, the pool MUST enter an error state after a | ||
| creation failure and immediately fail subsequent requests with the cached error. | ||
| - **FR-007**: When the blocking period is disabled, the pool MUST NOT enter an error state; | ||
| each request MUST independently attempt creation. | ||
| - **FR-008**: While in error state, the backoff MUST use exponential growth starting at 5s, | ||
| doubling each attempt, capped at 60s. | ||
| - **FR-009**: When an attempt succeeds, the pool MUST clear the error state and reset the | ||
| backoff to its initial value. | ||
| - **FR-010**: The `ErrorOccurred` property MUST return `true` when in the error state and | ||
| `false` otherwise. | ||
| - **FR-011**: `ClearPool` MUST clear the error state in addition to invalidating pooled | ||
| connections. | ||
| - **FR-012**: The rate limiter MUST be an optional `System.Threading.RateLimiting.ConcurrencyLimiter`. | ||
| When no limiter is supplied (`null`), the pool MUST apply no rate limiting. Support for other | ||
| `RateLimiter` types is intentionally out of scope for now and may be added later if needed. | ||
| - **FR-013**: When a limiter is supplied, it MUST be a | ||
| `System.Threading.RateLimiting.ConcurrencyLimiter` configured with the desired maximum number | ||
| of concurrent connection creation attempts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.