[BUG] IsFailSafeEnabled required to be set true in DefaultEntryOptions

[waynebrantley]

@jodydonetti Really amazing work. So many features and all pretty simple to use.

It appears that if the DefaultEntryOptions do not turn on Fail Safe, it cannot be turned on.
This fails for the 'false' case and succeeds for the 'true' case.

Is this expected behavior

    [TestCase(true)]
    [TestCase(false)]
    public void LocalFailSafeEnabled(bool setDefaultFailSafe)
    {
        var services = new ServiceCollection();
        services.AddMemoryCache();
        var cacheBuilder = services.AddFusionCache();
        cacheBuilder.WithDefaultEntryOptions(new FusionCacheEntryOptions
        {
            Duration = TimeSpan.FromHours(2), 
            IsFailSafeEnabled = setDefaultFailSafe,
            FailSafeMaxDuration = TimeSpan.FromDays(1), 
            FailSafeThrottleDuration = TimeSpan.FromSeconds(30),
            FactorySoftTimeout = TimeSpan.FromMilliseconds(100),
            FactoryHardTimeout = TimeSpan.FromMinutes(2),
        });

        var serviceProvider = services.BuildServiceProvider();
        var fusionCache = serviceProvider.GetRequiredService<IFusionCache>();
        
        var initialDateTime = fusionCache.GetOrSet("TEST",  _ =>
        {
            Thread.Sleep(TimeSpan.FromSeconds(3));
            return DateTime.Now;
        }, options => options.SetDuration(TimeSpan.FromSeconds(30)).SetFailSafe(true, TimeSpan.FromHours(2), TimeSpan.FromSeconds(30)).SetFactoryTimeouts(TimeSpan.FromMilliseconds(100)));

        var cachedDateTime = fusionCache.GetOrSet("TEST",  _ =>
        {
            Thread.Sleep(TimeSpan.FromSeconds(3));
            return DateTime.Now;
        }, options => options.SetDuration(TimeSpan.FromSeconds(30)).SetFailSafe(true, TimeSpan.FromHours(2), TimeSpan.FromSeconds(30)).SetFactoryTimeouts(TimeSpan.FromMilliseconds(100)));

        Assert.That(initialDateTime, Is.EqualTo(cachedDateTime));
     
        //now expire the cache
        fusionCache.Expire("TEST");
        var shouldUseExpiredValue = fusionCache.GetOrSet("TEST",  _ =>
        {
            Thread.Sleep(TimeSpan.FromSeconds(3));
            return DateTime.Now;
        }, options => options.SetDuration(TimeSpan.FromSeconds(30)).SetFailSafe(true, TimeSpan.FromHours(2), TimeSpan.FromSeconds(30)).SetFactoryTimeouts(TimeSpan.FromMilliseconds(100)));
     
        Assert.That(shouldUseExpiredValue, Is.EqualTo(initialDateTime));
        
        Thread.Sleep(TimeSpan.FromSeconds(3)); //should be enough time for it to have finished

        var shouldBeNewValue = fusionCache.GetOrSet("TEST",  _ =>
        {
            Thread.Sleep(TimeSpan.FromSeconds(3));
            return DateTime.Now;
        }, options => options.SetDuration(TimeSpan.FromSeconds(30)).SetFailSafe(true, TimeSpan.FromHours(2), TimeSpan.FromSeconds(30)).SetFactoryTimeouts(TimeSpan.FromMilliseconds(100)));

        Assert.That(shouldBeNewValue, Is.Not.EqualTo(initialDateTime));
    }

[jodydonetti]

Hi @waynebrantley , I'm checking this, will report back soon.

[waynebrantley]

@jodydonetti any luck with this or anyway I can help?

[jodydonetti]

Hi @waynebrantley , sorry for the delay!

The upcoming v2 absorbed me completely and I somehow forgot to answer you.

Short Version

Yes, I understood the cause of the problem.
No, it was not a bug.
But yes, I've made some changes to better handle expectations in cases like this.

The new v2 coming out soon will work as expected.

Longer Version

In the code above you did everything right, you set the correct defaults and everything. But then you called Expire().

Now, in FusionCache we have both Remove() and Expire(), and for a very good reason: to support fail-safe scenarios.
The idea seems to be that Remove() will in fact remove the entry, while Expire() mark it as expired, allowing you to still use it as a fallback in case of problems, right?

