Dev/mdaigle/pool rate limit

.github/copilot-instructions.md

@@ -150,6 +150,24 @@ When a new issue is created, follow these steps:
 - Do not modify the `CODEOWNERS` file directly.
 - Do not modify `CHANGELOG.md` unless executing a release workflow (see `release-notes` prompt).
 - Do not close issues without a fix or without providing a clear reason.
+- **Check `.github/instructions/` for matching `applyTo` patterns BEFORE starting any file creation or modification task.** Apply all conventions from those files from the start—do not refactor later.
+
+## Instruction File Lookup (Required Workflow)
+
+Before creating or modifying any file, check if matching instruction files exist:
+
+1. Scan `.github/instructions/` for files with `applyTo` patterns that match your file path
+2. Read **all** matching instruction files completely before writing code
+3. Apply **all** conventions, patterns, and requirements from the start
+4. Do not skip instructions or plan to refactor later
+
+**Example:** Creating `src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/NewTest.cs`
+- File matches `applyTo: "**/tests/**,**/*Test*.cs"` from `testing.instructions.md`
+- Must read `testing.instructions.md` before writing the first test method
+- Must apply AAA pattern, XML doc comments, and all testing conventions from the start
+- Do not create tests first and add documentation later
+
+This ensures consistency across the codebase and prevents rework.
 
 ## Terminal Execution Safety
 - Treat any non-zero shell exit code as a failed step that requires correction before proceeding.

.github/instructions/testing.instructions.md

@@ -189,10 +189,15 @@ class and method level.
 #### What the Comments Must Explain
 - The behavior/contract being tested (not just restating the method name).
 - Why the scenario matters (for example: regression guard, parsing contract, sync/async parity,
-  isolation requirement).
+  isolation requirement, thread-safety validation, edge case handling).
 - For helper methods, what side effects occur (for example console redirection, file system
   copying, process execution) and why they are needed.
 
