.NET SqlConnection class, connection pooling and reconnection logic

Question

We have some client code which is using the SqlConnection class in .NET to talk to a SQLServer database. It is intermittently failing with this error:

"ExecuteReader requires an open and available Connection. The connection's current state is Closed"

The "temporary" solution is to reboot the process, after which everything works - however, that's obviously unsatisfactory.

The code is keeping a cache of SqlConnection instances, one for each database.

We'd like to re-write the code, but before I do, I need to know a few things:

My first question is: Is it inefficient to repeatedly connect and disconnect SqlConnection objects, or does the underlying library perform connection pooling on our behalf?

// Is this bad/inefficient?
for(many-times)
{
    using(SQLConnection conn = new SQLConnection(connectionString))
    {
        // do stuff with conn
    }
}

Because our code does not do the above, what seems the likely cause of the problem is that something happens to the underlying SQLServer database during the "lifetime" of the connection that causes the connection to be closed...

If it turns out that it is worthwile to "cache" SqlConnection objects, what is the recommended way to handle all errors that could be resolved simply by "reconnecting" to the database. I'm talking about scenarios such as:

• The database is taken offline and brought back online, but the client process had no open transactions while this was happening
• The database was "disconnected", then "reconnected"

I notice that there is a "State" property on SqlConnection... is there an appropriate way to query that?

Finally, I have a test SQLServer instance set up with full access rights: how can I go about reproducing the exact error "ExecuteReader requires an open and available Connection. The connection's current state is Closed"

Solution 1

No, it's not inefficient to create lots of SqlConnection objects and close each of them when you're done. That's exactly the right thing to do. Let the .NET framework connection pooling do its job - don't try to do it yourself. You don't need to do anything specific to enable connection pooling (although you can disable it by setting Pooling=false in your connection string).

There are many things that could go wrong if you try to cache the connection yourself. Just say no :)

Solution 2

You should enable connection pooling on your connection string. In that case the runtime will add back your connections to the 'pool' when you close them, instead of really disconencting. When a 'new' connection is taken out of the pool it will be reset (ie. sp_reset_connection is called ) then presented to your application as a brand new, fresh connection. The pool is handling transparently such cases as if the connection is closed while idling in the pool.

The cost of creating a new connection 'from scratch' is significant because the authentication requires several roundtrips between client and server (depending on the authentication method and on SSL settings it can be 1 roundtrip in best case vs. about 10 in worse).

And to answer your question, connection raise the OnStateChange event when their state changes, but you shouldn't care about this if you use the pooling.

Solution 3

In my recent experience if you use this code:

using(SQLConnection conn = new SQLConnection(connectionString))
{
    // do stuff with conn
}

have an error, and do not explicitly close the connection, it will not be closed or checked back into the pool. So use a catch or finally block to close the connection

Comments

[Paul Hollingsworth]

What I was hoping to hear... Any ideas on how I could reproduce the exact error we are seeing? (just to "prove" that I've fixed the problem)?

[Jon Skeet]

Hard to say, to be honest. It could easily be a race condition, if you're trying to use the same connection from multiple threads.

[Richard]

To disable connection pooling: add "Pooling=False;" to the connection string.

[Richard]

Connection pooling is the default, you don't need to do anything to enable it. You can disable it, but they is a rare requirement.

[Jon Skeet]

@Richard: Yup, I added that a few minutes before your comment :)

[Anders Rune Jensen]

Please beware that if you have connection pooling enabled and you open a connection to check if a database exists, and then create the database based on this information. Then the sp_reset_connection trick will not enable you to see the database. So do the first connection without connection pooling.

[cmart]

@JonSkeet Sorry to poke on a such an old topic. But what about GC costs? I mean even though when the underlying SQL connection is pooled (still this connection is probably reset when disposing it which also makes calls to the DB: sp_reset_connection), the created SqlConnection instances will also need to be GC'd. So why not put the loop inside the using block and save connection reset and GC costs? Only reason i could think about is a possible use-case specific transactional safety for each DB operation. So what else were you thinking about concerning your answer back in 2009? ;-)

[vpalmu]

Turns out there's a critical level bug in the built-in connection pool. I got here looking for gotchas in trying to build your own when the built-in one already known to be unusable to us.

[AaA]

I'm having problem that I think is because of connection pooling. Lets say network connection dropped for few millisecond, ping is successful, telnet to SQL server port is connecting too, but .NET application keep giving connection error until either, I disable/enable network from cpanel or restart the app. Even though connection is created in using(){} block, it seems .net doesn't try to reconnect. It seems it is happening on .NET 4.7 and above, since the same code works fine with 4.5 and 4.0