Closed
Description
Hi,
Please give us a way to implement our custom UpdateTimestampsCache or consider early release of #2129 and #2115. There is a real performance issue.
I made an workaround...
I know that its not a good idea working with readonly fields.
Also I really do not understand why we lock "[MethodImpl(MethodImplOptions.Synchronized)]" retrieve operations with "IsUpdate" and "AreUpToDate" methods.
Below implementation has no lock and no concurrency issue, every second level select operation checks "UpdateTimestamp" from redis at the same time.
I think we need a lock just for "writing cache provider" and somehow key based could be awesome.
Java Implementation no lock
public static class SessionFactoryExtension
{
public static void FixUpdateTimestampsCacheLockIssue(this ISessionFactory sessionFactory)
{
var settings = typeof(SessionFactoryImpl).GetField("settings", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(sessionFactory) as Settings;
var properties = typeof(SessionFactoryImpl).GetField("properties", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(sessionFactory) as IDictionary<string, string>;
var entityPersisters = typeof(SessionFactoryImpl).GetField("entityPersisters", BindingFlags.Instance | BindingFlags.NonPublic)
.GetValue(sessionFactory) as IDictionary<string, IEntityPersister>;
var cacheableSpaces = entityPersisters.Where(x => x.Value.HasCache).SelectMany(x => x.Value.PropertySpaces).ToHashSet();
var updateTimestampsCache = new CustomUpdateTimestampsCache(settings, properties, cacheableSpaces);
sessionFactory.GetType().GetField("updateTimestampsCache", BindingFlags.Instance|BindingFlags.NonPublic)
.SetValue(sessionFactory, updateTimestampsCache);
var queryCache = ((SessionFactoryImpl)sessionFactory).QueryCache;
queryCache.GetType().GetField("_updateTimestampsCache", BindingFlags.Instance | BindingFlags.NonPublic)
.SetValue(queryCache, updateTimestampsCache);
}
}
public class CustomUpdateTimestampsCache : NHibernate.Cache.UpdateTimestampsCache
{
protected readonly ISet<string> CacheableSpaces;
protected readonly NHibernate.Cache.CacheBase UpdateTimestamps;
public CustomUpdateTimestampsCache(Settings settings, IDictionary<string, string> props, ISet<string> cacheableSpaces) : base(settings, props)
{
CacheableSpaces = cacheableSpaces;
UpdateTimestamps = typeof(NHibernate.Cache.UpdateTimestampsCache).GetField("_updateTimestamps", BindingFlags.Instance|BindingFlags.NonPublic)
.GetValue(this as NHibernate.Cache.UpdateTimestampsCache) as NHibernate.Cache.CacheBase;
}
#region Invalidate
public override void Invalidate(IReadOnlyCollection<string> spaces)
{
spaces = spaces.Where(x => CacheableSpaces.Contains(x)).ToList();
if (spaces.Count > 0)
{
base.Invalidate(spaces);
}
}
public override void PreInvalidate(IReadOnlyCollection<string> spaces)
{
spaces = spaces.Where(x => CacheableSpaces.Contains(x)).ToList();
if (spaces.Count > 0)
{
base.PreInvalidate(spaces);
}
}
public override Task InvalidateAsync(IReadOnlyCollection<string> spaces, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return Task.FromCanceled<object>(cancellationToken);
}
spaces = spaces.Where(x => CacheableSpaces.Contains(x)).ToList();
if (spaces.Count == 0)
{
return Task.CompletedTask;
}
return base.InvalidateAsync(spaces, cancellationToken);
}
public override Task PreInvalidateAsync(IReadOnlyCollection<string> spaces, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return Task.FromCanceled<object>(cancellationToken);
}
spaces = spaces.Where(x => CacheableSpaces.Contains(x)).ToList();
if (spaces.Count == 0)
{
return Task.CompletedTask;
}
return base.PreInvalidateAsync(spaces, cancellationToken);
}
#endregion
#region IsUpToDate
public override bool IsUpToDate(ISet<string> spaces, long timestamp)
{
if (spaces.Count == 0)
return true;
else
{
var lastUpdates = UpdateTimestamps.GetMany(spaces.ToArray<object>());
return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp));
}
}
public override bool[] AreUpToDate(ISet<string>[] spaces, long[] timestamps)
{
if (spaces.Sum(x => x.Count) == 0)
return Array.Empty<bool>();
else
{
var allSpaces = new HashSet<string>();
foreach (var sp in spaces)
{
allSpaces.UnionWith(sp);
}
if (allSpaces.Count == 0)
return ArrayHelper.Fill(true, spaces.Length);
var keys = allSpaces.ToArray<object>();
var index = 0;
var lastUpdatesBySpace =
UpdateTimestamps
.GetMany(keys)
.ToDictionary(u => keys[index++], u => u as long?);
var results = new bool[spaces.Length];
for (var i = 0; i < spaces.Length; i++)
{
var timestamp = timestamps[i];
results[i] = spaces[i].All(space => !IsOutdated(lastUpdatesBySpace[space], timestamp));
}
return results;
}
}
public override async Task<bool> IsUpToDateAsync(ISet<string> spaces, long timestamp, CancellationToken cancellationToken)
{
if (spaces.Count == 0)
return true;
else
return await base.IsUpToDateAsync(spaces, timestamp, cancellationToken);
}
public override async Task<bool[]> AreUpToDateAsync(ISet<string>[] spaces, long[] timestamps, CancellationToken cancellationToken)
{
if (spaces?.Sum(x => x.Count) == 0)
return Array.Empty<bool>();
else
return await base.AreUpToDateAsync(spaces, timestamps, cancellationToken);
}
#endregion
private static bool IsOutdated(long? lastUpdate, long timestamp)
{
if (!lastUpdate.HasValue)
{
//the last update timestamp was lost from the cache
//(or there were no updates since startup!)
//NOTE: commented out, since users found the "safe" behavior
// counter-intuitive when testing, and we couldn't deal
// with all the forum posts :-(
//updateTimestamps.put( space, new Long( updateTimestamps.nextTimestamp() ) )
//result = false; // safer
//OR: put a timestamp there, to avoid subsequent expensive
// lookups to a distributed cache - this is no good, since
// it is non-threadsafe (could hammer effect of an actual
// invalidation), and because this is not the way our
// preferred distributed caches work (they work by
// replication)
//updateTimestamps.put( space, new Long(Long.MIN_VALUE) )
}
else
{
if (lastUpdate >= timestamp)
{
return true;
}
}
return false;
}
}