nhibernate/NHibernate-Caches

StackExchangeRedis: Connection resilience

danspam opened this issue · 2 comments

When using cloud managed Redis like Azure Cache, you can get network blips when, for example, they patch servers or the like. This can cause connection failures, as described in: https://gist.github.com/JonCole/925630df72be1351b21440625ff2671f#file-redis-bestpractices-stackexchange-redis-md (see 'Reconnecting with Lazy pattern').

With NHibernate.Caches.StackExchangeRedis, the only way to recover from this situation seems to be to restart the servers but as per the example code in the above link, it is possible to recreate the connection multiplexer in these situations to force a re-connection.

It would be great to add this connection resilience to NHibernate.Caches.StackExchangeRedis. I was thinking perhaps adding additional functionality (e.g. a reconnect method) to the IConnectionMultiplexerProvider and passing this down into AbstractRegionStrategy instead of the IConnectionMultiplexer itself. We could then add retries in the various methods (get, put etc) if the connection exceptions occur and recreate the IConnectionMultiplexer and associated properties when the issue occurs.

Teoretically StackExchange.Redis should be resilient itself as it has a retry reconnect option, but from StackExchange/StackExchange.Redis#1120 it seems that a lot of people have this issue nonetheless. As it doesn't seem to be fixed any time soon, I will try to add this functionality.

I was thinking perhaps adding additional functionality (e.g. a reconnect method) to the IConnectionMultiplexerProvider and passing this down into AbstractRegionStrategy instead of the IConnectionMultiplexer itself

This would produce a breaking change and unwanted overhead for users that do not have this issue.

In order to recreate the connection multiplexer in NHibernate.Caches.StackExchangeRedis as transparent as possible and without producing a breaking change, is to create wrappers for IConnectionMultiplexer, IDatabase and ISubscriber interfaces which would handle such scenario. The only api that cannot be wrapped and would need to be handled by each strategy is the sequential subscription for a channel:

ConnectionMultiplexer.GetSubscriber().Subscribe(RegionKey).OnMessage((Action<ChannelMessage>) OnVersionMessage);

which would need to re-subscribe each time the connection multiplexer is recreated.

After some research, I decided to not include such functionality in this repository, in order to avoid binary breaking changes of StackExchange.Redis in the future. By creating wrappers around StackExchange.Redis interfaces we are vulnerable to their changes. In StackExchange.Redis version 2.1.0, IDatabase interface was changed and an additional dependency was added to support IAsyncEnumerable<>, which would force NHibernate.Caches.StackExchangeRedis to whether support version 2.1.x or 2.0.x, if this functionality would be included.

Instead, I made a separate repository https://github.com/maca88/StackExchange.Redis.Resilience for this purpose. I was able to support sequential subscriptions without having to modify the current code, so in order to use it for NHibernate.Caches.StackExchangeRedis, you have to create and register the following provider:

public class ResilientConnectionMultiplexerProvider : IConnectionMultiplexerProvider
{
  public IConnectionMultiplexer Get(string configuration)
  {
    return new ResilientConnectionMultiplexer(
      () => ConnectionMultiplexer.Connect(configuration),
      () => ConnectionMultiplexer.ConnectAsync(configuration)
    );
  }
}

As this functionality won't be implemented here, I am closing this issue.