Various improvements to Redis providers (#8261)

* Various improvements to Redis providers

* Use consistent configuration pattern
* Add consistent configuration validators
* Use unique key prefixes for all providers
* Add expiry to all keys for testing
* Consistently throw serializable exceptions
* Update doc comments
* Mark classes which do not need to be public as internal
* Other minor cleanup

* Review feedback

* Convert RedisReminderTable.UpsertRow implementation to Lua script instead of Redis transaction

* Fixes for RedisGrainStorage

src/Redis/Orleans.Clustering.Redis/Hosting/HostingExtensions.ICientBuilder.cs

@@ -1,9 +1,10 @@
-using System;
+using System;
 using Orleans;
 using Microsoft.Extensions.DependencyInjection;
 using Orleans.Hosting;
 using Orleans.Messaging;
 using Orleans.Clustering.Redis;
+using StackExchange.Redis;
 
 namespace Microsoft.Extensions.Hosting
 {
@@ -25,23 +26,22 @@ public static IClientBuilder UseRedisClustering(this IClientBuilder builder, Act
                 }
 
                 services
-                    .AddRedis()
+                    .AddRedisClustering()
                     .AddSingleton<IGatewayListProvider, RedisGatewayListProvider>();
             });
         }
 
         /// <summary>
         /// Configures Redis as the clustering provider.
         /// </summary>
-        public static IClientBuilder UseRedisClustering(this IClientBuilder builder, string redisConnectionString, int db = 0)
+        public static IClientBuilder UseRedisClustering(this IClientBuilder builder, string redisConnectionString)
         {
             return builder.ConfigureServices(services => services
                 .Configure<RedisClusteringOptions>(opt =>
                 {
-                    opt.ConnectionString = redisConnectionString;
-                    opt.Database = db;
+                    opt.ConfigurationOptions = ConfigurationOptions.Parse(redisConnectionString);
                 })
-                .AddRedis()
+                .AddRedisClustering()
                 .AddSingleton<IGatewayListProvider, RedisGatewayListProvider>());
         }
 

src/Redis/Orleans.Clustering.Redis/Hosting/HostingExtensions.ISiloBuilder.cs

@@ -1,8 +1,9 @@
-using System;
+using System;
 using Orleans;
 using Microsoft.Extensions.DependencyInjection;
 using Orleans.Hosting;
 using Orleans.Clustering.Redis;
+using StackExchange.Redis;
 
 namespace Microsoft.Extensions.Hosting
 {
@@ -23,23 +24,27 @@ public static ISiloBuilder UseRedisClustering(this ISiloBuilder builder, Action<
                     services.Configure(configuration);
                 }
 
-                services.AddRedis();
+                services.AddRedisClustering();
             });
         }
 
         /// <summary>
         /// Configures Redis as the clustering provider.
         /// </summary>
-        public static ISiloBuilder UseRedisClustering(this ISiloBuilder builder, string redisConnectionString, int db = 0)
+        public static ISiloBuilder UseRedisClustering(this ISiloBuilder builder, string redisConnectionString)
         {
             return builder.ConfigureServices(services => services
-                .Configure<RedisClusteringOptions>(options => { options.Database = db; options.ConnectionString = redisConnectionString; })
-                .AddRedis());
+                .Configure<RedisClusteringOptions>(options =>
+                {
+                    options.ConfigurationOptions = ConfigurationOptions.Parse(redisConnectionString);
+                })
+                .AddRedisClustering());
         }
 
-        internal static IServiceCollection AddRedis(this IServiceCollection services)
+        internal static IServiceCollection AddRedisClustering(this IServiceCollection services)
         {
             services.AddSingleton<RedisMembershipTable>();
+            services.AddSingleton<IConfigurationValidator, RedisClusteringOptionsValidator>();
             services.AddSingleton<IMembershipTable>(sp => sp.GetRequiredService<RedisMembershipTable>());
             return services;
         }

src/Redis/Orleans.Clustering.Redis/Orleans.Clustering.Redis.csproj

