Backport to net8 - Caching: SE.Redis update and fix naming inconsistency (#54413)

mgravell · web-flow · commit d638bba69440 · 2024-03-12T10:12:56.000-07:00
* Caching: SE.Redis update and fix naming inconsistency (#54239) * - rev to SE.Redis 2.7.27 - use new AddLibraryNameSuffix API (2.7.27) - do not use ConfigurationOptions.Clone() - impacts key rotation - attach suffix even if ConnectionMultiplexerFactory used (Aspire) * nit extra ;
diff --git a/eng/Versions.props b/eng/Versions.props
@@ -318,7 +318,7 @@
     <SeleniumWebDriverVersion>4.17.0</SeleniumWebDriverVersion>
     <SerilogExtensionsLoggingVersion>1.4.0</SerilogExtensionsLoggingVersion>
     <SerilogSinksFileVersion>4.0.0</SerilogSinksFileVersion>
-    <StackExchangeRedisVersion>2.6.122</StackExchangeRedisVersion>
+    <StackExchangeRedisVersion>2.7.27</StackExchangeRedisVersion>
     <SystemReactiveLinqVersion>5.0.0</SystemReactiveLinqVersion>
     <SwashbuckleAspNetCoreVersion>6.4.0</SwashbuckleAspNetCoreVersion>
     <XunitAbstractionsVersion>2.0.3</XunitAbstractionsVersion>
diff --git a/src/Caching/StackExchangeRedis/src/RedisCache.Log.cs b/src/Caching/StackExchangeRedis/src/RedisCache.Log.cs
@@ -12,5 +12,8 @@ private static partial class Log
     {
         [LoggerMessage(1, LogLevel.Warning, "Could not determine the Redis server version. Falling back to use HMSET command instead of HSET.", EventName = "CouldNotDetermineServerVersion")]
         public static partial void CouldNotDetermineServerVersion(ILogger logger, Exception exception);
+
+        [LoggerMessage(2, LogLevel.Debug, "Unable to add library name suffix.", EventName = "UnableToAddLibraryNameSuffix")]
+        internal static partial void UnableToAddLibraryNameSuffix(ILogger logger, Exception exception);
     }
 }
diff --git a/src/Caching/StackExchangeRedis/src/RedisCache.cs b/src/Caching/StackExchangeRedis/src/RedisCache.cs
@@ -257,14 +257,7 @@ private IDatabase Connect()
                 IConnectionMultiplexer connection;
                 if (_options.ConnectionMultiplexerFactory is null)
                 {
-                    if (_options.ConfigurationOptions is not null)
-                    {
-                        connection = ConnectionMultiplexer.Connect(_options.ConfigurationOptions);
-                    }
-                    else
-                    {
-                        connection = ConnectionMultiplexer.Connect(_options.Configuration!);
-                    }
+                    connection = ConnectionMultiplexer.Connect(_options.GetConfiguredOptions());
                 }
                 else
                 {
@@ -308,7 +301,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)
                 IConnectionMultiplexer connection;
                 if (_options.ConnectionMultiplexerFactory is null)
                 {
-                    connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net DC")).ConfigureAwait(false);
+                    connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions()).ConfigureAwait(false);
                 }
                 else
                 {
@@ -332,6 +325,7 @@ private void PrepareConnection(IConnectionMultiplexer connection)
         WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow);
         ValidateServerFeatures(connection);
         TryRegisterProfiler(connection);
+        TryAddSuffix(connection);
     }
 
     private void ValidateServerFeatures(IConnectionMultiplexer connection)
@@ -369,6 +363,19 @@ private void TryRegisterProfiler(IConnectionMultiplexer connection)
         }
     }
 
+    private void TryAddSuffix(IConnectionMultiplexer connection)
+    {
+        try
+        {
+            connection.AddLibraryNameSuffix("aspnet");
+            connection.AddLibraryNameSuffix("DC");
+        }
+        catch (Exception ex)
+        {
+            Log.UnableToAddLibraryNameSuffix(_logger, ex);
+        }
+    }
+
     private byte[]? GetAndRefresh(string key, bool getData)
     {
         ArgumentNullThrowHelper.ThrowIfNull(key);
diff --git a/src/Caching/StackExchangeRedis/src/RedisCacheOptions.cs b/src/Caching/StackExchangeRedis/src/RedisCacheOptions.cs
@@ -59,18 +59,13 @@ static bool GetDefaultValue() =>
         set => _useForceReconnect = value;
     }
 
-    internal ConfigurationOptions GetConfiguredOptions(string libSuffix)
+    internal ConfigurationOptions GetConfiguredOptions()
     {
-        var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!);
+        var options = ConfigurationOptions ?? ConfigurationOptions.Parse(Configuration!);
 
         // we don't want an initially unavailable server to prevent DI creating the service itself
         options.AbortOnConnectFail = false;
 
-        if (!string.IsNullOrWhiteSpace(libSuffix))
-        {
-            var provider = DefaultOptionsProvider.GetProvider(options.EndPoints);
-            options.LibraryName = $"{provider.LibraryName} {libSuffix}";
-        }
         return options;
     }
 }
diff --git a/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheOptions.cs b/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheOptions.cs
@@ -54,18 +54,13 @@ static bool GetDefaultValue() =>
         set => _useForceReconnect = value;
     }
 
-    internal ConfigurationOptions GetConfiguredOptions(string libSuffix)
+    internal ConfigurationOptions GetConfiguredOptions()
     {
-        var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!);
+        var options = ConfigurationOptions ?? ConfigurationOptions.Parse(Configuration!);
 
         // we don't want an initially unavailable server to prevent DI creating the service itself
         options.AbortOnConnectFail = false;
 
-        if (!string.IsNullOrWhiteSpace(libSuffix))
-        {
-            var provider = DefaultOptionsProvider.GetProvider(options.EndPoints);
-            options.LibraryName = $"{provider.LibraryName} {libSuffix}";
-        }
         return options;
     }
 }
diff --git a/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheStore.Log.cs b/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheStore.Log.cs
@@ -14,4 +14,7 @@ internal partial class RedisOutputCacheStore
 
     [LoggerMessage(2, LogLevel.Error, "Fatal error occurred executing redis output-cache GC loop.", EventName = "RedisOutputCacheGCFatalError")]
     internal static partial void RedisOutputCacheGCFatalError(ILogger logger, Exception exception);
+
+    [LoggerMessage(3, LogLevel.Debug, "Unable to add library name suffix.", EventName = "UnableToAddLibraryNameSuffix")]
+    internal static partial void UnableToAddLibraryNameSuffix(ILogger logger, Exception exception);
 }
diff --git a/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheStore.cs b/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/src/RedisOutputCacheStore.cs
@@ -332,7 +332,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)
                 IConnectionMultiplexer connection;
                 if (_options.ConnectionMultiplexerFactory is null)
                 {
-                    connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net OC")).ConfigureAwait(false);
+                    connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions()).ConfigureAwait(false);
                 }
                 else
                 {
@@ -415,6 +415,7 @@ private void PrepareConnection(IConnectionMultiplexer connection)
         WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow);
         ValidateServerFeatures(connection);
         TryRegisterProfiler(connection);
+        TryAddSuffix(connection);
     }
 
     private void ValidateServerFeatures(IConnectionMultiplexer connection)
@@ -451,6 +452,19 @@ private void TryRegisterProfiler(IConnectionMultiplexer connection)
         }
     }
 
+    private void TryAddSuffix(IConnectionMultiplexer connection)
+    {
+        try
+        {
+            connection.AddLibraryNameSuffix("aspnet");
+            connection.AddLibraryNameSuffix("OC");
+        }
+        catch (Exception ex)
+        {
+            UnableToAddLibraryNameSuffix(_logger, ex);
+        }
+    }
+
     private static void WriteTimeTicks(ref long field, DateTimeOffset value)
     {
         var ticks = value == DateTimeOffset.MinValue ? 0L : value.UtcTicks;
diff --git a/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/test/RedisConnectionFixture.cs b/src/Middleware/Microsoft.AspNetCore.OutputCaching.StackExchangeRedis/test/RedisConnectionFixture.cs
@@ -13,8 +13,9 @@ public RedisConnectionFixture()
         var options = new RedisOutputCacheOptions
         {
             Configuration = "127.0.0.1:6379", // TODO: CI test config here
-        }.GetConfiguredOptions("CI test");
+        }.GetConfiguredOptions();
         _muxer = ConnectionMultiplexer.Connect(options);
+        _muxer.AddLibraryNameSuffix("test");
     }
 
     public IDatabase Database => _muxer.GetDatabase();
diff --git a/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs b/src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs
@@ -13,6 +13,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using StackExchange.Redis;
+using RedisProtocol = Microsoft.AspNetCore.SignalR.StackExchangeRedis.Internal.RedisProtocol; // to disambiguate from StackExchange.Redis.RedisProtocol
 
 namespace Microsoft.AspNetCore.SignalR.StackExchangeRedis;
 
diff --git a/src/SignalR/server/StackExchangeRedis/test/TestConnectionMultiplexer.cs b/src/SignalR/server/StackExchangeRedis/test/TestConnectionMultiplexer.cs
@@ -1,19 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
 using System.Collections.Concurrent;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.IO;
 using System.Net;
 using System.Reflection;
-using System.Threading;
-using System.Threading.Tasks;
 using StackExchange.Redis;
 using StackExchange.Redis.Maintenance;
 using StackExchange.Redis.Profiling;
-using Xunit;
 
 namespace Microsoft.AspNetCore.SignalR.Tests;
 
@@ -237,6 +230,8 @@ public IServer[] GetServers()
     }
 
     public ValueTask DisposeAsync() => default;
+
+    public void AddLibraryNameSuffix(string suffix) { } // don't need to implement
 }
 
 public class TestRedisServer

Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,8 @@ private static partial class Log`
`12`	`12`	`{`
`13`	`13`	`[LoggerMessage(1, LogLevel.Warning, "Could not determine the Redis server version. Falling back to use HMSET command instead of HSET.", EventName = "CouldNotDetermineServerVersion")]`
`14`	`14`	`public static partial void CouldNotDetermineServerVersion(ILogger logger, Exception exception);`
	`15`	`+`
	`16`	`+ [LoggerMessage(2, LogLevel.Debug, "Unable to add library name suffix.", EventName = "UnableToAddLibraryNameSuffix")]`
	`17`	`+ internal static partial void UnableToAddLibraryNameSuffix(ILogger logger, Exception exception);`
`15`	`18`	`}`
`16`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -257,14 +257,7 @@ private IDatabase Connect()`
`257`	`257`	`IConnectionMultiplexer connection;`
`258`	`258`	`if (_options.ConnectionMultiplexerFactory is null)`
`259`	`259`	`{`
`260`		`- if (_options.ConfigurationOptions is not null)`
`261`		`- {`
`262`		`- connection = ConnectionMultiplexer.Connect(_options.ConfigurationOptions);`
`263`		`- }`
`264`		`- else`
`265`		`- {`
`266`		`- connection = ConnectionMultiplexer.Connect(_options.Configuration!);`
`267`		`- }`
	`260`	`+ connection = ConnectionMultiplexer.Connect(_options.GetConfiguredOptions());`
