Add Redis provider with batching support

[maca88]

This PR contains a Redis provider for NHibernate which uses StackExchange.Redis as the client. Key features:

• Supports batching based on the NHibernate PR that should be released in 5.2
• By default supports Clear operation
• Has the ability to use a faster region strategy that does not support Clear operation
• Supports Redis cluster by hash tagging the region key and correctly passing keys to lua scripts
• Can be extended to handle multiple individual Redis instances (test provided)
• Supports distributed locking

The async code was generated with a non yet released version that supports searching async counterparts in inherited types, used to generate async counterparts of IDatabase, located in IDatabaseAsync.
In comparison to the currently most popular provider, this provider does not suffer of producing a memory leak over time as it uses a different strategy where only one key per region is added without expiration that contains the current valid version (see DefaultRegionStrategy).

Open questions:
Should we provide a JSON serializer as it is stated that is faster than the currently default binary serializer and if yes, how?
What should be the final package name?

TODO:

• Wait for the NHibernate 5.2 release and update the code in order to use the IBatchableCache
• Define the final package name
• Add more logging
• Provide a JSON serializer? --> It won't be included
• Wait the AsyncGenerator to ship a new version and update it
• Add performance tests
• Make tests run on Appveyor
• Make ICache.Lock and ICache.Unlock thread safe

[gliljas]

Nice! I was doing the same thing, but as a fork of https://github.com/TheCloudlessSky/NHibernate.Caches.Redis. I think this feels a bit fresher, and there will be no conflict with that author.

I will review it and test it in a real project, which currently is stuck in NH4.

[fredericDelaporte]

I think we should not shorten the name of the library it uses. It should be NHibernate.Caches.StackExchangeRedis.

(Or maybe even NHibernate.Caches.StackExchangeRedisStrongName to tell it explicitly, and in case we end up at some point adding a version referencing the not strongly named one. (Some caches are already not strongly named in NHibernate.Caches, due to having non strongly named dependencies.) But since NHibernate is already strongly named, users have anyway already to deal with any issue caused by strong naming. So I do not think it would have much value to add a non strongly named version of the cache.)

