diff --git a/src/StackExchange.Redis/CommandRetry/DefaultRetry.cs b/src/StackExchange.Redis/CommandRetry/DefaultRetry.cs new file mode 100644 index 000000000..e903f76c1 --- /dev/null +++ b/src/StackExchange.Redis/CommandRetry/DefaultRetry.cs @@ -0,0 +1,55 @@ +namespace StackExchange.Redis +{ + /// + /// Command Policy to have all commands being retried for connection exception + /// + public class AlwaysRetryOnConnectionException : ICommandRetryPolicy + { + /// + /// + /// + /// + /// + public bool ShouldRetryOnConnectionException(CommandStatus commandStatus) => true; + } + + /// + /// Command Policy to have only commands that are not yet sent being retried for connection exception + /// + public class RetryIfNotSentOnConnectionException : ICommandRetryPolicy + { + /// + /// + /// + /// + /// + public bool ShouldRetryOnConnectionException(CommandStatus commandStatus) + { + return commandStatus == CommandStatus.WaitingToBeSent; + } + } + + /// + /// Command Policy to choose which commands will be retried on a connection exception + /// + public class CommandRetryPolicy + { + /// + /// Command Policy to have all commands being retried for connection exception + /// + /// + public ICommandRetryPolicy AlwaysRetryOnConnectionException() + { + return new AlwaysRetryOnConnectionException(); + } + + /// + /// Command Policy to have only commands that are not yet sent being retried for connection exception + /// + /// + public ICommandRetryPolicy RetryIfNotSentOnConnectionException() + { + return new RetryIfNotSentOnConnectionException(); + } + } +} diff --git a/src/StackExchange.Redis/CommandRetry/ICommandRetryPolicy.cs b/src/StackExchange.Redis/CommandRetry/ICommandRetryPolicy.cs new file mode 100644 index 000000000..be14d0fed --- /dev/null +++ b/src/StackExchange.Redis/CommandRetry/ICommandRetryPolicy.cs @@ -0,0 +1,16 @@ +namespace StackExchange.Redis +{ + /// + /// interface to implement command retry policy + /// + public interface ICommandRetryPolicy + { + /// + /// Called when a message failed due to connection error + /// + /// current state of the command + /// + public bool ShouldRetryOnConnectionException(CommandStatus commandStatus); + + } +} diff --git a/src/StackExchange.Redis/CommandRetry/IRetryOnReconnectPolicy.cs b/src/StackExchange.Redis/CommandRetry/IRetryOnReconnectPolicy.cs deleted file mode 100644 index 5abd25dd0..000000000 --- a/src/StackExchange.Redis/CommandRetry/IRetryOnReconnectPolicy.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace StackExchange.Redis -{ - /// - /// Interface for a policy that determines which commands should be retried upon restoration of a lost connection - /// - public interface IRetryOnReconnectPolicy - { - /// - /// Determines whether a failed command should be retried - /// - /// Current state of the command - /// True to retry the command, otherwise false - public bool ShouldRetry(CommandStatus commandStatus); - } -} diff --git a/src/StackExchange.Redis/CommandRetry/MessageRetryQueue.cs b/src/StackExchange.Redis/CommandRetry/MessageRetryQueue.cs index de70561ab..ba09c24da 100644 --- a/src/StackExchange.Redis/CommandRetry/MessageRetryQueue.cs +++ b/src/StackExchange.Redis/CommandRetry/MessageRetryQueue.cs @@ -20,7 +20,7 @@ internal MessageRetryQueue(IMessageRetryHelper messageRetryHelper, int? maxRetry _messageRetryHelper = messageRetryHelper; } - public int RetryQueueLength => _queue.Count; + public int CurrentQueueLength => _queue.Count; [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool TryHandleFailedCommand(Message message) @@ -31,6 +31,7 @@ internal bool TryHandleFailedCommand(Message message) int count = _queue.Count; if (_maxRetryQueueLength.HasValue && count >= _maxRetryQueueLength) { + // TODO: Log an error here when logging is supported return false; } wasEmpty = count == 0; diff --git a/src/StackExchange.Redis/CommandRetry/RetryOnReconnect.cs b/src/StackExchange.Redis/CommandRetry/RetryOnReconnect.cs deleted file mode 100644 index 505149ce5..000000000 --- a/src/StackExchange.Redis/CommandRetry/RetryOnReconnect.cs +++ /dev/null @@ -1,39 +0,0 @@ -using System; - -namespace StackExchange.Redis -{ - /// - /// Command retry policy to determine which commands will be retried after a lost connection is retored - /// - public class RetryOnReconnect : IRetryOnReconnectPolicy - { - private readonly Func _shouldRetry; - - internal RetryOnReconnect(Func shouldRetry) - { - _shouldRetry = shouldRetry; - } - - /// - /// Retry all commands - /// - /// An instance of a retry policy that retries all commands - public static IRetryOnReconnectPolicy Always - => new RetryOnReconnect(commandStatus => true); - - /// - /// Retry only commands which fail before being sent to the server - /// - /// An instance of a policy that retries only unsent commands - public static IRetryOnReconnectPolicy IfNotSent - => new RetryOnReconnect(commandStatus => commandStatus == CommandStatus.WaitingToBeSent); - - /// - /// Determines whether to retry a command upon restoration of a lost connection - /// - /// Status of the command - /// True to retry the command, otherwise false - public bool ShouldRetry(CommandStatus commandStatus) - => _shouldRetry.Invoke(commandStatus); - } -} diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs index 6d54db8b8..01b5d0b12 100644 --- a/src/StackExchange.Redis/ConfigurationOptions.cs +++ b/src/StackExchange.Redis/ConfigurationOptions.cs @@ -58,16 +58,16 @@ internal static SslProtocols ParseSslProtocols(string key, string value) return tmp; } - internal static IRetryOnReconnectPolicy ParseRetryCommandsOnReconnect(string key, string value) + internal static ICommandRetryPolicy ParseCommandRetryPolicy(string key, string value) { switch (value.ToLower()) { case "noretry": return null; case "alwaysretry": - return RetryOnReconnect.Always; + return new CommandRetryPolicy().AlwaysRetryOnConnectionException(); case "retryifnotsent": - return RetryOnReconnect.IfNotSent; + return new CommandRetryPolicy().RetryIfNotSentOnConnectionException(); default: throw new ArgumentOutOfRangeException(key, $"Keyword '{key}' can be NoRetry, AlwaysRetry or RetryIfNotSent; the value '{value}' is not recognised."); } @@ -106,8 +106,8 @@ internal const string Version = "version", WriteBuffer = "writeBuffer", CheckCertificateRevocation = "checkCertificateRevocation", - RetryCommandsOnReconnect = "RetryCommandsOnReconnect", - RetryQueueLength = "RetryQueueLength"; + CommandRetryPolicy = "commandRetryPolicy", + RetryQueueMaxLength = "retryQueueMaxLength"; private static readonly Dictionary normalizedOptions = new[] @@ -138,8 +138,8 @@ internal const string Version, WriteBuffer, CheckCertificateRevocation, - RetryCommandsOnReconnect, - RetryQueueLength, + CommandRetryPolicy, + RetryQueueMaxLength, }.ToDictionary(x => x, StringComparer.OrdinalIgnoreCase); public static string TryNormalize(string value) @@ -160,7 +160,7 @@ public static string TryNormalize(string value) private Version defaultVersion; - private int? keepAlive, asyncTimeout, syncTimeout, connectTimeout, responseTimeout, writeBuffer, connectRetry, configCheckSeconds, retryQueueLength; + private int? keepAlive, asyncTimeout, syncTimeout, connectTimeout, responseTimeout, writeBuffer, connectRetry, configCheckSeconds, retryQueueMaxLength; private Proxy? proxy; @@ -358,7 +358,7 @@ public bool PreserveAsyncOrder /// /// The retry policy to be used for command retries during connection reconnects /// - public IRetryOnReconnectPolicy RetryCommandsOnReconnect { get; set; } + public ICommandRetryPolicy CommandRetryPolicy { get; set; } /// /// Indicates whether endpoints should be resolved via DNS before connecting. @@ -426,9 +426,9 @@ public bool PreserveAsyncOrder public int ConfigCheckSeconds { get { return configCheckSeconds.GetValueOrDefault(60); } set { configCheckSeconds = value; } } /// - /// If retry policy is specified, Retry Queue max length, by default there's no queue limit + /// Maximum length of the retry queue (defaults to 100,000) /// - public int? RetryQueueMaxLength { get; set; } + public int? RetryQueueMaxLength { get { return retryQueueMaxLength.GetValueOrDefault(100000); } set { retryQueueMaxLength = value; } } /// /// Parse the configuration from a comma-delimited configuration string @@ -495,7 +495,7 @@ public ConfigurationOptions Clone() ReconnectRetryPolicy = reconnectRetryPolicy, SslProtocols = SslProtocols, checkCertificateRevocation = checkCertificateRevocation, - RetryCommandsOnReconnect = RetryCommandsOnReconnect, + CommandRetryPolicy = CommandRetryPolicy, RetryQueueMaxLength = RetryQueueMaxLength, }; foreach (var item in EndPoints) @@ -581,8 +581,8 @@ public string ToString(bool includePassword) Append(sb, OptionKeys.ConfigCheckSeconds, configCheckSeconds); Append(sb, OptionKeys.ResponseTimeout, responseTimeout); Append(sb, OptionKeys.DefaultDatabase, DefaultDatabase); - Append(sb, OptionKeys.RetryCommandsOnReconnect, RetryCommandsOnReconnect); - Append(sb, OptionKeys.RetryQueueLength, retryQueueLength); + Append(sb, OptionKeys.CommandRetryPolicy, CommandRetryPolicy); + Append(sb, OptionKeys.RetryQueueMaxLength, retryQueueMaxLength); commandMap?.AppendDeltas(sb); return sb.ToString(); } @@ -660,7 +660,7 @@ private static void Append(StringBuilder sb, string prefix, object value) private void Clear() { ClientName = ServiceName = User = Password = tieBreaker = sslHost = configChannel = null; - keepAlive = syncTimeout = asyncTimeout = connectTimeout = writeBuffer = connectRetry = configCheckSeconds = DefaultDatabase = retryQueueLength = null; + keepAlive = syncTimeout = asyncTimeout = connectTimeout = writeBuffer = connectRetry = configCheckSeconds = DefaultDatabase = retryQueueMaxLength = null; allowAdmin = abortOnConnectFail = highPrioritySocketThreads = resolveDns = ssl = null; SslProtocols = null; defaultVersion = null; @@ -671,7 +671,7 @@ private void Clear() CertificateValidation = null; ChannelPrefix = default(RedisChannel); SocketManager = null; - RetryCommandsOnReconnect = null; + CommandRetryPolicy = null; } object ICloneable.Clone() => Clone(); @@ -792,10 +792,10 @@ private void DoParse(string configuration, bool ignoreUnknown) case OptionKeys.SslProtocols: SslProtocols = OptionKeys.ParseSslProtocols(key, value); break; - case OptionKeys.RetryCommandsOnReconnect: - RetryCommandsOnReconnect = OptionKeys.ParseRetryCommandsOnReconnect(key, value); + case OptionKeys.CommandRetryPolicy: + CommandRetryPolicy = OptionKeys.ParseCommandRetryPolicy(key, value); break; - case OptionKeys.RetryQueueLength: + case OptionKeys.RetryQueueMaxLength: RetryQueueMaxLength = OptionKeys.ParseInt32(key, value, minValue: 0); break; default: diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.cs b/src/StackExchange.Redis/ConnectionMultiplexer.cs index 35a4276b4..213ddd5d1 100755 --- a/src/StackExchange.Redis/ConnectionMultiplexer.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.cs @@ -1287,7 +1287,7 @@ internal ServerEndPoint GetServerEndPoint(EndPoint endpoint, LogProxy log = null /// /// returns the current length of the retry queue /// - public int RetryQueueLength => RetryQueueManager.RetryQueueLength; + public int CurrentRetryQueueLength => RetryQueueManager.CurrentQueueLength; private ConnectionMultiplexer(ConfigurationOptions configuration) { @@ -2810,9 +2810,9 @@ internal bool TryMessageForRetry(Message message, Exception ex) } bool shouldRetry = false; - if (RawConfig.RetryCommandsOnReconnect != null && ex is RedisConnectionException) + if (RawConfig.CommandRetryPolicy != null && ex is RedisConnectionException) { - shouldRetry = message.IsInternalCall || RawConfig.RetryCommandsOnReconnect.ShouldRetry(message.Status); + shouldRetry = message.IsInternalCall || RawConfig.CommandRetryPolicy.ShouldRetryOnConnectionException(message.Status); } if (shouldRetry && RetryQueueManager.TryHandleFailedCommand(message)) diff --git a/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerIntegrationTests.cs b/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerIntegrationTests.cs index ed43765e8..611a9d206 100644 --- a/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerIntegrationTests.cs +++ b/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerIntegrationTests.cs @@ -21,7 +21,7 @@ public async Task RetryAsyncMessageIntegration(bool retryPolicySet) ConfigurationOptions configClient = new ConfigurationOptions(); configClient.EndPoints.Add("127.0.0.1"); configAdmin.AbortOnConnectFail = false; - configClient.RetryCommandsOnReconnect = retryPolicySet ? RetryOnReconnect.Always : null; + configClient.CommandRetryPolicy = retryPolicySet ? new CommandRetryPolicy().AlwaysRetryOnConnectionException() : null; using (var adminMuxer = ConnectionMultiplexer.Connect(configAdmin)) using (var clientmuxer = ConnectionMultiplexer.Connect(configClient)) diff --git a/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerTests.cs b/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerTests.cs index 1992b12a5..d453f6117 100644 --- a/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerTests.cs +++ b/tests/StackExchange.Redis.Tests/CommandRetry/CommandRetryQueueManagerTests.cs @@ -34,7 +34,7 @@ public void ValidateMaxQueueLengthFails() [Fact] public async void RetryMessageSucceeds() { - using (var muxer = Create(allowAdmin: true, retryPolicy: RetryOnReconnect.Always)) + using (var muxer = Create(allowAdmin: true, retryPolicy: new CommandRetryPolicy().AlwaysRetryOnConnectionException())) { var conn = muxer.GetDatabase(); var duration = await conn.PingAsync().ForAwait(); @@ -51,7 +51,7 @@ public void TryHandleFailedMessageSucceedsOnEndPointAvailable() messageRetryQueue.TryHandleFailedCommand(message); - Assert.True(messageRetryQueue.RetryQueueLength == 0); + Assert.True(messageRetryQueue.CurrentQueueLength == 0); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Once); } @@ -63,7 +63,7 @@ public void TryHandleFailedMessageWaitOnEndPointUnAvailable() messageRetryQueue.TryHandleFailedCommand(message); - Assert.True(messageRetryQueue.RetryQueueLength == 1); + Assert.True(messageRetryQueue.CurrentQueueLength == 1); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); } @@ -80,7 +80,7 @@ public void TryHandleFailedMessageTimedoutEndPointAvailable() messageRetryQueue.TryHandleFailedCommand(message); - Assert.True(messageRetryQueue.RetryQueueLength == 0); + Assert.True(messageRetryQueue.CurrentQueueLength == 0); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); messageRetryHelper.Verify(failedCommand => failedCommand.SetExceptionAndComplete(message, timeout), Times.Once); } @@ -94,7 +94,7 @@ public void TryHandleFailedMessageGetEndpointThrows() messageRetryQueue.TryHandleFailedCommand(message); - Assert.True(messageRetryQueue.RetryQueueLength == 0); + Assert.True(messageRetryQueue.CurrentQueueLength == 0); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); messageRetryHelper.Verify(failedCommand => failedCommand.SetExceptionAndComplete(message, ex), Times.Once); } @@ -108,7 +108,7 @@ public void TryHandleFailedMessageDrainsQueue() messageRetryQueue.TryHandleFailedCommand(message); messageRetryQueue.Dispose(); - Assert.True(messageRetryQueue.RetryQueueLength == 0); + Assert.True(messageRetryQueue.CurrentQueueLength == 0); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); messageRetryHelper.Verify(failedCommand => failedCommand.SetExceptionAndComplete(message, It.IsAny()), Times.Once); } @@ -125,7 +125,7 @@ public void CheckRetryForTimeoutTimesout(bool hasTimedout, int queueLength) messageRetryQueue.TryHandleFailedCommand(message); messageRetryQueue.CheckRetryQueueForTimeouts(); - Assert.Equal(queueLength, messageRetryQueue.RetryQueueLength); + Assert.Equal(queueLength, messageRetryQueue.CurrentQueueLength); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); messageRetryHelper.Verify(failedCommand => failedCommand.SetExceptionAndComplete(message, It.IsAny()), Times.Exactly(hasTimedout ? 1 : 0)); } @@ -140,7 +140,7 @@ public void TryHandleFailedMessageTimeoutThrow() messageRetryQueue.TryHandleFailedCommand(message); - Assert.True(messageRetryQueue.RetryQueueLength == 0); + Assert.True(messageRetryQueue.CurrentQueueLength == 0); messageRetryHelper.Verify(failedCommand => failedCommand.TryResendAsync(message), Times.Never); messageRetryHelper.Verify(failedCommand => failedCommand.SetExceptionAndComplete(message, It.IsAny()), Times.Once); } diff --git a/tests/StackExchange.Redis.Tests/TestBase.cs b/tests/StackExchange.Redis.Tests/TestBase.cs index 6477d4e8c..5dcef2af7 100644 --- a/tests/StackExchange.Redis.Tests/TestBase.cs +++ b/tests/StackExchange.Redis.Tests/TestBase.cs @@ -229,7 +229,7 @@ internal virtual IInternalConnectionMultiplexer Create( bool checkConnect = true, string failMessage = null, string channelPrefix = null, Proxy? proxy = null, string configuration = null, bool logTransactionData = true, - bool shared = true, int? defaultDatabase = null, IRetryOnReconnectPolicy retryPolicy = null, + bool shared = true, int? defaultDatabase = null, ICommandRetryPolicy retryPolicy = null, [CallerMemberName] string caller = null) { if (Output == null) @@ -270,7 +270,7 @@ public static ConnectionMultiplexer CreateDefault( string channelPrefix = null, Proxy? proxy = null, string configuration = null, bool logTransactionData = true, int? defaultDatabase = null, - IRetryOnReconnectPolicy retryPolicy = null, + ICommandRetryPolicy retryPolicy = null, [CallerMemberName] string caller = null) { StringWriter localLog = null; @@ -306,7 +306,7 @@ public static ConnectionMultiplexer CreateDefault( if (connectTimeout != null) config.ConnectTimeout = connectTimeout.Value; if (proxy != null) config.Proxy = proxy.Value; if (defaultDatabase != null) config.DefaultDatabase = defaultDatabase.Value; - if (retryPolicy != null) config.RetryCommandsOnReconnect = retryPolicy; + if (retryPolicy != null) config.CommandRetryPolicy = retryPolicy; var watch = Stopwatch.StartNew(); var task = ConnectionMultiplexer.ConnectAsync(config, log); if (!task.Wait(config.ConnectTimeout >= (int.MaxValue / 2) ? int.MaxValue : config.ConnectTimeout * 2))