Caching asynchronous operations

Question

I am looking for an elegant way of caching the results of my asynchronous operations.

I first had a synchronous method like this:

public String GetStuff(String url)
{
    WebRequest request = WebRequest.Create(url);
    using (var response = request.GetResponse())
    using (var sr = new StreamReader(response.GetResponseStream()))
        return sr.ReadToEnd();
}

Then I made it asynchronous:

public async Task<String> GetStuffAsync(String url)
{
    WebRequest request = WebRequest.Create(url);
    using (var response = await request.GetResponseAsync())
    using (var sr = new StreamReader(response.GetResponseStream()))
        return await sr.ReadToEndAsync();
}

Then I decided that I should cache the results, so I do not need to query outside that often:

ConcurrentDictionary<String, String> _cache = new ConcurrentDictionary<String, String>();

public async Task<String> GetStuffAsync(String url)
{
    return _cache.GetOrAdd(url, await GetStuffInternalAsync(url));
}

private async Task<String> GetStuffInternalAsync(String url)
{
    WebRequest request = WebRequest.Create(url);
    using (var response = await request.GetResponseAsync())
    using (var sr = new StreamReader(response.GetResponseStream()))
        return await sr.ReadToEndAsync();
}

Then I read an article (o watched a video) about how caching Task<T> is better, because creating them is expensive:

ConcurrentDictionary<String, Task<String>> _cache = new ConcurrentDictionary<String, Task<String>>();

public Task<String> GetStuffAsync(String url)
{
    return _cache.GetOrAdd(url, GetStuffInternalAsync(url));
}

private async Task<String> GetStuffInternalAsync(String url)
{
    WebRequest request = WebRequest.Create(url);
    using (var response = await request.GetResponseAsync())
    using (var sr = new StreamReader(response.GetResponseStream()))
        return await sr.ReadToEndAsync();
}

And now the problem is, that if the request fails (e.g.: a HTTP 401), the cache will contain a failed Task<String> and I will have to reset the app because it will be impossible to resend the request.

Is there a elegant way of using ConcurrentDictionary<T1,T2> to cache only successful tasks and still have the atomic behavior?

Answer 1

First of all, both your approaches are wrong, because they don't save you any requests (though the second one at least saves you time).

Your first code (the one with await) does this:

1. Make the request.
2. Wait for the request to complete.
3. If there already was a result in the cache, ignore the result of the request.

Your second code removes step 2, so it's faster, but you're still making lots of unnecessary requests.

What you should do instead is to use the overload of GetOrAdd() that takes a delegate:

public Task<String> GetStuffAsync(String url)
{
    return _cache.GetOrAdd(url, GetStuffInternalAsync);
}

This doesn't completely eliminate the possibility of requests that are ignored, but it does make them much less likely. (For that, you could try canceling requests that you know are being ignored, but I don't think that's worth the effort here.)

---

Now to your actual question. What I think you should do is to use the AddOrUpdate() method. If the value isn't there yet, add it. If it's there, replace it if it's faulted:

public Task<String> GetStuffAsync(String url)
{
    return _cache.AddOrUpdate(
        url, GetStuffInternalAsync, (u, task) =>
        {
            if (task.IsCanceled || task.IsFaulted)
                return GetStuffInternalAsync(u);
            return task;
        });
}

Answer 2

It's actually reasonable (and depending on your design and performance, crucial) to keep those failed tasks as a Negative Cache. Otherwise, if a url always fails, using it again and again defeats the point of using a cache altogether.

What you do need is a way to clear the cache from time to time. The simplest way is to have a timer that replaces the ConcurrentDictionarry instance. The more robust solution is to build your own LruDictionary or something similar.