`268`	`261`	`}`
`269`	`262`	`else`
`270`	`263`	`{`
`@@ -308,7 +301,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)`
`308`	`301`	`IConnectionMultiplexer connection;`
`309`	`302`	`if (_options.ConnectionMultiplexerFactory is null)`
`310`	`303`	`{`
`311`		`- connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net DC")).ConfigureAwait(false);`
	`304`	`+ connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions()).ConfigureAwait(false);`
`312`	`305`	`}`
`313`	`306`	`else`
`314`	`307`	`{`
`@@ -332,6 +325,7 @@ private void PrepareConnection(IConnectionMultiplexer connection)`
`332`	`325`	`WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow);`
`333`	`326`	`ValidateServerFeatures(connection);`
`334`	`327`	`TryRegisterProfiler(connection);`
	`328`	`+ TryAddSuffix(connection);`
`335`	`329`	`}`
`336`	`330`
`337`	`331`	`private void ValidateServerFeatures(IConnectionMultiplexer connection)`
`@@ -369,6 +363,19 @@ private void TryRegisterProfiler(IConnectionMultiplexer connection)`
`369`	`363`	`}`
`370`	`364`	`}`
`371`	`365`
	`366`	`+ private void TryAddSuffix(IConnectionMultiplexer connection)`
	`367`	`+ {`
	`368`	`+ try`
	`369`	`+ {`
	`370`	`+ connection.AddLibraryNameSuffix("aspnet");`
	`371`	`+ connection.AddLibraryNameSuffix("DC");`
	`372`	`+ }`
	`373`	`+ catch (Exception ex)`
	`374`	`+ {`
	`375`	`+ Log.UnableToAddLibraryNameSuffix(_logger, ex);`
	`376`	`+ }`
	`377`	`+ }`
	`378`	`+`
`372`	`379`	`private byte[]? GetAndRefresh(string key, bool getData)`
`373`	`380`	`{`
`374`	`381`	`ArgumentNullThrowHelper.ThrowIfNull(key);`
Original file line number	Diff line number	Diff line change
`@@ -59,18 +59,13 @@ static bool GetDefaultValue() =>`
`59`	`59`	`set => _useForceReconnect = value;`
`60`	`60`	`}`
`61`	`61`
`62`		`- internal ConfigurationOptions GetConfiguredOptions(string libSuffix)`
	`62`	`+ internal ConfigurationOptions GetConfiguredOptions()`
`63`	`63`	`{`
`64`		`- var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!);`
	`64`	`+ var options = ConfigurationOptions ?? ConfigurationOptions.Parse(Configuration!);`
