Skip to content

Fix #2173: Honor AbortOnConnectFail on sentinel path#3063

Closed
IvanWennberg wants to merge 2 commits intoStackExchange:mainfrom
IvanWennberg:main
Closed

Fix #2173: Honor AbortOnConnectFail on sentinel path#3063
IvanWennberg wants to merge 2 commits intoStackExchange:mainfrom
IvanWennberg:main

Conversation

@IvanWennberg
Copy link
Copy Markdown

When Connect is called with ServiceName and AbortOnConnectFail=false,
the non-sentinel path returns a disconnected multiplexer that reconnects in
the background. The sentinel path ignores this flag and throws unconditionally
with "Sentinel: The ConnectionMultiplexer is not a Sentinel connection.
Detected as: Standalone".

This is because SentinelConnect returns a disconnected multiplexer
(ServerType defaults to Standalone when the handshake hasn't completed),
and GetSentinelMasterConnection throws when it sees the wrong ServerType
— without checking AbortOnConnectFail.

Changes in GetSentinelMasterConnection:

  • Skip the ServerType check when AbortOnConnectFail is false and the
    connection hasn't completed handshake.
  • When primary discovery returns null and AbortOnConnectFail is false,
    return a disconnected multiplexer with OnManagedConnectionFailed /
    OnManagedConnectionRestored wired up instead of throwing. Self-healing
    works via the existing SwitchPrimary retry timer.

While Sentinel isn't the future direction for Redis HA, it's still widely
used — especially with Valkey, AWS ElastiCache, and Kubernetes deployments
that haven't moved to cluster mode.

Tests in SentinelAbortOnConnectTests.cs — no running Redis/Sentinel
instance required. Self-healing reuses the existing SwitchPrimary mechanism
tested by ManagedPrimaryConnectionEndToEndWithFailoverTest.

@mgravell
Copy link
Copy Markdown
Collaborator

Will try to look today

@IvanWennberg
Copy link
Copy Markdown
Author

This fixed the first hurdle, but after more extensive testing new issues keeps arising where the mux get stuck in a state it won't recover from depending on the code path it goes through.

I'll look into migrating to cluster instead of using sentinel.
Closing this one for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants