Could this code kill my server ?

Question

Hello

Im having an ongoing issue with my site, it basically times out and dies. I have gotten to the point now where I have had to set the application pool to auto recycle every 5 minutes, but even that has failed as I’ve just got back from work and my email inbox is full of 4000 emails all with the same error.

System.Data.SqlClient.SqlException: Timeout expired. The timeout period elapsed prior to completion of the operation or the server is not responding.

This morning I tried a test where I disabled pooling on the connection string, this also didn’t work.

Now I'm thinking that perhaps this isn’t an issue with the leaky connections, I've been through all that before, I think it maybe something to do with the static properties that are core to my site

Here is one of them

public static List<Member> AllMembers
{
    get
    {
        if (HttpRuntime.Cache["Members"] != null)
        {
            return (List<Member>)HttpRuntime.Cache["Members"];
        }
        else
        {
            GetAllMembers();
            return (List<Member>)HttpRuntime.Cache["Members"];
        }
    }
}

This is called whenever I want a list of the members, you can see that if its null it populates the cache, which will use the database, and if it’s not null then it will return the cache object. I also have SQLCacheDependancy too which will clear these cache objects so it will again populate them. So this property gets called ALOT.

Now this is a web application and as my traffic has increased its dyeing all the time,

Could my properties be the cause?

Any help is most appreciated

Truegilly

Answer 1

Just a few usual suspects:

1. Are you disposing all your SqlCommand's and readers?

2. Are you disposing/closing your SqlConnection's?

3. Is your database able to handle the load? Do you have performance problems there?

The code from your post doesn't look like the problem.

Answer 2

Assuming you are disposing of everything correctly, I have an alternative explaination:

If the cache is empty/expired and multiple pages try to call AllMembers at the same time, then each page may will end up calling GetAllMembers() simultaneously, slowing down the database query. This could start a vicious cycle if the calls start timing out.

You could place a lock around your code, so only one database query per property can be made. Here is how I might set it up:

private static object _allMembersLock = new object();
public static List<Member> AllMembers
{
    get
    {
        lock (_allMembersLock)
        {
            List<Member> members = (List<Member>)HttpRuntime.Cache["Members"];
            if (members == null)
            {
                members = GetAllMembers();
                HttpRuntime.Cache["Members"] = members;
            }
            return members;
        }
    }
}

Answer 3

I don't see any guard here to prevent multiple concurrent calls to the getter triggering GetAllMembers. Surely you only want one thread at a time to load the property. Some kind of lock() appears to be required.

You should make sure that the code that flushes the cache also uses the same lock().

If this code can be called from multiple servers, you also need to make sure that any database-side program logic (Stored procedures, or dynamic SQL from the client) that underpins GetAllMembers efficiently and correctly handles multiple concurrent readers.