[RedisCache] Allow injection of ConnectionMultiplexer

[epignosisx]

Currently the RedisCache creates the ConnectionMultiplexer and does not allow access to it. This prevents the application from reusing the ConnectionMultiplexer for other operations like PubSub.

Granted, the application could create another ConnectionMultiplexer, but the recommendation is to have only one per application.

I was thinking if it would be possible to have a separate constructor for RedisCache that would receive the ConnectionMultiplexer. I understand that so far Microsoft.Extensions.Caching.Redis did not leak any implementation details about its usage of StackExchange.Redis. However, I noticed that Microsoft.AspNetCore.DataProtection.Redis leaks StackExchange.Redis types here. So, I'm unclear about how you feel about keeping StackExchange.Redis as an implementation detail.

Proposed API

namespace Microsoft.Extensions.Caching.StackExchangeRedis
{
    public class RedisCacheOptions
    {
+       Func<IConnectionMultiplexer> ConnectionMultiplexerFactory { get; set; }     
    }

[epignosisx]

Just noticed there is an open pull request enabling this scenario. But did not find an issue discussing the proposed change, so I'll keep this one open for discussion.

[muratg]

Closing aspnet/Caching#318 as it entails to a breaking change. Keeping this bug open for any alternatives or for a potential major release change.

[israellot]

I believe that injecting IConnectionMultiplexer as a dependency is the path to go.

I share @epignosisx concerns about StackExchange.Redis being a implementation detail or not.
The pragmatic way to address it would be have a service interface and ** StackExchange.Redis ** being used in a adaptor that implements that interface. But that would possibly mean too many levels of generalization though.

[epignosisx]

@muratg could you share where does the team stand on whether StackExchange.Redis should be kept as an implementation detail or not?

Answering this question is critical to make progress on this issue. As @israellot pointed out the alternatives are more abstractions that probably will be undesirable.

[muratg]

cc @davidfowl

[davidfowl]

Yes we should allow for this. Not sure about injecting the IConnectionMultiplexer as a dependency though we should have an option that allows setting a Func<IConnectionMultiplexer>

[JadynWong]

I know the application has created a ConnectionMultiplexer.
Why the application must be create another ConnectionMultiplexer?

[muratg]

Tentatively putting this in 2.2.0. We'll need to identify what the best way to achieve this is.

[schneiBoy]

FYI this issue is referenced here:

https://stackoverflow.com/questions/49403973/net-core-idistributedcache-redis-sentinel-support-for-master-slave/49931794#49931794

[shravan2x]

@muratg Any update to this?

[ekarlso]

Over a year old and nothing new ?

[muratg]

This is not currently in scope. We'll revisit in a future release.

cc @DamianEdwards @davidfowl

[effyteva]

+1
Perhaps a much easier solution would be to allow getting the current Multiplexer? This way Microsoft.Extensions.Caching.Redis would create the Multiplexer instance, and any external code would use the same instance.

[shravan2x]

@effyteva That would become a problem if other libraries did the same thing - everyone would create their own multiplexer and expect others to use theirs.