nhibernate/NHibernate-Caches

Issues with custom Regions with RedisCacheProvider

gokhanabatay opened this issue · 4 comments

Hi @maca88 ,
Because of "RedisCacheProvider.ConfiguredCacheRegions" is private readonly I cannot specify region based expiration with hibernate config file, give us a way to implement custom configuration.
If we want to write our RedisCacheProvider we are nearly copying this project.

Below config option is not enough with .net core add support for region configuration like "ConfigureRegions(RegionConfig[] regions)"

NHibernate.Caches.StackExchangeRedis

        static RedisCacheProvider()
        {
            Log = NHibernateLogger.For(typeof(RedisCacheProvider));
            ConfiguredCacheRegions = new Dictionary<string, RegionConfig>();

            if (!(ConfigurationManager.GetSection("redis") is CacheConfig config))
                return;

            ConfiguredCache = config;
	    foreach (var cache in config.Regions)
	    {
			ConfiguredCacheRegions.Add(cache.Region, cache);
	    }
        }

Also if we specify "cache.region_prefix" in hibernate configuration file it is added twice to key.
One is added in Nhibernate code the other one is NHibernate.Caches.StackExchangeRedis;

NHibernate

                internal string GetFullCacheRegionName(string name)
		{
			var prefix = CacheRegionPrefix;
			if (!string.IsNullOrEmpty(prefix))
				return prefix + '.' + name;
			return name;
		}

NHibernate.Caches.StackExchangeRedis

        private void BuildDefaultConfiguration(IDictionary<string, string> properties)
        {
            var config = CacheConfiguration;

            config.CacheKeyPrefix = GetString(RedisEnvironment.KeyPrefix, properties, config.CacheKeyPrefix);
            Log.Debug("Cache key prefix: {0}", config.CacheKeyPrefix);

            config.EnvironmentName = GetString(RedisEnvironment.EnvironmentName, properties, config.EnvironmentName);
            Log.Debug("Cache environment name: {0}", config.EnvironmentName);

            config.RegionPrefix = GetString(Cfg.Environment.CacheRegionPrefix, properties, config.RegionPrefix);
            // if region name contains region prefix so do not add to RegionPrefix, because it's triggered from Nhibernate code base?
        .
        .
        .
        }

Thanks for the detailed report, I've made a PR that resolves both issues.

Your welcome to allow override of virtual methods please make _connectionMultiplexer protected.

Also thank you for quick response.

 protected override CacheBase BuildCache(RedisCacheRegionConfiguration regionConfiguration, IDictionary<string, string> properties)
        {
            var connectionMultiplexer = this.GetType().GetField("_connectionMultiplexer", BindingFlags.Instance |
                                                                  BindingFlags.Public |
                                                                  BindingFlags.NonPublic).GetValue(this) as IConnectionMultiplexer;
            var regionStrategy =
                CacheConfiguration.RegionStrategyFactory.Create(connectionMultiplexer, regionConfiguration, properties);

            regionStrategy.Validate();

            return new RedisInMemoryCache(regionConfiguration.RegionName, regionStrategy);
        }

Also there is another bug I want to set zero expiration for UpdateTimestampsCache but;

ExpirationEnabled needs to be Expiration == TimeSpan.Zero;

                /// <summary>
		/// Is the expiration enabled?
		/// </summary>
		public bool ExpirationEnabled => Expiration != TimeSpan.Zero;

                public override void Validate()
		{
			if (!ExpirationEnabled)
			{
				throw new CacheException($"Expiration must be greater than zero for cache region: '{RegionName}'");
			}
		}

Also there is another bug I want to set zero expiration

It is not a bug but as this validation was intentionally added for DefaultRegionStrategy. The reason is because DefaultRegionStrategy will use a different key prefix (increments the version number) when the ICache.Clear operation is triggered which means that the same cache key will be stored in a different location after the ICache.Clear operation. Setting a zero expiration for a DefaultRegionStrategy could produce an out of memory exception as the keys of the previous versions does not expire. If your application does not produce the ICache.Clear method to be triggered you can use FastRegionStrategy which supports having a zero expiration.