Note that NHibernate.Caches.StackExchange.Redis (additional dot) is already taken on NuGet, and it could require at least one month to reclaim that name. (The process is to send a message to the NuGet owner asking for getting ownership with some rationals explaining why, and send a copy to NuGet support. Then in case of lack of response in one month, ask NuGet support to transfer ownership. That package here seems to have been published by someone else than its author, and the author is no more working on it: see ozzymcduff/NHibernate.Caches.StackExchange.Redis#1.)

About the JSON serializer, the main issue is testing it. Implementation should be as easy as referencing Newtonsoft Json.Net and using it directly without any additional options.
Documentation about this is a bit lacking, but the objects sent by NHibernate for being cached are arrays of basic types obtained by calling Disassemble on the NHibernate type of the thing to serialize. (By example, in case of entities, that does call in turn Disassemble on each of its cacheable property type on the property value, storing all that in an array.) The default disassemble implementation for basic types it to yield the underlying .Net typed value.
So for most cases, the default JSON serialization should do it.
But testing this is not well supported by current tests, since they all work without actually implying NHibernate assemble/disassemble logic. (There are no "full integration" tests with NHibernate, tests only works on the caching interfaces.)

But note that "basic types" also include System.Type (which is already an issue for binary serialization under anything else than netfx), System.Uri, System.Globalization.CultureInfo, System.Xml.XmlDocument (already not binary serializable), System.Xml.Linq.XDocument (not binary serializable either), and custom user types which may have similar issues. The easiest fix for the NHibernate type would be to override their Assemble and Disassemble for yielding a better cacheable representation. (Typically, simple strings for all those types! They already do that for storing in db.) I think this would be an acceptable gray area possible breaking change for 5.2. (Users of these types and of distributed caches or caches persisted to disk would have to make sure to clear their caches after upgrading.)
For the custom user type, that would be the user responsibility to implement Assemble and Disassemble for them to yield a representation serializable by default to JSON.
(SerializableType is not troublesome: its Disassemble yields the byte array representing it.)

[maca88]

As NHibernate is strongly named and does not have a StrongName postfix and NHibernate.Caches.StackExchange.Redis is occupied, I would vote for NHibernate.Caches.StackExchangeRedis.

After better looking at the current Assemble and Disassemble methods in NHibernate and also at their JSON serializer there is also the issue of having internal constructors for ObjectTypeCacheEntry (returned by AnyType), CacheEntry, CollectionCacheEntry which forces to have a custom JSON converter for those types.

[gliljas]

I was hoping for NHibernate.Caches.StackExchange.Redis, but NHibernate.Caches.StackExchangeRedis will do just fine.

[fredericDelaporte]

I have overlooked that the state was encapsulated in these CacheEntry classes, which are quite less serialization friendly. I think their fields should be made read-write, and a parameter-less constructor added.

There also seems to have an issue for many numeric types by default: Json.Net is said by your link as not preserving their exact type (which is not surprising with an object array), causing failures on assembling. So indeed a custom Json serialization would need to be provided. Maybe you will have better to just provide a way to inject a custom serializer.

Update: full list of custom classes sent as cache values: CacheEntry, CollectionCacheEntry, ObjectTypeCacheEntry, but also CachedItem (encapsulating previous classes) and CacheLock when using the strict read-write cache.
Those classes are not easily serializable with anything else than binary serialization. They do not expose their internal state through settable properties.
IList (concrete class List<object>) is also sent by the query cache.

IList and IDictionary (including good old Hashtable) may also be sent instead of the Entry instances, if "structured" caching is enabled (NHibernate.Cfg.Setting.IsStructuredCacheEntriesEnabled). So enabling structured caching could be an easier way to enable custom serialization: only CachedItem and CacheLock would still have to be dealt with. (Unless some cacheable queries yields an AnyType, I do not known if that is possible. If yes, ObjectTypeCacheEntry will not be "structured" when cached as a query result.)

[gliljas]

I think it's great that the ConnectionMultiplexer can be configured and instantiated like this. However, ConnectionMultiplexers should be "as singleton as possible". It would be nice to have a way to provide it programmatically, if there is already an instance that you want to use. This could be done like in the other provider (NHibernate.Caches.Redis), which has a static SetConnectionMultiplexer method, but even better would be a callback. Another callback could be used intercept GetDatabase.

[maca88]

In my opinion the right way to go is to have an additional constructor that takes the ConnectionMultiplexer as a parameter. I know that NHibernate currently does not have a simple way to set the IObjectsFactory which would allow us to use our favourite IoC container. My plan would be to provide a PR to decuple the IObjectsFactory from BytecodeProvider and then add the extra constructor with the ConnectionMultiplexer if you agree? This is an example how the final solution would look like:

container.RegisterSingleton(myConnectionMultiplexer);
Cfg.Environment.ObjectsFactory = new MyObjectsFactory(container);
... // build a session factory
// The constructor with the ConnectionMultiplexer parameter would be called by the IoC container

For GetDatabase I would rather provide a IRedisDatabaseProvider interface that would look like:

public interface IRedisDatabaseProvider {
  IDatabase GetDatabase(ConnectionMultiplexer connectionMultiplexer, int database)
}

which could be configured in the configuration.

[gliljas]

Yes, if the IoC experience gets better (there's so much static stuff in the initialization...), that would be a good way. Probably using IConnectionMultiplexer, instead of ConnectionMultiplexer, though.

[maca88]

I was wrong about saying that add an additional constructor overload is the way to as it is considered an anti-pattern. You mentioned about configuring it with a callback, but how would that callback be set, statically?
A possible solution to avoid having static configuration methods and multiple constructors would be to have an interface similar to what I proposed for the GetDatabase:

public interface IConnectionMultiplexerProvider {
  IConnectionMultiplexer GetConnectionMultiplexer(IDictionary<string, string> properties);
}

that could be configured by a NHibernate configuration property or by an IoC container, example with an IoC container:

// Create a custom connection multiplexer and register it as a singleton
IConnectionMultiplexer connectionMultiplexer = CreateConnection(); 
container.RegisterSingleton(connectionMultiplexer);

// MyConnectionMultiplexerProvider would have the IConnectionMultiplexer as parameter in the
// constructor that would be then used as a return value in GetConnectionMultiplexer method
container.Register<IConnectionMultiplexerProvider, MyConnectionMultiplexerProvider>(); 

// Setup the ObjectsFactory with a custom one that uses our favorite IoC container
Cfg.Environment.ObjectsFactory = new MyObjectsFactory(container);
nhConfig.BuildSessionFactory(); // build a session factory

[maca88]

Maybe you will have better to just provide a way to inject a custom serializer.

There is already a configuration to set the serializer (see IRedisSerializer) by using the NHibernate configuration property cache.serializer. So for now i think it would be better to leave the JSON serializer out of this PR and rather add in the documentation that providing a custom JSON serializer may increase the overall performance.

[fredericDelaporte]

Yes, agreed. And such a NHibernate compatible serializer could eventually be provided as maybe something like a NHibernate.Cache.Util.JsonSerializer package. (But it would not implement any specific cache implementation interface, since it would have to be independent of a specific cache implementation.)

Update: there are more classes to handle than the Entry ones, see the updated comment.

[maca88]

I've added tests for serialization of NHibernate types and as expected they throws for types that @fredericDelaporte mentioned. With nhibernate/nhibernate-core#1765 the broken tests should be fixed.

[maca88]

Unfortunately, I was wrong about stating that I could make ICache.Lock and ICache.Unlock thread safe. With the provided test we can see that a different thread after failing to acquire a lock will unlock the key that the other thread acquired and a third thread will then be able to acquire the lock for the key which should still be locked. Without a change from the NHibernate side by providing us a CacheBase proposed by @fredericDelaporte, I don't see a way of fixing that.

[maca88]

As with NHibernate 5.2 the serialization of cache types will be more user friendly I made a json serializer to see how would impact the overall performance. The serializer reduces the json size by mapping the registered types to shorten names as they have to be included in the json in order to deserialize them correctly. For the following test the Redis cache was located on a different local machine in order to simulate a real environment.

Results with the json serializer:

Get operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 1
Times per iteration 442ms,439ms,443ms,427ms,483ms
Average 446,8ms

GetMany operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 20
Times per iteration 55ms,52ms,54ms,49ms,45ms
Average 51ms

Put operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 1
Times per iteration 470ms,471ms,473ms,451ms,468ms
Average 466,6ms

PutMany operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 20
Times per iteration 54ms,53ms,54ms,54ms,58ms
Average 54,6ms

Results with the binary serializer:

Get operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 1
Times per iteration 480ms,470ms,468ms,486ms,480ms
Average 476,8ms

GetMany operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 20
Times per iteration 98ms,97ms,114ms,86ms,87ms
Average 96,4ms

Put operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 1
Times per iteration 477ms,508ms,462ms,464ms,504ms
Average 483ms

PutMany operation for 1000 keys with region strategy FastRegionStrategy:
Total iterations: 5
Batch size: 20
Times per iteration 69ms,76ms,73ms,68ms,70ms
Average 71,2ms

As we can see for single operations the json serializer is up to 7% faster which is not that much, but for batch operations is up to 90% faster. If you agree I can prepare a different PR for having the mentioned serializer in a separate package NHibernate.Cache.Util.JsonSerializer, as previously suggested.

[fredericDelaporte]

If you agree I can prepare a different PR for having the mentioned serializer in a separate package NHibernate.Cache.Util.JsonSerializer, as previously suggested.

Yes, go for it. I also wonder if we should add some NHibernate.Cache.Common library for by example holding a serializer interface for the JsonSerializer to implement and allow injecting it without hard-dependencies on its current implementation. But better discuss that in another PR, it is off-topic here.

[maca88]

In the current state the cache provider can be configured in several ways, let's suppose that we want to configure the IDatabaseProvider instance:

1. Register it in the NH configuration

<property name="cache.database_provider">MyAssembly.MyDatabaseProvider, MyAssembly</property>

2. Extending the RedisCacheProvider and register it in the NH configuration

Extended class:

public class MyRedisCacheProvider : RedisCacheProvider
{
  public MyRedisCacheProvider()
  {
    CacheConfiguration.DatabaseProvider = new MyDatabaseProvider();
  }
}

NH configuration:

<property name="cache.provider_class">MyAssembly.MyRedisCacheProvider, MyAssembly</property>

3. Register it with a custom IObjectsFactory which uses an IoC container

container.Register<IDatabaseProvider, MyDatabaseProvider>();
Cfg.Environment.ObjectsFactory = new MyObjectsFactory(container);

4. Configure the RedisCacheProvider by code and register it with an IoC container as singleton

Configure RedisCacheProvider by code:

var provider = new RedisCacheProvider();
provider.CacheConfiguration.DatabaseProvider = new MyDatabaseProvider();
container.RegisterSingleton<ICacheProvider>(provider);
Cfg.Environment.ObjectsFactory = new MyObjectsFactory(container);

In order to work correctly we have to alter the NH configuration by setting the interface as the provider class:

<property name="cache.provider_class">NHibernate.Cache.ICacheProvider, NHibernate</property>

For the last example, I think it would be nice if NHibernate would try to get the type by the interface when a custom IObjectsFactory is used, before using the default one.

Update: Added a fifth option

5. Configure RedisCacheProvider default configuration by a static property

RedisCacheProvider.DefaultCacheConfiguration.DatabaseProvider = new MyDatabaseProvider();

[maca88]

Another way in order to avoid having try/catches and a user mapping api for registering implementations/interfaces would be to use the IServiceProvider interface, which is
used for ASP.NET Core. The nice thing about it, is that its GetService method is designed to not throw if the type is not registered and null is returned instead. I've checked some of the most popular IoC frameworks out there and the following have a built-in in support for it:

• ASP.NET Core DependencyInjection
• Autofac
• SimpleInjector
• Ninject
• Windsor

We could obsolete the IObjectsFactory interface in favor of IServiceProvider or we could try to cast the IObjectsFactory in IServiceProvider prior using the default one.

[gokhanabatay]

@fredericDelaporte , @gliljas , @maca88
I need second level cache and cache needs to be distributed for other instances of our Web Api.

Is this pr about second level cache get multiple records with single request? (about 50 - 150 record)
If its then at below scenario I have a problem;
When I call code it retrieves all items from cache but, retrieves them one by one so overall time very slow for our expectation.

In this scenario is Redis right choice?

var result = query.WithOptions(options => { options.SetReadOnly(true); options.SetCacheable(true); }) .AsEnumerable();

[fredericDelaporte]

Currently, caches do not support batching of operations, and do them all one by one.

Depending on the application usage pattern, a distributed second level cache can have worst performances than not using the second level cache. This happens when the cache usage causes a lot of cache operations for just a few SQL queries avoided: when distributed, the IO cost of the many cache operations may overrun the savings bound to not running even complex SQL queries.

In such case, it is better to not use a distributed cache, and this is not specific to Redis. Better use an in-memory cache then, or not use the second level cache at all.

nhibernate/nhibernate-core#1633 add support for batching some operations together, in NHibernate 5.2. For NHibernate-Caches updated to support batching, this will reduces the cases where using a distributed cache is worst than not using one.

This #45 PR does that for a new Redis provider. The already existing one, CoreDistributedCache.Redis, cannot support batching, because it uses an abstraction, Microsoft.Extensions.Caching.Abstractions, which does not support it.

[maca88]

@gokhanabatay

Is this pr about second level cache get multiple records with single request?

Yes it is.

In this scenario is Redis right choice?
var result = query.WithOptions(options => { options.SetReadOnly(true); options.SetCacheable(true); }) .AsEnumerable();

Yes, with NHibernate 5.2 and having the provider from this PR, the records for the cached query will be retrieved in batches.
Here is an example that you can try by placing the following method in the BatchableCacheFixture class:

[Test]
public void CacheReadOnlyQueryTest()
{
  List<ReadOnly> items;
  var persister = Sfi.GetEntityPersister(typeof(ReadOnly).FullName);
  var cache = (BatchableCache)  persister.Cache.Cache;

  cache.ClearStatistics();

  // Put items in the cache
  using (var s = Sfi.OpenSession())
  using (var tx = s.BeginTransaction())
  {
    items = s.Query<ReadOnly>()
           .WithOptions(o => o.SetCacheable(true))
           .ToList();
    tx.Commit();
  }

  Assert.That(items, Has.Count.EqualTo(6));
  Assert.That(cache.PutCalls, Has.Count.EqualTo(0));
  Assert.That(cache.PutMultipleCalls, Has.Count.EqualTo(1)); // Put all items at once

  using (var s = Sfi.OpenSession())
  using (var tx = s.BeginTransaction())
  {
    items = s.Query<ReadOnly>() // Run the same query for the second time
           .WithOptions(o => o.SetCacheable(true))
           .ToList();
    tx.Commit();
  }

  Assert.That(items, Has.Count.EqualTo(6));
  Assert.That(cache.GetCalls, Has.Count.EqualTo(0));
  Assert.That(cache.GetMultipleCalls, Has.Count.EqualTo(2)); // Get items with two calls, because batch size for ReadOnly entity is 3
}

[fredericDelaporte]

Rebased for solving conflicts with the latest merge.

[maca88]

All TODOs are done, now it is ready to be reviewed.

[maca88]

With version 2.0 the .StrongName package does not exist anymore as StackExchange.Redis is now strong-named.

[maca88]

This is obsolete with 2.0, and is false by default (see here for more info).

[fredericDelaporte]

@maca88, do you agree with the removal of RemoveMany?

[maca88]

I agree, we will add it when NHibernate will support it.