Lite: a server that just went down no longer holds a connection slot while its retries wait (#4722)

[erikdarlingdata]

Fixes #4722

Why

Lite's CreateConnectionAsync held one of the seven s_connectionThrottle slots across the whole RetryHelper loop: up to four opens, each up to the connect timeout, plus 1 + 2 + 4 seconds of backoff between them. The dead-server errors (-2, -1, 2, 53, 10060, 10061) are on the transient list in PerformanceMonitor.Common/SqlErrorClassification.cs, so a server that had gone down since its last status check kept a slot through all of that, for every collector that tried it. Seven of those at once stopped collection for every server.

This is the Lite side of #4710. Lite's scheduler already skips servers marked offline and it has no runtime to drop on a -2 timeout, so nothing else from that issue applies here.

What changes

Lite/Services/RemoteCollectorService.cs:

• New internal static ExecuteThrottledWithRetryAsync<T>(SemaphoreSlim throttle, Func<Task<T>> attempt, ILogger? logger, string operationName, CancellationToken cancellationToken). CreateConnectionAsync runs its connect attempt through it with s_connectionThrottle. The semaphore is a parameter so the tests use one of their own; the production one is process-wide and the suite runs classes in parallel.
• The throttle is taken at the top of every attempt and released in a finally, so the backoff RetryHelper waits out between attempts holds no slot. A wait for a slot that is cancelled releases nothing, because nothing was held.
• The interactive sign-in lock is still taken first and held outside, as before. The attempt body (the device-code prompt and the open) stays inside the throttle, so a slot is held before any window shows. The throttle is not narrowed to OpenAsync.
• The connection string is resolved before the throttle is taken instead of inside it. It is a local credential read, not a login, and inside a per-attempt throttle it would be re-resolved on every retry.
• The doc comment on s_connectionThrottle is reworded for the per-attempt hold.
• A connection whose open fails is now disposed, as OpenAzureDatabaseConnectionAsync already does; before, each failed attempt left one undisposed, up to four per collector per cycle against a server that is down. The catch in the retry lambda is no longer filtered on the server being interactive: the interactive branch still records a declined sign-in exactly as before, and then every failed attempt disposes its connection before rethrowing. A successful attempt still returns the open connection undisposed.
• Each attempt now creates its connection after the device-code prompt starts, so an EntraDeviceCodeAuth.Begin that throws leaves no connection behind.

The change is four commits. The first extracts the function with today's shape (the throttle around the whole retry loop), so the new test could be run against the old behavior. The second is the throttle fix. The third disposes the connection of a failed attempt. The fourth moves the connection's creation below the device-code prompt.

Test plan

New Lite.Tests/ConnectionThrottlePerAttemptTests.cs, 9 tests:

• TheSlotIsFree_WhileTheRetryBackoffWaits: a logger sees the "Retrying in" line, reads the semaphore's CurrentCount, then cancels so the test skips the delay.
• EveryAttempt_RunsHoldingASlot: refused once with 10061, then opens; both attempts read 6 of 7 free from inside (one real second of backoff).
• ASuccessfulAttempt_HoldsASlotWhileItRuns_AndReleasesItOnReturn: 6 of 7 inside the attempt, 7 of 7 after. This one also fails a fix that stops taking the throttle at all.
• ACancelledAttempt_ReleasesItsSlot, APermanentFailure_ReleasesItsSlot_AndIsNotRetried, ACancelledWaitForASlot_ReleasesNothing (0 of 7 free while queued, 7 of 7 after the slots come back).
• CreateConnectionAsync_TakesTheSignInLockFirst_ThenRunsItsAttemptsThroughTheThrottle: a source pin for the wiring, the sign-in lock before the throttle and the device-code prompt inside it.
• AFailedOpen_DisposesItsConnection_ForEveryKindOfServer: a source pin over the code of CreateConnectionAsync (comments and strings blanked). The catch around the open is not filtered, connection.Dispose(); sits at the catch block's own depth (not only inside the interactive branch) and before the rethrow, and the success path returns connection without disposing it.
• TheTestException_IsATransientSqlError: checks the reflection-built SqlException really carries the transient numbers, so the retry tests can't pass for the wrong reason.

Results:

• dotnet build Lite.Tests/Lite.Tests.csproj -c Debug and dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: 0 warnings, 0 errors.
• The 8 throttle tests against the first commit (today's shape): 1 failed, TheSlotIsFree_WhileTheRetryBackoffWaits, with "the throttle read 6 of 7 free while RetryHelper waited out its backoff; a slot is held across the delay". The other 7 passed.
• The same 8 tests against the throttle fix: 8 passed, the backoff read 7 of 7.
• The disposal pin against the throttle fix, before the disposal change: 9 tests, 1 failed, AFailedOpen_DisposesItsConnection_ForEveryKindOfServer, on the catch clause catch (Exception ex) when (isInteractiveServer) being filtered.
• The same class after the disposal change: 9 passed.
• dotnet build of Lite, Lite.Tests and Darling.Tests: 0 warnings, 0 errors.
• Lite classes that read RemoteCollectorService.cs (EntraDeviceCodeTests, CollectorGateSurfacePinTests, EmptyEnumerationNoteTests, RefreshCollectionGateTests, TimeHonestyRungTests, MeasurementContractCensusTests, LockTimeoutYieldTests, AzureMasterFallbackTests), after the disposal change: 141 passed.
• Darling classes that read the same file (RemoteCollectorServiceCancellationCensusTests, CollectorStateContractTests, CollectorMeasurementSeamTests, LiteJobHistorySourceTests), after the disposal change: 48 run, 0 failed, 1 skipped.
• Full Lite.Tests at the throttle fix (before the disposal change): Total 5593, Failed 0, Skipped 0.
• Full Lite.Tests after the disposal change: not re-run here; CI runs it.
• Not run: a connect against a real server that is down. The tests use a semaphore of their own and an attempt that fails with the driver's own -2 and 10061 errors.

CHANGELOG

SECTION: Fixed
ENTRY:

• In Lite, a server that just went down no longer holds a connection slot while its retries wait ([Lite: a server that just went down holds connection-throttle slots through its retry backoff #4722]) - Each connect attempt now takes a slot and gives it back when the attempt ends, so a server that went down since its last status check no longer holds a slot through four connect timeouts and seven seconds of backoff, and can't stall collection for the other servers that way.
  REF:
  [Lite: a server that just went down holds connection-throttle slots through its retry backoff #4722]: Lite: a server that just went down holds connection-throttle slots through its retry backoff #4722