ZiggyCreatures / FusionCache

FusionCache is an easy to use, fast and robust hybrid cache with advanced resiliency features.
MIT License
1.71k stars 90 forks source link

Nullable `Size` option #216

Closed JoeShook closed 6 months ago

JoeShook commented 6 months ago

Problem

When moving from release .26 to 1.0 it has come to my attention that within FusionCacheGlobalDefaults the following code changed:

before (v.0.26.0): public static long? EntryOptionsSize { get; set; } = 1;

after (v1.0.0)): public static long? EntryOptionsSize { get; set; } = null;

I could not see any comments on why this happened. It may have happened inadvertently during the merge? I could understand it being the new normal but I am not sure you did this intentionally.

Additional context

I have observed existing code that took for granted that the FusionCacheEntryOptions.Size would always be 1 when a MemoryCache was create with a SizeLimit.

So we now get the following error when we do not set the Size propery: Cache entry must specify a value for Size when SizeLimit is set.

It was just unexpected. I looked through my tests and I always set the size per entry. But again I am seeing issues in the wild where this change is unexpected.

Solution

Of course one solution would be to udate the EntryOptionsSize back to 1.

Alternatives

I am thinking of setting the following code when SizeLimit is != null

FusionCacheGlobalDefaults.EntryOptionsSize = CacheOptions.DefaultOptions?.Size ?? 1;

Do you see any issue with this?
There is a note at the top of FusionCacheGlobalDefaults that says, NOTE:</strong> since these values are used *globally*, they should be changed only as a last resort, and if you *really* know what you are doing. If I understand Size correctly it is just a strategy to give a cache entry a relative weight in terms of size to compared to the SizeLimit set for the MemoryCache. 1 as a default is the simplest and basic strategy ignoring the actual size of the cache entry.

Our internal usage is FusionCache is wrapped in a package so we have a way to ensure this happens transparent to package consumers.

Thanks, Jody.

Looking forward to your comments.

jodydonetti commented 6 months ago

Hi @JoeShook , long time no see!

If you look at the release notes for v1.0 there's this:

image

Hope this helps.

jodydonetti commented 6 months ago

(I changed the issue title to ease future searches)

jodydonetti commented 6 months ago

Also, about this:

I am thinking of setting the following code when SizeLimit is != null FusionCacheGlobalDefaults.EntryOptionsSize = CacheOptions.DefaultOptions?.Size ?? 1; Do you see any issue with this?

No, I don't see any: by changing it there you are basically reverting the change of the default value. A little "strong move", so to speak, but still nothing to worry about if you control the entire thing and if you do it as soon as possible in the app lifecycle, just to be on the safe side.

I'm closing this for now, but will gladyl reopen in case is necessary.