In Real World Monitoring and Tuning ASP.NET Caching Part 1, I showed the behavior of a certain ASP.NET application under load.  We resolved the issue today, which turned out to be a result of two related issues that, thankfully in our case, were limited to a single class in our application that was responsible for interacting with our cache.  Here’s the old behavior:

The new version of the same application, under pretty much the same load, looks like this:

Note the lack of ever-growing memory followed by high-CPU utilization as the memory is reclaimed.  In my last post, I suggested that there were several approaches we could take to correct the issue, one of them being to throw hardware at it.  We tried that, in fact, and added an 8-core 8GB server to the NLB for the application.  Unfortunately, it had the same characteristic climbing RAM and huge drop as the lightweight server shown here, with the added *bonus* that when it was reclaiming its RAM, it was unresponsive for several times longer (over a minute).  Not acceptable.

Using a variety of memory profiler tools, including the ones built into VS2010, we determined that a very large number of CacheDependency objects were being created by the application, so we started looking there.  A potentially related issue we’d noticed (I’d seen for years, actually) with the application was that sometimes, on a cold start under load, it would take a long time to start responding, and it would log errors stating that it was unable to talk to the SqlCacheDependency database (these would be timeout errors).  The stack trace looked like this:

System.Web.HttpException (0x80004005): Unable to connect to SQL database 'MyDatabase' for cache dependency polling. at System.Web.Caching.SqlCacheDependencyManager.EnsureTableIsRegisteredAndPolled(String database, String table) at System.Web.Caching.SqlCacheDependency.GetDependKey(String database, String tableName)

If you’re seeing these kinds of errors, but only on app startup, then you probably have a similar issue to ours.  Keep reading.

The Cache Access Pattern

I’ve written before about the ideal cache access pattern.  Some things have changed with .NET over the years, making it much easier to use delegates via lambdas, so the pattern has evolved a bit.  Here’s what the pattern we were using before today looked like, encapsulated into a single class implementing an ICacher interface.

// Problem Code - Don't Use
private TObjectToCache GetCachedObject<TObjectToCache>(string cacheKey, 
    Func<TObjectToCache> getObjectToCache, 
    CacheDependency cacheDependency, 
    DateTime absoluteExpiration)
    where TObjectToCache : class
{
    if (string.IsNullOrEmpty(cacheKey))
    {
        throw new ArgumentNullException("cacheKey", 
        "Given cache key cannot be null or empty.");
    }
    if (getObjectToCache == null)
    {
        throw new ArgumentNullException("getObjectToCache");
    }
    var cachedObject = _cache[cacheKey] as TObjectToCache;
    if (cachedObject == null)
    {
        cachedObject = getObjectToCache();
        InsertIntoCache(cacheKey, cachedObject, 
            cacheDependency, absoluteExpiration);
    }
    return cachedObject;
}

So, this is pretty decent code.  It would run almost any ASP.NET site just fine.  But it has a very real problem, which presents under load.  Can you see it?  Scroll down for the solution.

.

.

.

.

.

.

.

Hint: Here’s an example of how this would be called:

public override IEnumerable<Publisher> GetTop20RankedPublishers()
{
    const string cacheKey = "GetTop20RankedPublishers()";

    var cacheDependencies = new[]
    {
        DependencyFactory.CreateSqlCacheDependency(PublisherTableDependency),
    };
    var cacheDependency = 
        DependencyFactory.GetAggregateDependency(cacheDependencies);
    return _cacher.Cache(cacheKey, 
        () => BaseGetTop20RankedPublishers(), 
        cacheDependency);
}

var cachedObject = _cache[cacheKey] as TObjectToCache;
if (cachedObject == null)
{
    using (TimedLock.Lock(CacheLock<TObjectToCache>.GetLock(), Timeout))
    {
        cachedObject = _cache[cacheKey] as TObjectToCache;
        if (cachedObject == null)
        {
            cachedObject = getObjectToCache();
            InsertIntoCache(cacheKey, cachedObject, cacheDependency, absoluteExpiration);
        }
    }
}