`65`	`65`
`66`	`66`	`// we don't want an initially unavailable server to prevent DI creating the service itself`
`67`	`67`	`options.AbortOnConnectFail = false;`
`68`	`68`
`69`		`- if (!string.IsNullOrWhiteSpace(libSuffix))`
`70`		`- {`
`71`		`- var provider = DefaultOptionsProvider.GetProvider(options.EndPoints);`
`72`		`- options.LibraryName = $"{provider.LibraryName} {libSuffix}";`
`73`		`- }`
`74`	`69`	`return options;`
`75`	`70`	`}`
`76`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,18 +54,13 @@ static bool GetDefaultValue() =>`
`54`	`54`	`set => _useForceReconnect = value;`
`55`	`55`	`}`
`56`	`56`
`57`		`- internal ConfigurationOptions GetConfiguredOptions(string libSuffix)`
	`57`	`+ internal ConfigurationOptions GetConfiguredOptions()`
`58`	`58`	`{`
`59`		`- var options = ConfigurationOptions?.Clone() ?? ConfigurationOptions.Parse(Configuration!);`
	`59`	`+ var options = ConfigurationOptions ?? ConfigurationOptions.Parse(Configuration!);`
`60`	`60`
`61`	`61`	`// we don't want an initially unavailable server to prevent DI creating the service itself`
`62`	`62`	`options.AbortOnConnectFail = false;`
`63`	`63`
`64`		`- if (!string.IsNullOrWhiteSpace(libSuffix))`
`65`		`- {`
`66`		`- var provider = DefaultOptionsProvider.GetProvider(options.EndPoints);`
`67`		`- options.LibraryName = $"{provider.LibraryName} {libSuffix}";`
`68`		`- }`
`69`	`64`	`return options;`
`70`	`65`	`}`
`71`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,4 +14,7 @@ internal partial class RedisOutputCacheStore`
`14`	`14`
`15`	`15`	`[LoggerMessage(2, LogLevel.Error, "Fatal error occurred executing redis output-cache GC loop.", EventName = "RedisOutputCacheGCFatalError")]`
`16`	`16`	`internal static partial void RedisOutputCacheGCFatalError(ILogger logger, Exception exception);`
	`17`	`+`
	`18`	`+ [LoggerMessage(3, LogLevel.Debug, "Unable to add library name suffix.", EventName = "UnableToAddLibraryNameSuffix")]`
	`19`	`+ internal static partial void UnableToAddLibraryNameSuffix(ILogger logger, Exception exception);`
`17`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,7 @@ private async ValueTask<IDatabase> ConnectSlowAsync(CancellationToken token)`
`332`	`332`	`IConnectionMultiplexer connection;`
`333`	`333`	`if (_options.ConnectionMultiplexerFactory is null)`
`334`	`334`	`{`
`335`		`- connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions("asp.net OC")).ConfigureAwait(false);`
	`335`	`+ connection = await ConnectionMultiplexer.ConnectAsync(_options.GetConfiguredOptions()).ConfigureAwait(false);`
`336`	`336`	`}`
`337`	`337`	`else`
`338`	`338`	`{`
`@@ -415,6 +415,7 @@ private void PrepareConnection(IConnectionMultiplexer connection)`
`415`	`415`	`WriteTimeTicks(ref _lastConnectTicks, DateTimeOffset.UtcNow);`
`416`	`416`	`ValidateServerFeatures(connection);`
`417`	`417`	`TryRegisterProfiler(connection);`
	`418`	`+ TryAddSuffix(connection);`
`418`	`419`	`}`
`419`	`420`
`420`	`421`	`private void ValidateServerFeatures(IConnectionMultiplexer connection)`
`@@ -451,6 +452,19 @@ private void TryRegisterProfiler(IConnectionMultiplexer connection)`
`451`	`452`	`}`
`452`	`453`	`}`
`453`	`454`
	`455`	`+ private void TryAddSuffix(IConnectionMultiplexer connection)`
	`456`	`+ {`
	`457`	`+ try`
	`458`	`+ {`
	`459`	`+ connection.AddLibraryNameSuffix("aspnet");`
	`460`	`+ connection.AddLibraryNameSuffix("OC");`
	`461`	`+ }`
	`462`	`+ catch (Exception ex)`
	`463`	`+ {`
	`464`	`+ UnableToAddLibraryNameSuffix(_logger, ex);`
	`465`	`+ }`
	`466`	`+ }`
	`467`	`+`
`454`	`468`	`private static void WriteTimeTicks(ref long field, DateTimeOffset value)`
`455`	`469`	`{`
`456`	`470`	`var ticks = value == DateTimeOffset.MinValue ? 0L : value.UtcTicks;`
Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,9 @@ public RedisConnectionFixture()`
`13`	`13`	`var options = new RedisOutputCacheOptions`
`14`	`14`	`{`
`15`	`15`	`Configuration = "127.0.0.1:6379", // TODO: CI test config here`
`16`		`- }.GetConfiguredOptions("CI test");`
	`16`	`+ }.GetConfiguredOptions();`
`17`	`17`	`_muxer = ConnectionMultiplexer.Connect(options);`
	`18`	`+ _muxer.AddLibraryNameSuffix("test");`
`18`	`19`	`}`
`19`	`20`
`20`	`21`	`public IDatabase Database => _muxer.GetDatabase();`