Well this is true, except that - up until now - when calling Expire() FusionCache also checked the IsFailSafeEnabled entry option passed to it, to know if it should try to support fail-safe or not:

• if set to true, the data will be expired (but kept around)
• if set to false, it will be effectively removed

Now: as always, if no entry options are passed, the DefaultEntryOptions will be used, so in your case when setting the default to false, even by passing true to each GetOrSet method calls, the Expire() call without entry options used the DefaultEntryOptions, for which IsFailSafeEnabled was set to false, and therefore actually removing the cache entry.

I did it this way back when I added the Expire() method to allow maximum flexibility, but... well, the name of the method itself is already a clear indication of what the user want to do, right?
I mean, if they call Remove() they want to effectively remove the cache entry, and if they call Expire() well, they would like to expire the cache entry, meaning keeping it around for fail-safe uses later on.

So, what did I do?

Starting with v2, the behaviour will change, and it will be simpler:

• calling Remove() will effectively remove the cache entry
• calling Expire() will mark the cache entry as expired, ready to be used for fail-safe later on

Of course the rest is still valid: the cache entry must be set with fail-safe enabled from the start to allow it to stick around for more, and there will still be total control over the "get" part so that, even if a cache entry has been saved with fail-safe enabled, you'll still be able to ignore it when getting it for maximum granularity.

Makes sense?

Btw thanks again for spotting this, I think it helped a lot in rationalizing a better, more sensible, less surprising behaviour for FusionCache.

ps: I tried your MRE with the changes mentioned above applied, and it worked perfectly well.

[jodydonetti]

Hi all, v2.0.0-preview-3 is out 🥳

[waynebrantley]

@jodydonetti Thanks so much for the detailed response. It does indeed seem that v2 will be much better. To be clear the scenario is in general (default options) I wanted fail-safe to be off, but then for certain scenarios I wanted it to be on.

The main reason I need it off is the factory that creates the cache entity is using an injected object. This injected object is scoped to the http request (web app), so by the time you call the factory function the item is already disposed. I know there are several issues/topics around this issue and ways to 'work around' but essentially that is a major blocker to being able to use that.

We can discuss solutions in another topic, but what comes to mind is:

• have fusionCache support a factory function that takes the container as a parameter.
• have fusionCache support DI on factory functions much like asp.net does with MapGet, MapPost, etc.

[jodydonetti]

Hi @waynebrantley

@jodydonetti Thanks so much for the detailed response. It does indeed seem that v2 will be much better. To be clear the scenario is in general (default options) I wanted fail-safe to be off, but then for certain scenarios I wanted it to be on.

Understood, and something pretty standard with FusionCache: you can turn it on in the DefaultEntryOptions, and turn it off for certain calls.

The main reason I need it off is the factory that creates the cache entity is using an injected object. This injected object is scoped to the http request (web app), so by the time you call the factory function the item is already disposed.

Very similar to when using EF with the normal scoped DbContext or similar, see here for more.

In general the solution is to not ask for a scoped object but ask for an IServiceScopeFactory and, inside the factory when calling GetOrSet(), do serviceScopeFactory.CreateScope() so that the scope will be limited to the factory execution lifetime.

Hope this helps.

[waynebrantley]

@jodydonetti

Understood, and something pretty standard with FusionCache: you can turn it on in the DefaultEntryOptions, and turn it off for certain calls.

Just to be super clear - was looking for the opposite of this. Turn it off in DefaultEntryOptions and turn it on for certain calls.

Very similar to when using EF with the normal scoped DbContext

Understood and I can make that work, but it is not great. In our situation we use CQRS and have a decorator that does caching. The query (CQRS) is written unaware of it being cached or not (decorators for the win). With the above solution, anything decorated would have to for sure know that so it can manage the injected values differently - not injecting them - but injecting the ScopeFactory. If the factory for the cache resolved params (like MapGet) or passed in the IServiceScopeFactory then it would be a bit cleaner. This is probably for a longer conversation though.

[jodydonetti]

Just to be super clear - was looking for the opposite of this. Turn it off in DefaultEntryOptions and turn it on for certain calls.

Got it, but to be clear from my side: it's the same 👍

This is probably for a longer conversation though.

Agree, not an easy answer, I would need to understand more about the scenario, but I think something can be done so that it works, and I mean something good not some hacky trick. Maybe by looking at it from a different perspective.

[jodydonetti]

Hi all, I still can't believe it but v2.0.0 is finally out 🎉
Now I can rest.