Timeout is an int property on the Cacher class, set to 5 (seconds) by default.  One reason why I hadn’t ever bothered to add locking to this code in the past was that I didn’t want to block additional requests for unknown periods of time, but the TimedLock is beautiful in that it lets you easily configure how long you’re willing for the lock to last.  the CacheLock<T> class looks like this, and generates a unique lock per type of thing you might be getting from the database.  If you are grabbing a lot of the same type (e.g. DataTable, DataSet, String) then you might need to use a different lock object strategy so you’re not being overly aggressive with your locking.

public class CacheLock<CachedObjectType>
{
    private static object objectLock = new object();

    public static object GetLock() { return objectLock; }
}

// define cache key

// new up dependencies

// call _cacher.Cache(key,()=> GetData, dependencies)

private TObjectToCache GetCachedObject<TObjectToCache>(string cacheKey, 
    Func<TObjectToCache> getObjectToCache,
    Func<CacheDependency> getCacheDependency, 
    DateTime absoluteExpiration)
    where TObjectToCache : class
{
    if (string.IsNullOrEmpty(cacheKey))
    {
        throw new ArgumentNullException("cacheKey", 
        "Given cache key cannot be null or empty.");
    }
    if (getObjectToCache == null)
    {
        throw new ArgumentNullException("getObjectToCache");
    }
    var cachedObject = _cache[cacheKey] as TObjectToCache;
    if (cachedObject == null)
    {
        using (TimedLock.Lock(CacheLock<TObjectToCache>.GetLock(), Timeout))
        {
            cachedObject = _cache[cacheKey] as TObjectToCache;
            if (cachedObject == null)
            {
                cachedObject = getObjectToCache();
                InsertIntoCache(cacheKey, 
                    cachedObject, 
                    getCacheDependency == null ? null : getCacheDependency(), 
                    absoluteExpiration);
            }
        }
    }
    return cachedObject;
}

public override IEnumerable<Publisher> GetTop20RankedPublishers()
{
    const string cacheKey = "GetTop20RankedPublishers()";

    return _cacher.Cache(cacheKey,
        () => BaseGetTop20RankedPublishers(),
        () => DependencyFactory.CreateSqlCacheDependency(PublisherTableDependency));
}

Summary

Analyzing memory leaks can be very tricky business.  The tools included with VS2010 are quite good.  I also played around with JetBrains dotTrace Memory 3.5 (trial) for this, and found it easy to use and helpful as well (here’s a screenshot of dotTrace):

The two takeaways I have from this for others are:

1) Implement locking on your cache access if you find that it’s causing problems.  If you don’t have pain, don’t necessarily worry about it.

2) Don’t create SqlCacheDependencies (or any CacheDependency) if you’re not going to use it.  In our case this was a problem caused by our implementation of the Cacher / ICacher interface, because we chose to take in the CacheDependency as a parameter.  Use delegates to get expensive objects you may or may not need, so that you can get them just-in-time and only if required.

Some things that made fixing this easy once the issue was identified:

1) Abstract your cache access.  We have an ICacher interface that defines how we add things to a cache.  We also, separately, have an ICache interface that wraps the actual cache implementation.  We have tests of ICacher that run without any real cache instance involved, which was invaluable in testing this change and getting it done in a few hours of coding.

2) Abstract your caching from your data access.  We’re using the Repository pattern heavily, along with an IOC container for wireup.  We have, for instance, an IFooRepository interface, and a FooRepository implementation that knows how to fetch data from the database (usually through an ORM), but knows nothing about caching.  We then use the Decorator pattern to implement caching, subclassing our FooRepository to create a CachedFooRepository.  This class simply overrides each method that we want to cache, and calls its base method to fetch the actual data.

Making the changes in our code to add locking and how we add cache dependencies was almost completely done in one file, and the calls to that code simply had to change from passing an instance to passing in a lambda, like () => GetCacheDependencies() or () => { return new SqlCacheDependency(foo,bar);}.  The Inline refactoring (ctrl-alt-N) proved to be very useful for this.

For more on this kind of things, you should follow me on twitter, and of course you can subscribe to my blog feed or via email.