@@ -7,6 +7,7 @@
     <PackageTags>$(PackageTags) Redis Clustering</PackageTags>
     <TargetFrameworks>$(DefaultTargetFrameworks)</TargetFrameworks>
     <VersionSuffix Condition="$(VersionSuffix) == ''">beta1</VersionSuffix>
+    <OrleansBuildTimeCodeGen>true</OrleansBuildTimeCodeGen>
   </PropertyGroup>
 
   <ItemGroup>

src/Redis/Orleans.Clustering.Redis/Providers/RedisClusteringOptions.cs

@@ -1,3 +1,4 @@
+using Orleans.Runtime;
 using StackExchange.Redis;
 using System;
 using System.Threading.Tasks;
@@ -10,26 +11,55 @@ namespace Orleans.Clustering.Redis
     public class RedisClusteringOptions
     {
         /// <summary>
-        /// Specifies the database identi
+        /// Gets or sets the Redis client configuration.
         /// </summary>
-        public int Database { get; set; }
+        [RedactRedisConfigurationOptions]
+        public ConfigurationOptions ConfigurationOptions { get; set; }
 
         /// <summary>
-        /// The connection string.
+        /// The delegate used to create a Redis connection multiplexer.
         /// </summary>
-        public string ConnectionString { get; set; } = "localhost:6379";
+        public Func<RedisClusteringOptions, Task<IConnectionMultiplexer>> CreateMultiplexer { get; set; } = DefaultCreateMultiplexer;
 
         /// <summary>
-        /// The delegate used to create a Redis connection multiplexer.
+        /// Entry expiry, null by default. A value should be set ONLY for ephemeral environments (like in tests).
+        /// Setting a value different from null will cause entries to be deleted after some period of time.
         /// </summary>
-        public Func<RedisClusteringOptions, Task<IConnectionMultiplexer>> CreateMultiplexer { get; set; } = DefaultCreateMultiplexer;
+        public TimeSpan? EntryExpiry { get; set; } = null;
 
         /// <summary>
         /// The default multiplexer creation delegate.
         /// </summary>
         public static async Task<IConnectionMultiplexer> DefaultCreateMultiplexer(RedisClusteringOptions options)
         {
-            return await ConnectionMultiplexer.ConnectAsync(options.ConnectionString);
+            return await ConnectionMultiplexer.ConnectAsync(options.ConfigurationOptions);
+        }
+    }
+
+    internal class RedactRedisConfigurationOptions : RedactAttribute
+    {
+        public override string Redact(object value) => value is ConfigurationOptions cfg ? cfg.ToString(includePassword: false) : base.Redact(value);
+    }
+
+    /// <summary>
+    /// Configuration validator for <see cref="RedisClusteringOptions"/>.
+    /// </summary>
+    public class RedisClusteringOptionsValidator : IConfigurationValidator
+    {
+        private readonly RedisClusteringOptions _options;
+
+        public RedisClusteringOptionsValidator(RedisClusteringOptions options)
+        {
+            _options = options;
+        }
+
+        /// <inheritdoc/>
+        public void ValidateConfiguration()
+        {
+            if (_options.ConfigurationOptions == null)
+            {
+                throw new OrleansConfigurationException($"Invalid configuration for {nameof(RedisMembershipTable)}. {nameof(RedisClusteringOptions)}.{nameof(_options.ConfigurationOptions)} is required.");
+            }
         }
     }
 }

src/Redis/Orleans.Clustering.Redis/Storage/RedisMembershipTable.cs

@@ -6,8 +6,8 @@
 using Newtonsoft.Json;
 using System.Linq;
 using Microsoft.Extensions.Options;
-using System.Runtime.CompilerServices;
 using System.Globalization;
