diff --git a/.gitignore b/.gitignore index e3325d77..9d023ee7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ bin obj csx +.vs +_ReSharper.Caches *.exe *.dll diff --git a/src/OutputCacheProvider_net452/RedisOutputCacheProvider_net452.csproj b/src/OutputCacheProvider_net452/RedisOutputCacheProvider_net452.csproj index 1fbfc26c..019a6646 100644 --- a/src/OutputCacheProvider_net452/RedisOutputCacheProvider_net452.csproj +++ b/src/OutputCacheProvider_net452/RedisOutputCacheProvider_net452.csproj @@ -73,6 +73,12 @@ ChangeTrackingSessionStateItemCollection.cs + + ConnectionMultiplexerFactory.cs + + + IConnectionMultiplexerFactory.cs + IRedisClientConnection.cs diff --git a/src/OutputCacheProvider_net462/RedisOutputCacheProvider_net462.csproj b/src/OutputCacheProvider_net462/RedisOutputCacheProvider_net462.csproj index 1034e666..1d81f4cc 100644 --- a/src/OutputCacheProvider_net462/RedisOutputCacheProvider_net462.csproj +++ b/src/OutputCacheProvider_net462/RedisOutputCacheProvider_net462.csproj @@ -77,6 +77,12 @@ ChangeTrackingSessionStateItemCollection.cs + + ConnectionMultiplexerFactory.cs + + + IConnectionMultiplexerFactory.cs + IRedisClientConnection.cs diff --git a/src/RedisSessionStateProvider/RedisConnectionWrapper.cs b/src/RedisSessionStateProvider/RedisConnectionWrapper.cs index f26f83fa..d3f80ed0 100644 --- a/src/RedisSessionStateProvider/RedisConnectionWrapper.cs +++ b/src/RedisSessionStateProvider/RedisConnectionWrapper.cs @@ -11,19 +11,16 @@ namespace Microsoft.Web.Redis { internal class RedisConnectionWrapper : ICacheConnection { - internal static RedisSharedConnection sharedConnection; - static object lockForSharedConnection = new object(); + internal static RedisSharedConnection sharedConnection; // todo remove field + static readonly object lockForSharedConnection = new object(); internal static RedisUtility redisUtility; public KeyGenerator Keys { set; get; } internal IRedisClientConnection redisConnection; - ProviderConfiguration configuration; - public RedisConnectionWrapper(ProviderConfiguration configuration, string id) { - this.configuration = configuration; Keys = new KeyGenerator(id, configuration.ApplicationName); // only single object of RedisSharedConnection will be created and then reused @@ -62,7 +59,8 @@ public TimeSpan GetLockAge(object lockId) // ARGV[1] = session-timeout // this order should not change LUA script depends on it // if data doesn't exists then do nothing - static readonly string updateExpiryTimeScript = (@" + // TODO TrimScript + static readonly string updateExpiryTimeScript = (@" local dataExists = redis.call('EXISTS', KEYS[1]) if dataExists == 0 then return 1; diff --git a/src/RedisSessionStateProvider_net452/RedisSessionStateProvider_net452.csproj b/src/RedisSessionStateProvider_net452/RedisSessionStateProvider_net452.csproj index 4ce67798..b916d76d 100644 --- a/src/RedisSessionStateProvider_net452/RedisSessionStateProvider_net452.csproj +++ b/src/RedisSessionStateProvider_net452/RedisSessionStateProvider_net452.csproj @@ -73,6 +73,12 @@ ChangeTrackingSessionStateItemCollection.cs + + ConnectionMultiplexerFactory.cs + + + IConnectionMultiplexerFactory.cs + IRedisClientConnection.cs diff --git a/src/RedisSessionStateProvider_net462/RedisSessionStateProvider_net462.csproj b/src/RedisSessionStateProvider_net462/RedisSessionStateProvider_net462.csproj index 9342409a..8784b665 100644 --- a/src/RedisSessionStateProvider_net462/RedisSessionStateProvider_net462.csproj +++ b/src/RedisSessionStateProvider_net462/RedisSessionStateProvider_net462.csproj @@ -77,6 +77,12 @@ ChangeTrackingSessionStateItemCollection.cs + + ConnectionMultiplexerFactory.cs + + + IConnectionMultiplexerFactory.cs + IRedisClientConnection.cs diff --git a/src/Shared/ChangeTrackingSessionStateItemCollection.cs b/src/Shared/ChangeTrackingSessionStateItemCollection.cs index 21b7ad31..53586cbb 100644 --- a/src/Shared/ChangeTrackingSessionStateItemCollection.cs +++ b/src/Shared/ChangeTrackingSessionStateItemCollection.cs @@ -192,7 +192,7 @@ internal object GetDataWithoutUpdatingModifiedKeys(string name) { return valueWrapper.GetActualValue(_utility); } - return null;; + return null; } public override IEnumerator GetEnumerator() diff --git a/src/Shared/ConnectionMultiplexerFactory.cs b/src/Shared/ConnectionMultiplexerFactory.cs new file mode 100644 index 00000000..71ce6033 --- /dev/null +++ b/src/Shared/ConnectionMultiplexerFactory.cs @@ -0,0 +1,125 @@ +using System; +using StackExchange.Redis; + +namespace Microsoft.Web.Redis +{ + internal class ConnectionMultiplexerFactory : IConnectionMultiplexerFactory + { + private readonly ConfigurationOptions _configOption; + + internal static DateTimeOffset lastReconnectTime = DateTimeOffset.MinValue; + internal static DateTimeOffset firstErrorTime = DateTimeOffset.MinValue; + internal static DateTimeOffset previousErrorTime = DateTimeOffset.MinValue; + static readonly object reconnectLock = new object(); + internal static TimeSpan ReconnectFrequency = TimeSpan.FromSeconds(60); + internal static TimeSpan ReconnectErrorThreshold = TimeSpan.FromSeconds(30); + + public ConnectionMultiplexerFactory(ProviderConfiguration configuration) + { + // If connection string is given then use it otherwise use individual options + if (!string.IsNullOrEmpty(configuration.ConnectionString)) + { + _configOption = ConfigurationOptions.Parse(configuration.ConnectionString); + // Setting explicitly 'abortconnect' to false. It will overwrite customer provided value for 'abortconnect' + // As it doesn't make sense to allow to customer to set it to true as we don't give them access to ConnectionMultiplexer + // in case of failure customer can not create ConnectionMultiplexer so right choice is to automatically create it by providing AbortOnConnectFail = false + _configOption.AbortOnConnectFail = false; + } + else + { + _configOption = new ConfigurationOptions(); + if (configuration.Port == 0) + { + _configOption.EndPoints.Add(configuration.Host); + } + else + { + _configOption.EndPoints.Add(configuration.Host + ":" + configuration.Port); + } + _configOption.Password = configuration.AccessKey; + _configOption.Ssl = configuration.UseSsl; + _configOption.AbortOnConnectFail = false; + + if (configuration.ConnectionTimeoutInMilliSec != 0) + { + _configOption.ConnectTimeout = configuration.ConnectionTimeoutInMilliSec; + } + + if (configuration.OperationTimeoutInMilliSec != 0) + { + _configOption.SyncTimeout = configuration.OperationTimeoutInMilliSec; + } + } + } + + public IConnectionMultiplexer CreateMultiplexer() + { + lastReconnectTime = DateTimeOffset.UtcNow; + return LogUtility.logger == null + ? ConnectionMultiplexer.Connect(_configOption) + : ConnectionMultiplexer.Connect(_configOption, LogUtility.logger); + } + + private static void CloseMultiplexer(IConnectionMultiplexer oldMultiplexer) + { + try + { + oldMultiplexer.Close(); + } + catch (Exception) + { + // Example error condition: if accessing old.Value causes a connection attempt and that fails. + } + } + + public IConnectionMultiplexer RestartMultiplexer(IConnectionMultiplexer connectionMultiplexer) + { + var previousReconnect = lastReconnectTime; + var elapsedSinceLastReconnect = DateTimeOffset.UtcNow - previousReconnect; + + // If mulitple threads call RestartMultiplexer at the same time, we only want to honor one of them. + if (elapsedSinceLastReconnect > ReconnectFrequency) + { + lock (reconnectLock) + { + var utcNow = DateTimeOffset.UtcNow; + elapsedSinceLastReconnect = utcNow - lastReconnectTime; + + if (elapsedSinceLastReconnect < ReconnectFrequency) + { + return connectionMultiplexer; // Some other thread made it through the check and the lock, so nothing to do. + } + + if (firstErrorTime == DateTimeOffset.MinValue) + { + // We got error first time after last reconnect + firstErrorTime = utcNow; + previousErrorTime = utcNow; + return connectionMultiplexer; + } + + var elapsedSinceFirstError = utcNow - firstErrorTime; + var elapsedSinceMostRecentError = utcNow - previousErrorTime; + previousErrorTime = utcNow; + + if ((elapsedSinceFirstError >= ReconnectErrorThreshold) && (elapsedSinceMostRecentError <= ReconnectErrorThreshold)) + { + LogUtility.LogInfo($"RestartMultiplexer: now: {utcNow.ToString()}"); + LogUtility.LogInfo($"RestartMultiplexer: elapsedSinceLastReconnect: {elapsedSinceLastReconnect.ToString()}, ReconnectFrequency: {ReconnectFrequency.ToString()}"); + LogUtility.LogInfo($"RestartMultiplexer: elapsedSinceFirstError: {elapsedSinceFirstError.ToString()}, elapsedSinceMostRecentError: {elapsedSinceMostRecentError.ToString()}, ReconnectErrorThreshold: {ReconnectErrorThreshold.ToString()}"); + + firstErrorTime = DateTimeOffset.MinValue; + previousErrorTime = DateTimeOffset.MinValue; + + //var oldMultiplexer = _redisMultiplexer; + CloseMultiplexer(connectionMultiplexer); + return CreateMultiplexer(); + } + + return connectionMultiplexer; + } + } + return connectionMultiplexer; + } + } +} \ No newline at end of file diff --git a/src/Shared/IConnectionMultiplexerFactory.cs b/src/Shared/IConnectionMultiplexerFactory.cs new file mode 100644 index 00000000..32f5f633 --- /dev/null +++ b/src/Shared/IConnectionMultiplexerFactory.cs @@ -0,0 +1,22 @@ +using StackExchange.Redis; + +namespace Microsoft.Web.Redis +{ + public interface IConnectionMultiplexerFactory + { + /// + /// This method provides either new or already existing instance of . + /// + /// Fully configured connection multiplexer + IConnectionMultiplexer CreateMultiplexer(); + + /// + /// When fails with + /// then it sends to and attempt to cleanup a failed multiplexer. + /// Additionally, the factory itself is responsible for providing a clean, fresh instance of IConnectionMultiplexer (it can be the same instance). + /// + /// Instance that failed with + /// + IConnectionMultiplexer RestartMultiplexer(IConnectionMultiplexer connectionMultiplexer); + } +} \ No newline at end of file diff --git a/src/Shared/ProviderConfiguration.cs b/src/Shared/ProviderConfiguration.cs index c3423252..76795bf7 100644 --- a/src/Shared/ProviderConfiguration.cs +++ b/src/Shared/ProviderConfiguration.cs @@ -29,6 +29,7 @@ internal class ProviderConfiguration public int OperationTimeoutInMilliSec { get; set; } public string ConnectionString { get; set; } public string RedisSerializerType { get; set; } + public string ConnectionMultiplexerFactoryType { get; internal set; } /* Empty constructor required for testing */ internal ProviderConfiguration() @@ -83,6 +84,7 @@ private ProviderConfiguration(NameValueCollection config) AccessKey = GetStringSettings(config, "accessKey", null); UseSsl = GetBoolSettings(config, "ssl", true); RedisSerializerType = GetStringSettings(config, "redisSerializerType", null); + ConnectionMultiplexerFactoryType = GetStringSettings(config, "connectionMultiplexerFactoryType", null); // All below parameters are only fetched from web.config DatabaseId = GetIntSettings(config, "databaseId", 0); ApplicationName = GetStringSettings(config, "applicationName", null); diff --git a/src/Shared/RedisSharedConnection.cs b/src/Shared/RedisSharedConnection.cs index 87c285fc..9f3f70c2 100644 --- a/src/Shared/RedisSharedConnection.cs +++ b/src/Shared/RedisSharedConnection.cs @@ -10,16 +10,9 @@ namespace Microsoft.Web.Redis { internal class RedisSharedConnection { - private ProviderConfiguration _configuration; - private ConfigurationOptions _configOption; - private Lazy _redisMultiplexer; - - internal static DateTimeOffset lastReconnectTime = DateTimeOffset.MinValue; - internal static DateTimeOffset firstErrorTime = DateTimeOffset.MinValue; - internal static DateTimeOffset previousErrorTime = DateTimeOffset.MinValue; - static object reconnectLock = new object(); - internal static TimeSpan ReconnectFrequency = TimeSpan.FromSeconds(60); - internal static TimeSpan ReconnectErrorThreshold = TimeSpan.FromSeconds(30); + private readonly ProviderConfiguration _configuration; + private readonly IConnectionMultiplexerFactory _factory; + private Lazy _connectionMultiplexer; // Used for mocking in testing internal RedisSharedConnection() @@ -28,123 +21,34 @@ internal RedisSharedConnection() public RedisSharedConnection(ProviderConfiguration configuration) { _configuration = configuration; - - // If connection string is given then use it otherwise use individual options - if (!string.IsNullOrEmpty(configuration.ConnectionString)) - { - _configOption = ConfigurationOptions.Parse(configuration.ConnectionString); - // Setting explicitly 'abortconnect' to false. It will overwrite customer provided value for 'abortconnect' - // As it doesn't make sense to allow to customer to set it to true as we don't give them access to ConnectionMultiplexer - // in case of failure customer can not create ConnectionMultiplexer so right choice is to automatically create it by providing AbortOnConnectFail = false - _configOption.AbortOnConnectFail = false; - } - else - { - _configOption = new ConfigurationOptions(); - if (configuration.Port == 0) - { - _configOption.EndPoints.Add(configuration.Host); - } - else - { - _configOption.EndPoints.Add(configuration.Host + ":" + configuration.Port); - } - _configOption.Password = configuration.AccessKey; - _configOption.Ssl = configuration.UseSsl; - _configOption.AbortOnConnectFail = false; - - if (configuration.ConnectionTimeoutInMilliSec != 0) - { - _configOption.ConnectTimeout = configuration.ConnectionTimeoutInMilliSec; - } + _factory = string.IsNullOrEmpty(configuration.ConnectionMultiplexerFactoryType) + ? new ConnectionMultiplexerFactory(configuration) + : CreateMultiplexerFactory(configuration.ConnectionMultiplexerFactoryType); - if (configuration.OperationTimeoutInMilliSec != 0) - { - _configOption.SyncTimeout = configuration.OperationTimeoutInMilliSec; - } - } - CreateMultiplexer(); + _connectionMultiplexer = new Lazy(_factory.CreateMultiplexer); } public IDatabase Connection { - get { return _redisMultiplexer.Value.GetDatabase(_configOption.DefaultDatabase ?? _configuration.DatabaseId); } - } - - public void ForceReconnect() - { - var previousReconnect = lastReconnectTime; - var elapsedSinceLastReconnect = DateTimeOffset.UtcNow - previousReconnect; - - // If mulitple threads call ForceReconnect at the same time, we only want to honor one of them. - if (elapsedSinceLastReconnect > ReconnectFrequency) + get { - lock (reconnectLock) - { - var utcNow = DateTimeOffset.UtcNow; - elapsedSinceLastReconnect = utcNow - lastReconnectTime; - - if (elapsedSinceLastReconnect < ReconnectFrequency) - { - return; // Some other thread made it through the check and the lock, so nothing to do. - } - - if (firstErrorTime == DateTimeOffset.MinValue) - { - // We got error first time after last reconnect - firstErrorTime = utcNow; - previousErrorTime = utcNow; - return; - } - - var elapsedSinceFirstError = utcNow - firstErrorTime; - var elapsedSinceMostRecentError = utcNow - previousErrorTime; - previousErrorTime = utcNow; - - if ((elapsedSinceFirstError >= ReconnectErrorThreshold) && (elapsedSinceMostRecentError <= ReconnectErrorThreshold)) - { - LogUtility.LogInfo($"ForceReconnect: now: {utcNow.ToString()}"); - LogUtility.LogInfo($"ForceReconnect: elapsedSinceLastReconnect: {elapsedSinceLastReconnect.ToString()}, ReconnectFrequency: {ReconnectFrequency.ToString()}"); - LogUtility.LogInfo($"ForceReconnect: elapsedSinceFirstError: {elapsedSinceFirstError.ToString()}, elapsedSinceMostRecentError: {elapsedSinceMostRecentError.ToString()}, ReconnectErrorThreshold: {ReconnectErrorThreshold.ToString()}"); - - firstErrorTime = DateTimeOffset.MinValue; - previousErrorTime = DateTimeOffset.MinValue; - - var oldMultiplexer = _redisMultiplexer; - CloseMultiplexer(oldMultiplexer); - CreateMultiplexer(); - } - } + var db = _configuration.DatabaseId; + return db == default(int) + ? _connectionMultiplexer.Value.GetDatabase() + : _connectionMultiplexer.Value.GetDatabase(db); } } - private void CreateMultiplexer() + public void ForceReconnect() { - if (LogUtility.logger == null) - { - _redisMultiplexer = new Lazy(() => ConnectionMultiplexer.Connect(_configOption)); - } - else - { - _redisMultiplexer = new Lazy(() => ConnectionMultiplexer.Connect(_configOption, LogUtility.logger)); - } - lastReconnectTime = DateTimeOffset.UtcNow; + var cm = _factory.RestartMultiplexer(_connectionMultiplexer.Value); + _connectionMultiplexer = new Lazy(() => cm); } - private void CloseMultiplexer(Lazy oldMultiplexer) + private static IConnectionMultiplexerFactory CreateMultiplexerFactory(string connectionMultiplexerFactoryType) { - if (oldMultiplexer.Value != null) - { - try - { - oldMultiplexer.Value.Close(); - } - catch (Exception) - { - // Example error condition: if accessing old.Value causes a connection attempt and that fails. - } - } + var serializerType = Type.GetType(connectionMultiplexerFactoryType, true); + return (IConnectionMultiplexerFactory)Activator.CreateInstance(serializerType); } - } -} +} \ No newline at end of file diff --git a/test/RedisSessionStateProviderUnitTest/ProviderConfigurationTests.cs b/test/RedisSessionStateProviderUnitTest/ProviderConfigurationTests.cs index 8e972fdd..c68ecd90 100644 --- a/test/RedisSessionStateProviderUnitTest/ProviderConfigurationTests.cs +++ b/test/RedisSessionStateProviderUnitTest/ProviderConfigurationTests.cs @@ -238,6 +238,25 @@ public void GetConnectionString_Valid() config.Add(settingsMethodName, "GetSettings"); Assert.Equal("localhost:6380", ProviderConfiguration.GetConnectionString(config)); } + + [Fact] + public void GetConnectionMultiplexerFactoryType_Valid() + { + // arrange + const string connectionMultiplexerFactoryType = "test type"; + var config = new NameValueCollection + { + ["connectionMultiplexerFactoryType"] = connectionMultiplexerFactoryType + }; + + + // act + var configuration = ProviderConfiguration.ProviderConfigurationForSessionState(config); + + // assert + Assert.Equal(connectionMultiplexerFactoryType, configuration.ConnectionMultiplexerFactoryType); + + } } public class Logger diff --git a/test/RedisSessionStateProviderUnitTest/RedisSharedConnectionTests.cs b/test/RedisSessionStateProviderUnitTest/RedisSharedConnectionTests.cs new file mode 100644 index 00000000..c6f502c9 --- /dev/null +++ b/test/RedisSessionStateProviderUnitTest/RedisSharedConnectionTests.cs @@ -0,0 +1,88 @@ +using System; +using System.Threading; +using FakeItEasy; +using StackExchange.Redis; +using Xunit; + +namespace Microsoft.Web.Redis.Tests +{ + public class RedisSharedConnectionTests + { + private class TestingConnectionMultiplexerFactory : IConnectionMultiplexerFactory + { + public TestingConnectionMultiplexerFactory() + { + SetFactory(); + } + +#if DOTNET_462 + private static AsyncLocal _factoryProxy { get; } = new AsyncLocal(); +#else + [ThreadStatic] + private static Lazy _factoryProxy; +#endif + + public static IConnectionMultiplexerFactory FactoryProxy => _factoryProxy.Value; + + private static void SetFactory() + { + var factory = A.Fake(); +#if DOTNET_462 + _factoryProxy.Value = factory; +#else + _factoryProxy = new Lazy(() => factory); +#endif + } + + public IConnectionMultiplexer CreateMultiplexer() + { + return _factoryProxy.Value.CreateMultiplexer(); + } + + public IConnectionMultiplexer RestartMultiplexer(IConnectionMultiplexer connectionMultiplexer) + { + return _factoryProxy.Value.RestartMultiplexer(connectionMultiplexer); + } + } + + [Fact(DisplayName = "ConnectionMultiplexerFactory should Be Created When Accessing Shared Connection")] + public void ConnectionMultiplexerFactory_Should_CreateConnection() + { + // arrange + var configuration = new ProviderConfiguration + { + ConnectionMultiplexerFactoryType = typeof(TestingConnectionMultiplexerFactory).AssemblyQualifiedName + }; + + // act + var sharedConnection = new RedisSharedConnection(configuration); + var connection = sharedConnection.Connection; // getting a connection calls IConnectionMultiplexerFactory.CreateMultiplexer() + + // assert + var connectionFactory = TestingConnectionMultiplexerFactory.FactoryProxy; + A.CallTo(() => connectionFactory.CreateMultiplexer()).MustHaveHappened(Repeated.Exactly.Once); + A.CallTo(() => connectionFactory.RestartMultiplexer(A.Ignored)).MustNotHaveHappened(); + } + + public void ConnectionMultiplexerFactory_ForceReconnect() + { + // arrange + var connectionFactory = TestingConnectionMultiplexerFactory.FactoryProxy; + var connectionMultiplexer = A.Fake(); + A.CallTo(() => connectionFactory.CreateMultiplexer()).Returns(connectionMultiplexer); + var configuration = new ProviderConfiguration + { + ConnectionMultiplexerFactoryType = typeof(TestingConnectionMultiplexerFactory).AssemblyQualifiedName + }; + + // act + var sharedConnection = new RedisSharedConnection(configuration); + var connection = sharedConnection.Connection; + sharedConnection.ForceReconnect(); + + // assert + A.CallTo(() => connectionFactory.CreateMultiplexer()).MustHaveHappened(Repeated.Exactly.Once); + A.CallTo(() => connectionFactory.RestartMultiplexer(connectionMultiplexer)).MustHaveHappened(Repeated.Exactly.Once); + } + } +} \ No newline at end of file diff --git a/test/RedisSessionStateProviderUnitTest_452/RedisSessionStateProvider.UnitTest_452.csproj b/test/RedisSessionStateProviderUnitTest_452/RedisSessionStateProvider.UnitTest_452.csproj index 5e1b13a2..1d6b7c23 100644 --- a/test/RedisSessionStateProviderUnitTest_452/RedisSessionStateProvider.UnitTest_452.csproj +++ b/test/RedisSessionStateProviderUnitTest_452/RedisSessionStateProvider.UnitTest_452.csproj @@ -95,6 +95,9 @@ RedisSessionStateProviderTests.cs + + RedisSharedConnectionTests.cs + RedisUtilityTests.cs diff --git a/test/RedisSessionStateProviderUnitTest_462/RedisSessionStateProvider.UnitTest_net462.csproj b/test/RedisSessionStateProviderUnitTest_462/RedisSessionStateProvider.UnitTest_net462.csproj index 7ccd20a8..688c2aff 100644 --- a/test/RedisSessionStateProviderUnitTest_462/RedisSessionStateProvider.UnitTest_net462.csproj +++ b/test/RedisSessionStateProviderUnitTest_462/RedisSessionStateProvider.UnitTest_net462.csproj @@ -99,6 +99,9 @@ RedisSessionStateProviderTests.cs + + RedisSharedConnectionTests.cs + RedisUtilityTests.cs