Revert "Fix Sentinel connection leak and AbortOnConnectFail=false handling"

[cbbp4pjf4j-png]

Reverts #3187

[mgravell]

I'm going to need more detail than "reverts". What is the motivation? What problem are you seeing?

Without that context, this is unmergeable and I'll have no option other than to close it. With that context, I suspect the real fix is some compromise that solves both problems/scenarios, one of which is currently unstated.

[mgravell]

I am genuinely interested in what is happening here, but I can't do anything until I understand that context. Consequently, I couldn't merge this for today's build. Please let me know what you're seeing, so I can consider it for next!

[mgravell]

Since there's still no description, I've tried to work out what this revert could be fixing by looking at what #3187 actually changes and measuring the affected paths against a throwaway redis + replica + sentinel.

Who is affected. With abortConnect=true (the default), #3187 changes nothing except that the leaked multiplexers are now disposed. Every behavioural change is confined to abortConnect=false, or to code that calls SentinelConnect + GetSentinelMasterConnection directly. So if this bit you, I'd expect you're in one of those two groups.

What I measured (abortConnect=false):

• Sentinel port refused at startup: Connect used to throw; it now returns in ~60ms with IsConnected=false and LastException set, and operations fail with a connection exception until sentinels appear. Once a sentinel came up, the connection recovered within ~5s, and it followed a later SENTINEL FAILOVER in the same time as a connection that had been healthy from the start.
• Live sentinel, unknown serviceName: used to throw "Failed connecting to configured primary" at startup; now returns a disconnected multiplexer, and every operation fails with that same message. Diagnosable, but late.
• SentinelConnect then GetSentinelMasterConnection(config) where config has only a ServiceName: the failure is now AggregateException(InvalidOperationException: "No nodes to consider") rather than RedisConnectionException. That's a bug on our side and I'll fix it regardless.

My best guesses at what went wrong for you:

1. Code that relied on the sentinel path throwing under abortConnect=false, e.g. a retry/Polly loop around Connect, or a startup probe. It now gets a multiplexer back and the loop never triggers. If the loop re-calls Connect while !IsConnected without disposing, it leaks a connection per iteration. If this is you: abortConnect=true is the setting that means "throw if you can't connect", and that path is unchanged.
2. A misconfiguration that used to fail fast now fails on first use.
3. Separate sentinel credentials: during the outage the recovering connection dials the sentinel addresses with the data-plane credentials, which would show up as AUTH failures in sentinel logs until recovery. Not measured, but plausible from the code.
4. Hosts that drop SYNs rather than refuse: startup now costs ConnectTimeout twice (sentinel connect, then the seed connect).

What I'm going to do regardless of this PR: fix the exception type above; subscribe to +switch-master once a soft-failed sentinel connection actually reaches a sentinel (today it never does, and relies on the fallback polling); clear LastException after recovery; and document the abortConnect=false semantics for sentinel.

What I'm not going to do is revert the leak fix and the non-throwing behaviour, because they match how every other connection type honours abortConnect=false, and #2980 was a legitimate report.

If none of the above matches what you saw, I need: your connection string with secrets removed, what Connect returned or threw before and after, and whether you're calling Connect or GetSentinelMasterConnection yourself. Without that I'll close this at the next build.

[mgravell]

The follow-ups I said I'd do regardless of this PR are in #3280: the discovery failure now surfaces as RedisConnectionException again for direct GetSentinelMasterConnection callers, a soft-failed sentinel connection subscribes to +switch-master once it reaches a sentinel, LastException is cleared on recovery, and the docs now spell out what abortConnect means for sentinel. The questions above still stand if you want this looked at further.