+using System.Text;
 
 namespace Orleans.Clustering.Redis
 {
@@ -26,7 +26,7 @@ public RedisMembershipTable(IOptions<RedisClusteringOptions> redisOptions, IOpti
         {
             _redisOptions = redisOptions.Value;
             _clusterOptions = clusterOptions.Value;
-            _clusterKey = $"{_clusterOptions.ServiceId}/{_clusterOptions.ClusterId}";
+            _clusterKey = Encoding.UTF8.GetBytes($"{_clusterOptions.ServiceId}/members/{_clusterOptions.ClusterId}");
             _jsonSerializerSettings = JsonSettings.JsonSerializerSettings;
         }
 
@@ -40,11 +40,16 @@ public async Task DeleteMembershipTableEntries(string clusterId)
         public async Task InitializeMembershipTable(bool tryInitTableVersion)
         {
             _muxer = await _redisOptions.CreateMultiplexer(_redisOptions);
-            _db = _muxer.GetDatabase(_redisOptions.Database);
+            _db = _muxer.GetDatabase();
 
             if (tryInitTableVersion)
             {
                 await _db.HashSetAsync(_clusterKey, TableVersionKey, SerializeVersion(DefaultTableVersion), When.NotExists);
+
+                if (_redisOptions.EntryExpiry is { } expiry)
+                {
+                    await _db.KeyExpireAsync(_clusterKey, expiry);
+                }
             }
 
             this.IsInitialized = true;

src/Redis/Orleans.GrainDirectory.Redis/Hosting/RedisGrainDirectoryExtensions.cs

@@ -8,10 +8,13 @@
 
 namespace Orleans.Hosting
 {
+    /// <summary>
+    /// Extensions for configuring Redis as a grain directory provider.
+    /// </summary>
     public static class RedisGrainDirectoryExtensions
     {
         /// <summary>
-        /// Use a Redis data-store as the default Grain Directory
+        /// Adds a default grain directory which persists entries in Redis.
         /// </summary>
         public static ISiloBuilder UseRedisGrainDirectoryAsDefault(
             this ISiloBuilder builder,
@@ -21,7 +24,7 @@ public static ISiloBuilder UseRedisGrainDirectoryAsDefault(
         }
 
         /// <summary>
-        /// Use a Redis data-store as the default Grain Directory
+        /// Adds a default grain directory which persists entries in Redis.
         /// </summary>
         public static ISiloBuilder UseRedisGrainDirectoryAsDefault(
             this ISiloBuilder builder,
@@ -31,7 +34,7 @@ public static ISiloBuilder UseRedisGrainDirectoryAsDefault(
         }
 
         /// <summary>
-        /// Add a Redis data-store as a named Grain Directory
+        /// Adds a named grain directory which persists entries in Redis.
         /// </summary>
         public static ISiloBuilder AddRedisGrainDirectory(
             this ISiloBuilder builder,
@@ -42,7 +45,7 @@ public static ISiloBuilder AddRedisGrainDirectory(
         }
 
         /// <summary>
-        /// Add a Redis data-store as a named Grain Directory
+        /// Adds a named grain directory which persists entries in Redis.
         /// </summary>
         public static ISiloBuilder AddRedisGrainDirectory(
             this ISiloBuilder builder,
@@ -59,7 +62,7 @@ private static IServiceCollection AddRedisGrainDirectory(
         {
             configureOptions.Invoke(services.AddOptions<RedisGrainDirectoryOptions>(name));
             services
-                .AddTransient<IConfigurationValidator>(sp => new RedisGrainDirectoryOptionsValidator(sp.GetRequiredService<IOptionsMonitor<RedisGrainDirectoryOptions>>().Get(name)))
+                .AddTransient<IConfigurationValidator>(sp => new RedisGrainDirectoryOptionsValidator(sp.GetRequiredService<IOptionsMonitor<RedisGrainDirectoryOptions>>().Get(name), name))
                 .ConfigureNamedOptionForLogging<RedisGrainDirectoryOptions>(name)
                 .AddSingletonNamedService<IGrainDirectory>(name, (sp, name) => ActivatorUtilities.CreateInstance<RedisGrainDirectory>(sp, sp.GetOptionsByName<RedisGrainDirectoryOptions>(name)))
                 .AddSingletonNamedService<ILifecycleParticipant<ISiloLifecycle>>(name, (s, n) => (ILifecycleParticipant<ISiloLifecycle>)s.GetRequiredServiceByName<IGrainDirectory>(n));

src/Redis/Orleans.GrainDirectory.Redis/Options/RedisGrainDirectoryOptions.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Threading.Tasks;
 using Orleans.GrainDirectory.Redis;
 using Orleans.Runtime;
 using StackExchange.Redis;
@@ -11,37 +12,53 @@ namespace Orleans.Configuration
     public class RedisGrainDirectoryOptions 
     {
         /// <summary>
-        /// Configure the Redis client
+        /// Gets or sets the Redis client configuration.
         /// </summary>
         [RedactRedisConfigurationOptions]
         public ConfigurationOptions ConfigurationOptions { get; set; }
 
+        /// <summary>
+        /// The delegate used to create a Redis connection multiplexer.
+        /// </summary>
+        public Func<RedisGrainDirectoryOptions, Task<IConnectionMultiplexer>> CreateMultiplexer { get; set; } = DefaultCreateMultiplexer;
+
         /// <summary>
         /// Entry expiry, null by default. A value should be set ONLY for ephemeral environments (like in tests).
         /// Setting a value different from null will cause duplicate activations in the cluster.
         /// </summary>
         public TimeSpan? EntryExpiry { get; set; } = null;
+
+        /// <summary>
+        /// The default multiplexer creation delegate.
+        /// </summary>
+        public static async Task<IConnectionMultiplexer> DefaultCreateMultiplexer(RedisGrainDirectoryOptions options) => await ConnectionMultiplexer.ConnectAsync(options.ConfigurationOptions);
     }
 
-    public class RedactRedisConfigurationOptions : RedactAttribute
+    internal class RedactRedisConfigurationOptions : RedactAttribute
     {
         public override string Redact(object value) => value is ConfigurationOptions cfg ? cfg.ToString(includePassword: false) : base.Redact(value);
     }
 
+    /// <summary>
+    /// Configuration validator for <see cref="RedisGrainDirectoryOptions"/>.
+    /// </summary>
     public class RedisGrainDirectoryOptionsValidator : IConfigurationValidator
     {
-        private readonly RedisGrainDirectoryOptions options;
+        private readonly RedisGrainDirectoryOptions _options;
+        private readonly string _name;
 
-        public RedisGrainDirectoryOptionsValidator(RedisGrainDirectoryOptions options)
+        public RedisGrainDirectoryOptionsValidator(RedisGrainDirectoryOptions options, string name)
         {
-            this.options = options;
+            _options = options;
+            _name = name;
         }
 
+        /// <inheritdoc/>
         public void ValidateConfiguration()
         {
-            if (this.options.ConfigurationOptions == null)
+            if (_options.ConfigurationOptions == null)
             {
-                throw new OrleansConfigurationException($"Invalid {nameof(RedisGrainDirectoryOptions)} values for {nameof(RedisGrainDirectory)}. {nameof(options.ConfigurationOptions)} is required.");
+                throw new OrleansConfigurationException($"Invalid configuration for {nameof(RedisGrainDirectory)} with name {_name}. {nameof(RedisGrainDirectoryOptions)}.{nameof(_options.ConfigurationOptions)} is required.");
             }
         }
     }

src/Redis/Orleans.GrainDirectory.Redis/Orleans.GrainDirectory.Redis.csproj

@@ -7,6 +7,7 @@
     <PackageTags>$(PackageTags) Redis Grain Directory</PackageTags>
     <TargetFrameworks>$(DefaultTargetFrameworks)</TargetFrameworks>
     <VersionSuffix Condition="$(VersionSuffix) == ''">beta1</VersionSuffix>
+    <OrleansBuildTimeCodeGen>true</OrleansBuildTimeCodeGen>
   </PropertyGroup>
 
   <ItemGroup>

src/Redis/Orleans.GrainDirectory.Redis/Properties/AssemblyInfo.cs

@@ -0,0 +1,3 @@
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Tester.Redis")]