Async support suggestion

[srivathsah]

This is wrt #89 Async support. I suggest the usage of ValueTask in place of Task to support async await so that all synchronous code are covered. And as ValueTask is like a superset of Task asynchronous code is also covered even though only few libraries are supported.

in summary, the methods that can sometimes be asynchronous but mostly synchronous can return ValueTask instead of Task

[MichaCo]

Hi,
primary reason I didn't added async yet is that only a very few use cases would benefit from it and only the Redis client really supports async. Everything else would run sync anyways.
And, most importantly, it will double the APIs on ICacheManager which causes a lot of work, like writing all unit tests etc...
I'll add it to the backlog for now..

[petriashev]

You can introduce new IAsyncCache abstraction with new methods and implement providers that support asyncs.

[AccessViolator]

I'm actually VERY surprised to see that there's no any async support in the library. While I agree that async has little sense for inproc caches, it may be useful for remote. But that's still a "nice to have" category. What I believe is an absolute must have is support for async value factories. Absence of those means I can't use factory versions of methods like GetOrAdd() if I want to retrieve missing value from database asynchronously. I have to first check if value is in the cache, then if it's not, do async db call to get the value, then add it to the cache and by this time another thread may have already added the value.

On this note a question (didn't want to dig sources) - I suppose when long running value factory lambda is in progress all other threads trying to retrieve the value from cache lock waiting for the value. Is that right? Or there's nothing preventing multiple threads from running factories to retrieve the same value and every one of them then updates the value in cache? If threads do lock it may make a noticeable difference in thread pool usage of a busy website with many short lived cache values compared to if the threads were able to instead asynchronously wait for missing value to be retrieved by the first thread.

[MichaCo]

@AccessViolator I totally agree regarding the async factories. I had that planned already and I know it is painful to use right now with sync over asnyc etc... Anyways,, it will get enhanced soon­™ (when I find some time to implement it)

Regarding locking for value factories. No, there is nothing preventing multiple threads running the "expensive" factory. This would be your job to implement mechanisms to prevent thundering herd problems and alike. That's way out of scope of what CacheManager is supposed to do.
In distributed systems, this can be very complicated to achieve.

That being said, you can implement a simple distributed lock with CacheManager by using the Add method and e.g. Redis. Add only adds a key if it doesn't exist, now, if multiple threads or processes try to do the same thing, only one will succeed... You can "abuse" that to implement a distributed lock. Then, it is up to you to handle timeouts and errors in the process which got the lock, so that you never lock something for ever...
That's where it gets more complicated and custom to your use-cases and that's why I'm not going to implement that out of the box in CacheManager

The second part of the question was, can multiple threads update the same key at the same time? Yes and no. The Update method will make sure that you always work with the most recent value. If there was a conflict, the factory will get invoked again with the new value. So, yes, conflicts will be handled.
If you use GetOrAdd, only one thread will add the key, ever. Every other thread might run the factory though, but then fails to add the key and in the next retry will Get the key from cache...

All that has nothing to do with async though

Hope that all makes sense ;)

[AccessViolator]

Thanks for clarification.

For now I think I'll use extension methods similar to suggested in another thread, e.g.

        public static async Task<T> GetOrAddSlidingAsync<T>(
            this ICacheManager<T> self,
            string key,
            short minutes,
            Func<string, Task<T>> factory)
        {
            T value;
            if ((value = self.Get<T>(key)) == null)
            {
                value = await factory(key);
                self.Add(new CacheItem<T>(key, value, ExpirationMode.Sliding, TimeSpan.FromMinutes(minutes)));
            }
            return value;
        }

[MichaCo]

Yup that looks fine. Just a small thing, you don't have to use Get<T> if self is already of type T, just use Get which will be faster because it doesn't try to cast it.
Then, self.Add can, in theory, return false, which would mean that between your .Get(..) == null check and Add in the meantime, another process or thread already added the same key.
This can totally happen, especially if the factory takes a while to process...
If Add returns false, you'd have to retry

[AccessViolator]

Thanks.

Re retrying - I think in most cases for factories like db retrieval it's pretty safe to assume that if another thread added the key between null check and Add using the same factory the value would be the same anyway :)

[silashansen]

Any progress on the async valueFactory support? 😄

[andreymir]

@MichaCo we need async methods to use the cache in .net core application.
We use redis cache handler and under load 20 rps the application hangs on LoadScripts(). We switched back to .net 4.6.2 as a temporary fix.

[MichaCo]

@andreymir
Do you have a way to reproduce this?

I don't thing that has anything todo with .NET Core. The problem might be that load script blocks or tries to load multiple times. So that's probably another issue we can improve to basically use a distributed lock to determine if someone is already loading the scripts.
But I'm not sure. Could be other stuff too.

[andreymir]

@MichaCo , I created a sample https://github.com/andreymir/load-demo
To run the application as net462 execute dotnet run -c Release -f net462
To run the application as netcoreapp execute dotnet run -c Release -f netcoreapp2.1
The application starts on http://localhost:5000

To create the load you need nodejs and npm. Run npm install and then npm run load. It will run the load of 3 rps on http://localhost:5000 for 5 minutes (see load.yml).

To reproduce the issue try to restart the application during the load. Under net462 configuration application restarts fine. Under netcoreapp2.1 the application hangs and does not respond.

You also need redis running on localhost:6379 for the cache.

[andreymir]

@MichaCo, did you be able to reproduce the issue with the demo I created?

I want to try to add async methods to ICache interface. Do you have any suggestions of how to approach this? Could it be better to create a new interface like IAsyncCache?