Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Cannot set SlidingExpiration and AbsoluteExpirationRelativeToNow on RedisCache #194

Closed
camp-007 opened this issue May 23, 2016 · 5 comments
Assignees
Milestone

Comments

@camp-007
Copy link

camp-007 commented May 23, 2016

When using the RedisCache you can set both a SlidingExpiration and an AbsoluteExpiration. For example, you can set a sliding expiration of 5 seconds and an absolute expiration of 60 seconds. If the cached item is accessed every 5 seconds it will remain in cache up until 60 seconds from the original creation.

However, this only works if you are setting the DistributedCacheEntryOptions.AbsoluteExpiration property. If you set the DistrubutedCacheEntryOption.AbsoluteExpirationRelativeToNow then only the sliding expiration is respected/used.

The meta-data stored in Redis for each cache entry includes only the SlidingExpiration and the AbsoluteExpiration values, but ignores the AbsoluteExpirationRelativeToNow property.

The AbsoluteExpirationRelativeToNow property is very convenient (as the caller often doesn't want to have to worry about calculating the actual expiration time, just specifying how far into the future it should be). However, it seems that this property should really cause all the same behaviors as the AbsoluteEpiration property.

Thanks for listening!

@Tratcher
Copy link
Member

Tratcher commented May 23, 2016

Hmm, it should be checked right here:

if (options.AbsoluteExpirationRelativeToNow.HasValue)

@camp-007
Copy link
Author

Correct, it is checked there in GetExpirationInSeconds(), however, that is only used to initially set the TTL/expiry on the redis key. The way the sliding expiration works, is that each "Get" will use the GetAndRefresh() method to update the key TTL/expiry. This is done by getting the SlidingExpiration and AbsoluteExpiration out of the metadata and passing them into the refresh method (shown here):

MapMetadata(results, out absExpr, out sldExpr);

But the problem, I think, is that when the meta data is stored, it only accounts for the AbsoluteExpiration property (and it ignores the AbsoluteExpirationRelativeToNow property). This can be seen here:

options.AbsoluteExpiration?.Ticks ?? NotPresent,

When the metadata is written with the key, it should calculate an absolute expiration time based on EITHER the AbsoluteExpiration OR the AbsoluteExpirationRelativeToNow property.

@camp-007
Copy link
Author

Here is a simple repro:

using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Caching.Redis;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CacheBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var cache = new RedisCache(new RedisCacheOptions() { Configuration = "localhost" });

            var cacheOptions = new DistributedCacheEntryOptions();
            cacheOptions.SlidingExpiration = TimeSpan.FromSeconds(5);
            cacheOptions.AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(10);  //Will never actually expire

            //Uncomment to see correct/desired behavior
            //cacheOptions.AbsoluteExpiration = DateTimeOffset.UtcNow.AddSeconds(10);  //Will correctly expire after 10 seconds

            var message = "Hello World!";
            var data = Encoding.UTF8.GetBytes(message);
            cache.Set("expirationbug", data, cacheOptions);

            while (!string.IsNullOrWhiteSpace(message))
            {
                data = cache.Get("expirationbug");
                if(data == null)
                {
                    message = null;
                    Console.WriteLine("Data has expired. Program will end.");
                }
                else
                {
                    message = Encoding.UTF8.GetString(data);
                    Console.WriteLine($"{DateTime.UtcNow.ToString()}: {message}");
                    Task.Delay(TimeSpan.FromSeconds(1)).Wait();
                }

                //Program never ends if using AbsoluteExpirationRelativeToNow
            }

        }
    }
}

@Tratcher Tratcher added the bug label May 24, 2016
@Tratcher
Copy link
Member

Ah, you are correct. We'll get this fixed.

@BrennanConroy
Copy link
Member

Fixed in 68829e2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants