Absolute expiration not working with TimeSpan

[mtschneiders]

I'm not sure if this should be a supported scenario, but when setting Absolute Expiration with a TimeSpan, the cache entry does not expire.

For example, the following test fails:

IAppCache cache = new CachingService(new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions())));

cache.GetOrAdd<int?>("KEY", entry =>
{
	entry.SetAbsoluteExpiration(TimeSpan.FromSeconds(10));
	return 1;
});

Thread.Sleep(TimeSpan.FromSeconds(11));
Assert.Null(cache.Get<int?>("KEY")); //Fails

However, when we set the expiration with a DateTimeOffset, it works:

IAppCache cache = new CachingService(new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions())));

cache.GetOrAdd<int?>(KEY, entry =>
{
	entry.SetAbsoluteExpiration(DateTimeOffset.UtcNow.AddSeconds(10));
	return 1;
});

Thread.Sleep(TimeSpan.FromSeconds(11));

Assert.Null(cache.Get<int?>(KEY)); //Works

Meanwhile, when using .NET Core MemoryCache, setting the expiration with a TimeSpan works normally:

IMemoryCache cache = new MemoryCache(new MemoryCacheOptions());

cache.GetOrCreate<int?>("KEY", entry =>
{
	entry.SetAbsoluteExpiration(TimeSpan.FromSeconds(10));
	return 1;
});

Thread.Sleep(TimeSpan.FromSeconds(11));

Assert.Null(cache.Get<int?>("KEY")); //Works

The question is, should it be possible to set the Absolute Expiration with a TimeSpan or is it out of the library scope?

[Sinhk]

I figured out why this does not work, but I'm not sure it's easily fixable.

MemoryCache calculates AbsoluteExpiration from AbsoluteExpirationRelativeToNow in the SetEntry method. With LazyCache the SetEntry method is executed before the factory, and therefore before the entry options are set, because those are set in the factory. So when AbsoluteExpirationRelativeToNow is set by the factory, MemoryCache has already decided there is no absolute expiration.
This only applies to the GetOrAdd methods.

[mtschneiders]

What if after the factory method we verify if the AbsoluteExpirationRelativeToNow is set and if so, call SetAbsoluteExpiration with a DateTime offset. This would make it set the underlying AbsoluteExpiration property.

Example:

public virtual T GetOrAdd<T>(string key, Func<ICacheEntry, T> addItemFactory)
{
    ValidateKey(key);

    object cacheItem;
    locker.Wait(); //TODO: do we really need this? Could we just lock on the key?
    try
    {
        cacheItem = CacheProvider.GetOrCreate<object>(key, entry =>
            new Lazy<T>(() =>
            {
                var result = addItemFactory(entry);

                //Add this
                if (entry.AbsoluteExpirationRelativeToNow.HasValue)
                    entry.SetAbsoluteExpiration(DateTimeOffset.UtcNow.Add(entry.AbsoluteExpirationRelativeToNow.Value));

                EnsureEvictionCallbackDoesNotReturnTheAsyncOrLazy<T>(entry.PostEvictionCallbacks);
                return result;
            })
        );
    }
    finally
    {
        locker.Release();
    }

    try
    {
        return GetValueFromLazy<T>(cacheItem);
    }
    catch //addItemFactory errored so do not cache the exception
    {
        CacheProvider.Remove(key);
        throw;
    }
}

[johnrom]

oh, this exists already. created proof of concept while figuring out why this wasn't working:

https://github.com/johnrom/lazycache-relative-absolute-expiration

[yangirov]

Absolutely confirm this problem.

It dont' works:
await cache.GetOrAddAsync("List", () => GetFromDb(), TimeSpan.FromSeconds(cacheTime))

It works:
await cache.GetOrAddAsync("List", () => GetFromDb(), DateTimeOffset.UtcNow.AddSeconds(cacheTime))

[AlmostRemember]

Are there any plans to fix this issue? In my scenario I'm caching the result of an expensive query. The value should be available for a fixed period of time. I can't predict how long the query will take to execute. Currently the only option is to use absolute expiration and pad the time with an approximate guess. Not ideal.

[johnrom]

@AlmostRemember you should be able to set the absolute expiration after you run your query. Like:

            var absolute = _cache.GetOrAdd(CACHED_ITEM_ABSOLUTE_KEY, (entry) => {
                var cachedItem = await GetExpensiveResult();

                // UtcNow is now the time _after_ the query.
                entry.AbsoluteExpiration = DateTime.UtcNow + TimeSpan.FromMinutes(20);

                return cachedItem;
            });

[AlmostRemember]

@johnrom thanks for your suggestion. That does produce the behavior I want. I overlooked that you have access to ICacheEntry in your factory and you can just reset it. Works for me.

[johnrom]

Glad I could help!

[alastairtree]

Because of the way LazyCache works with a Lazy to handle some of the locking changing the properties on the cache entry object inside the delegate/callback means they may be executed at the wrong time and so do not behave as expected. This is partly because I ported to dotnet core without a big API rewrite, so did not fully adopt the new MemoryCache in core in all its flexibility in favour of maintaining the API from the full framework as closely as possible.
Also LazyCache generally uses TimeSpan for sliding expiration and DateTimeOffset for absolute expiration.

So...

If you want to do absolute expiration you are best to do it by just passing in the expiration date using an offset to GetOrAdd :

cache.GetOrAdd<int?>("KEY", entry =>
{
	return 1;
}, DateTimeOffset.Now.AddSeconds(30));

or alternatively preparing the options in advance:

var memoryCacheEntryOptions = new MemoryCacheEntryOptions
{
  AbsoluteExpiration = DateTimeOffset.Now.AddSeconds(30)
};
cache.GetOrAdd<int?>("KEY", entry =>
{
	return 1;
}, memoryCacheEntryOptions );

And If you want to do sliding expiration then you should use TimeSpan:

cache.GetOrAdd<int?>("KEY", entry =>
{
	return 1;
}, TimeSpan.FromSeconds(30));

Maybe in future we will support the more sophisticated methods on the MemoryCacheEntryOptions in the callback

[bjornicus]

It's probably worth adding a note in the docs about things that don't work which people who were using IMemoryCache will expect to work. I was trying to figure out why my cached items were not expiring like I expected, and cooked up the following test, then discovered this open issue when came to open a new one. Had this been mentioned in the docs I would have discovered it sooner.

Here's my test

        [Test]
        public async Task GetOrAddAsyncOnCore31ExpiresTheItem()
        {
            var cachedResult = await sut.GetOrAddAsync(TestKey, async entry =>
            {
                // this does not work
                // entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(1);

                // but this does
                entry.SetAbsoluteExpiration(DateTime.Now.AddSeconds(1));
                return "SomeValue";
            });

            Assert.IsNotNull(cachedResult);
            Assert.AreEqual("SomeValue", cachedResult);

            await Task.Delay(1500);

            cachedResult = sut.Get<string>(TestKey);
            Assert.IsNull(cachedResult);
        }