+#### Implementation Guidance
+- Always use the **Arrange-Act-Assert (AAA)** pattern with explicit section comments (see [Test Structure](#test-structure-arrange-act-assert-aaa-pattern)).
+- Keep the test body focused on a single logical assertion; avoid testing multiple unrelated behaviors.
+- Extract complex setup into helper methods or fixtures rather than embedding it in the test.
+
 #### Style Guidance
 - Keep comments concise and factual.
 - Prefer behavior-focused wording over implementation trivia.
@@ -212,25 +217,41 @@ public void AppRunWithMalformedConnectionStringReturnsOneAndWritesParseError()
 }
 ```
 
-### Test Structure
+### Test Structure: Arrange-Act-Assert (AAA) Pattern
+
+All test methods **must** follow the **Arrange-Act-Assert (AAA)** pattern with explicit section comments.
+This pattern improves readability and maintainability by clearly delineating setup, execution, and validation.
+
+**Pattern:**
 ```csharp
-public class FeatureNameTests
+[Fact]
+public void MethodName_Scenario_ExpectedResult()
 {
-    [Fact]
-    public void MethodName_Scenario_ExpectedResult()
-    {
-        // Arrange
-        var sut = new SystemUnderTest();
-
-        // Act
-        var result = sut.PerformAction();
-
-        // Assert
-        Assert.Equal(expected, result);
-    }
+    // Arrange
+    // Set up test fixtures, initial state, dependencies, and test data
+    var sut = new SystemUnderTest();
+    var input = new TestData();
+
+    // Act
+    // Execute the code under test
+    var result = sut.PerformAction(input);
+
+    // Assert
+    // Validate outcomes and expectations
+    Assert.Equal(expected, result);
 }
 ```
 
+**Benefits:**
+- Clear visual separation of setup, execution, and validation
+- Easier to identify test logic flow
+- Simpler to debug failing tests (know which section failed)
+- Consistent convention across codebase
+- Improves AI agent comprehension and test generation
+
+**Guideline:** Each section should be visually distinct. If AAA sections become too large (e.g., Arrange > 20 lines),
+consider extracting helper methods or fixtures rather than embedding complexity in the test method itself.
+
 ### Naming Conventions
 - Test class: `{ClassName}Tests`
 - Test method: `{Method}_{Scenario}_{ExpectedBehavior}`

Directory.Packages.props

@@ -150,13 +150,15 @@
   <ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))">
     <PackageVersion Include="Microsoft.Bcl.Cryptography" Version="9.0.13" />
     <PackageVersion Include="Microsoft.Bcl.TimeProvider" Version="9.0.16" />
+    <PackageVersion Include="System.Threading.RateLimiting" Version="9.0.13" />
     <PackageVersion Include="System.Configuration.ConfigurationManager" Version="9.0.13" />
     <PackageVersion Include="System.Security.Cryptography.Pkcs" Version="9.0.13" />
   </ItemGroup>
 
   <ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))">
     <PackageVersion Include="Microsoft.Bcl.Cryptography" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Bcl.TimeProvider" Version="8.0.1" />
+    <PackageVersion Include="System.Threading.RateLimiting" Version="8.0.0" />
     <PackageVersion Include="System.Configuration.ConfigurationManager" Version="8.0.1" />
     <PackageVersion Include="System.Security.Cryptography.Pkcs" Version="8.0.1" />
   </ItemGroup>

specs/006-pool-rate-limiting/diagrams.md

@@ -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["RateLimiter<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
+```

specs/006-pool-rate-limiting/spec.md

@@ -0,0 +1,146 @@
+# 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 wait in FIFO order until the limiter allows them to
+proceed, subject to their `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 Limiter Built on System.Threading.RateLimiting (P3)
+
+The pool uses `System.Threading.RateLimiting.RateLimiter` as the base abstraction and
+`ConcurrencyLimiter` as the initial implementation. No custom rate limiting primitives are
+defined.
+
+**Acceptance Scenarios**:
+
+1. **Given** the pool is configured with the default `ConcurrencyLimiter`, **When** connections
+   are created, **Then** the limiter throttles concurrent creation to the configured maximum.
+2. **Given** a different `RateLimiter` implementation is substituted, **When** connections are
+   created, **Then** the pool delegates throttling to the substituted implementation without
+   code changes to pool logic.
+
+---
+
+## 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 wait
+  in FIFO order until capacity is available or their timeout expires.
+- **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 use `System.Threading.RateLimiting.RateLimiter` as the base
+  abstraction so that any `RateLimiter` implementation can be substituted without modifying
+  pool acquisition logic.
+- **FR-013**: The initial implementation MUST use
+  `System.Threading.RateLimiting.ConcurrencyLimiter` configured with the desired maximum number
+  of concurrent connection creation attempts.

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj

@@ -309,6 +309,7 @@
     <PackageReference Include="System.Security.Cryptography.Pkcs" />
     <PackageReference Include="System.Text.Json" />
     <PackageReference Include="System.Threading.Channels" />
+    <PackageReference Include="System.Threading.RateLimiting" />
     <PackageReference Include="System.ValueTuple" />
   </ItemGroup>
 
@@ -321,6 +322,7 @@
     <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
     <PackageReference Include="System.Configuration.ConfigurationManager" />
     <PackageReference Include="System.Security.Cryptography.Pkcs" />
+    <PackageReference Include="System.Threading.RateLimiting" />
   </ItemGroup>
 
   <!-- CodeGen Targets ================================================= -->

src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs

@@ -175,6 +175,27 @@ internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, Ti
             }
         }
 
+        internal static ITimer UnsafeCreateTimer(
+            TimeProvider timeProvider,
+            TimerCallback callback,
+            object state,
+            TimeSpan dueTime,
+            TimeSpan period)
+        {
+            // Don't capture the current ExecutionContext and its AsyncLocals onto
+            // a timer causing them to live forever. Honor the supplied TimeProvider
+            // so callers can inject a test double for deterministic scheduling.
+            if (ExecutionContext.IsFlowSuppressed())
+            {
+                return timeProvider.CreateTimer(callback, state, dueTime, period);
+            }
+
+            using (ExecutionContext.SuppressFlow())
+            {
+                return timeProvider.CreateTimer(callback, state, dueTime, period);
+            }
+        }
+
 
 #region COM+ exceptions
         internal static ArgumentException Argument(string error)