Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let users explicty configure ServerSelectionStrategy.ServerType in cases they are using a Redis Cluster. #1381

Conversation

dceravigupta
Copy link

@dceravigupta dceravigupta commented Mar 12, 2020

We are using 1.2.6 version of StackExchange.Redis with Azure Redis cache and occasionally we get bunch of "No connection is available to service this operation" and sometime it goes into a state where it doesn't ever recover from this issue and we are forces to reboot those VMs to fix the problem.

I was looking at 1374 and using the same setup but using code of 1.2.6 SDK version with actual Azure Redis cache. I was able to repo the problem and noticed that whenever it happens, ConnectionMultiplexer -> ReconfigureAsyc function fails while PINGing Redis Server and as a result the clusterCount remain 0.

image

Now since we are not sure what type of Redis we are talking to, we default to ServerSelectionStrategy.ServerType i.e. ServerType.Standalone. I didn't went that deep into code, but somehow the endpoint is able to recover but we are still stuck with ServerSelectionStrategy.ServerType as Standalone due to which all subsequent calls fails with "No connection is available to service this operation". Given the part of ReconfigureAsyc I'm talking about is same in latest version as well, I'm pretty sure the problem will exists in there as well. Happy to try it out with latest version as well, just wanted to validate the change is acceptable first.

image

By giving option to explicitly specify, I'm thinking that way in case of failure, we don't have to assume about the type of Redis we are talking to and hence more deterministic behavior.

@dceravigupta dceravigupta changed the title Let uses explicty configure in cases they are using a Redis Cluster. Let uses explicty configure ServerSelectionStrategy in cases they are using a Redis Cluster. Mar 12, 2020
@dceravigupta dceravigupta changed the title Let uses explicty configure ServerSelectionStrategy in cases they are using a Redis Cluster. Let uses explicty configure ServerSelectionStrategy.ServerType in cases they are using a Redis Cluster. Mar 12, 2020
@NickCraver
Copy link
Collaborator

I get the issue, but I'm not sure this is the right place to plugin a change - the main thing is it's not a proxy. None is correct here. It confuses the abstraction/type to have Cluster there, perhaps we can find a better place to force the fact we're talking to a cluster even though it may be failing when we first connect, etc.

Alternatively, if we're always re-evaluating this on reconnect (we should be) and the issue only happens during failure, then perhaps what we need is better "hey...assume it's still a cluster" logic in our retry mechanism. I'd say it's a safe assumption to assume someone didn't change the server types while we disconnected, in 99.99% of cases. And even if they did, we'd just be making bad assumptions until reconnect. And honestly, in that low-percentage case they'd have to shut down the works to change to/from cluster, etc.

@mgravell
Copy link
Collaborator

What concerns me here is that regular cluster is not a proxy; however there is a cluster proxy that is a proxy, and I think this would cause huge confusion. I think we should reserve this change for https://github.com/RedisLabs/redis-cluster-proxy

@NickCraver
Copy link
Collaborator

Closing this out since it's not the right approach - the #1374 change should help here though. We'll take a look at assuming last server state as well, just not in this manner - hopefully the above explains why!

@NickCraver NickCraver closed this Mar 18, 2020
@dceravigupta dceravigupta changed the title Let uses explicty configure ServerSelectionStrategy.ServerType in cases they are using a Redis Cluster. Let users explicty configure ServerSelectionStrategy.ServerType in cases they are using a Redis Cluster. Apr 23, 2020
@dceravigupta
Copy link
Author

@NickCraver Keeping your feedback in mind, I have floated another PR 1444 to fix the above mentioned problem. Please have a look.

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