Repository navigation
Sentinel soft-fail follow-ups: discovery failure as RedisConnectionException, late sentinel init, clear LastException - #3280
Merged
Conversation
…ception, late sentinel init, clear LastException Follow-ups to #3187 (AbortOnConnectFail=false for sentinel), prompted by the investigation of #3271: - GetSentinelMasterConnection with a config that has no endpoints of its own and nothing discovered threw AggregateException(InvalidOperationException "No nodes to consider") from the seed connect. It now throws the discovery RedisConnectionException, as it did before #3187. - A sentinel connection whose initial connect soft-failed never ran InitializeSentinel, because only the connect paths called it; such a connection never subscribed to +switch-master and tracked failovers only through the fallback polling. InitializeSentinel is now idempotent and is also reached from a non-first reconfigure that detects a sentinel. - The discovery failure recorded as LastException at connect time stayed there after recovery and was folded into every later "no connection available" message. It is cleared once the managed connection is restored. - Document what abortConnect means for sentinel connections.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-ups to #3187 (
AbortOnConnectFail=falsefor sentinel), as promised in #3271. None of this changes behaviour forabortConnect=true.Changes
RedisConnectionException.GetSentinelMasterConnectionwith a config that has no endpoints of its own (the direct-API pattern: a long-livedSentinelConnect, then a per-service config carrying onlyServiceName) threwAggregateException(InvalidOperationException("No nodes to consider"))from the seed connect when nothing was discovered. It now throws the discoveryRedisConnectionException, as it did before Fix Sentinel connection leak and AbortOnConnectFail=false handling #3187. Measured against a live sentinel with an unknown service name.InitializeSentinel, because only the connect paths call it. Such a connection recovered and followed failovers, but only via the fallback polling: it never subscribed to+switch-master. Measured: subscription absent after recovery.InitializeSentinelis now idempotent and is also reached from a non-first reconfigure that detects a sentinel. The first connect still leaves it toConnectImpl, so the happy-path ordering is unchanged.LastExceptioncleared on recovery. The discovery failure recorded at connect time stayed inLastExceptionafter the connection reached its primary, and the exception factory folded it into every later "no connection available" message. Cleared inOnManagedConnectionRestoredonce the switch succeeds.Configuration.mdnow says whatabortConnectmeans for sentinel connections, and thatabortConnect=trueis the fail-fast setting.Tests
SentinelConfigTests.GetSentinelMasterConnection_NothingToSeedWith_AbortDisabled_ThrowsTheDiscoveryFailure(closed loopback port, no topology).SentinelTests.AbortOnConnectFailFalseRecoversOnceSentinelsAppearadditionally asserts the+switch-mastersubscription and a nullLastExceptionafter recovery; both were false before this change.Sentinel*classes pass against a local primary/replica/three-sentinel topology (32 passed, 1 long-running skip).dotnet build Build.csproj -c Release /p:CI=trueis clean. No public API changes.