[Performance] RedisRemoteEvictionExtension evicts just added key from all running instances

[jerackista]

What does the bug do?

Even though the key was just added to cache the eviction is called on all currently running instances. Which makes startup performance really bad when a lot of new keys are added to cache.
So when calling GetOrSetAsync even though key is not present in any layer of cache it still get's evicted from memory layer from all other instances which makes a lot of overhead when adding new keys ( especially startup).

How can it be reproduced?

You can reproduce the bug by running this code...

var redisCacheLayer = new RedisCacheLayer(redisConnection);
            var memoryCacheLayer = new MemoryCacheLayer();
            var cacheStack = new CacheStack(new ICacheLayer[]
            {
                memoryCacheLayer,
                redisCacheLayer

            }, new ICacheExtension[]
            {
                new RedisRemoteEvictionExtension(redisConnection, new ICacheLayer[] {memoryCacheLayer})
            });

cacheStack.GetOrSetAsync(); //even though it's first call for specific key it calls evict on all other instances. 

[Turnerj]

Thanks for raising this issue - that is pretty strange, will dig into the code in the next day or two to get it sorted. It definitely shouldn't be doing what you've described.

[jerackista]

Here is the call stack which happens.

GetOrSetAsync
RefreshValueAsync
WithRefreshAsync
SetAsync
Extensions.OnCacheUpdateAsync
RedisRemoteEvictionExtension.OnCacheUpdateAsync
FlagEvictionAsync(cacheKey);

Let me know if I can help you with anything. I could try to write unit test and fix the thing but not sure where is the most correct place to break the eviction chain.

[Turnerj]

The call stack is super helpful. I get why it is doing that and it even makes sense but yeah, logically it needs to consider not evicting fresh items in a cache. It should only play out like that for older existing items.

The solution might be checking the expiry on the receiving eviction extension that it is within X seconds of being set itself.

[jerackista]

How about changing interface of ICacheChangeExtension.OnCacheUpdateAsync to something like ValueTask OnCacheUpdateAsync(string cacheKey, DateTime expiry, bool newEntry) then eviction could be easily skipped and maybe the new parameter will be helpful also for other extensions?

[Turnerj]

Yeah, something like that could work. Because OnCacheUpdateAsync is called from SetAsync, the method itself won't know if it exists or not so I need to be cautious how I pass the state through.

I'm thinking there are two states:

• Override
• New

Basically all calls to SetAsync are the former - without knowing the state, we only know the intention is to override the value. When SetAsync is called internally from the refreshing logic, we do know then if the value is new or not - it is then we set the "New" flag.

I've recreated your issue as a test case in #178 and plan to do the change tomorrow, hopefully with a new release in the next day or two.

[Turnerj]

Version 0.11.0 of Cache Tower is now released with the fix from #178 included - that should solve your problem. If you're still encountering issues, let me know!

[jerackista]

I confirm that issue is fixed in version 0.11.0. Thank you.