Pooling errors - any ideas?

C

Chris Ashley

We're getting occasional pooling errors on a web app. It seems to
happen for about 30 seconds or so, and then be fine for several hours,
so it's very difficult to replicate the circumstances in which it
happens. However the app is quite high load (Several hundred users at a
time)...

I use a base connection class which has the following code:

public class DataConnection : IDisposable
{
private SqlConnection sqlConn;
protected SqlCommand sqlCmd;

protected DataConnection()
{
try
{
sqlConn = new SqlConnection(GetDBaseConnectionString());
sqlConn.Open();

sqlCmd = new SqlCommand();
sqlCmd.Connection = sqlConn;
}
catch(System.Data.SqlClient.SqlException eSql)
{
throw new ArgumentException("Error opening database: " +
eSql.Message);
}
}

/* Apparently blank destructor is bad
~DataConnection()
{

}
*/

/// <summary>
/// Our implementation of the Dispose funtion from IDisposable.
/// </summary>
public void Dispose()
{
// Dispose of our db connection, and return it to the pool
sqlConn.Close();
sqlConn.Dispose();
GC.SuppressFinalize(sqlConn);

// Dispose our command object (not as important as the conn, but we
may as well do it here anyway)
sqlCmd.Dispose();
GC.SuppressFinalize(sqlCmd);

/* NOTE: We do NOT dispose ourself here. We have disposed of the db
connection,
which is the most important thing, so we can leave ourselves to be
cleaned up by the GC.
*/
}


private string GetDBaseConnectionString()
{
return ConfigurationSettings.AppSettings["DBConnStr"];
}
}


Then my data retrieval classes inherit the connection class like so:

public class DataQuote : DataConnection
{

public DataQuote() : base()
{
}

public string InitiateNewQuote(int intBranchId)
{
sqlCmd.CommandType = CommandType.StoredProcedure;
sqlCmd.CommandText = "sproc_InitiateNewQuote";
sqlCmd.Parameters.Add("@branchId", intBranchId);

try
{
return sqlCmd.ExecuteScalar().ToString();
}
catch(Exception e)
{
// Fatal error
throw new Exception("Error initiating new quote for branch id " +
Convert.ToString(intBranchId) + ". Error: " + e);
}
}

Is there anything wrong with my implementation?
 
S

Sahil Malik [MVP C#]

The class that is using DataQuote may not disposing the connection properly.

Instead of an architecture based on inheritance, you would probably be
better off by creating an architecture based on a layering mechanism instead
where one class calls another rather than inherits from it.

That way, you can ensure in the caller class that Dispose is always called.

- Sahil Malik [MVP]
ADO.NET 2.0 book -
http://codebetter.com/blogs/sahil.malik/archive/2005/05/13/63199.aspx
__________________________________________________________
 
C

Chris Ashley

Thanks for that.

Is enclosing the call in a 'using' block enough, or do I need to be
explicitly calling dispose?

Like so:

using (DataQuote objQuote = new DataQuote())
{
SqlDataReader objDtRdr =
objQuote.GetStepOneGeneralQuestions(strQuoteGuid);
if (objDtRdr.Read())
{
if (!Convert.ToBoolean(objDtRdr["noClaimMade"]) ||
!Convert.ToBoolean(objDtRdr["noClaimAgainst"]))
{
blnInsistOnClaim = true;
}
*/
if (!Convert.ToBoolean(objDtRdr["noClaimMade"]))
{
blnInsistOnClaim = true;
}
}
}
 
M

Marina Levit [MVP]

You didn't actually give us the error message you get, you just gave a vague
'connection pooling' description, but I will assume you are running out of
connections from the pool.

With a web app, you should not be opening a connection and keeping it open
indefinitely. When you wait for Dispose on your page to kill the
connection, it gets killed only when the GC needs to reclaim memory - who
knows when that will be?

Every method should open its own connection, do its work, and then close it
again. You need to use try/catch/finally blocks, and close the connection
in the Finally to ensure that it always gets called.

Chris Ashley said:
We're getting occasional pooling errors on a web app. It seems to
happen for about 30 seconds or so, and then be fine for several hours,
so it's very difficult to replicate the circumstances in which it
happens. However the app is quite high load (Several hundred users at a
time)...

I use a base connection class which has the following code:

public class DataConnection : IDisposable
{
private SqlConnection sqlConn;
protected SqlCommand sqlCmd;

protected DataConnection()
{
try
{
sqlConn = new SqlConnection(GetDBaseConnectionString());
sqlConn.Open();

sqlCmd = new SqlCommand();
sqlCmd.Connection = sqlConn;
}
catch(System.Data.SqlClient.SqlException eSql)
{
throw new ArgumentException("Error opening database: " +
eSql.Message);
}
}

/* Apparently blank destructor is bad
~DataConnection()
{

}
*/

/// <summary>
/// Our implementation of the Dispose funtion from IDisposable.
/// </summary>
public void Dispose()
{
// Dispose of our db connection, and return it to the pool
sqlConn.Close();
sqlConn.Dispose();
GC.SuppressFinalize(sqlConn);

// Dispose our command object (not as important as the conn, but we
may as well do it here anyway)
sqlCmd.Dispose();
GC.SuppressFinalize(sqlCmd);

/* NOTE: We do NOT dispose ourself here. We have disposed of the db
connection,
which is the most important thing, so we can leave ourselves to be
cleaned up by the GC.
*/
}


private string GetDBaseConnectionString()
{
return ConfigurationSettings.AppSettings["DBConnStr"];
}
}


Then my data retrieval classes inherit the connection class like so:

public class DataQuote : DataConnection
{

public DataQuote() : base()
{
}

public string InitiateNewQuote(int intBranchId)
{
sqlCmd.CommandType = CommandType.StoredProcedure;
sqlCmd.CommandText = "sproc_InitiateNewQuote";
sqlCmd.Parameters.Add("@branchId", intBranchId);

try
{
return sqlCmd.ExecuteScalar().ToString();
}
catch(Exception e)
{
// Fatal error
throw new Exception("Error initiating new quote for branch id " +
Convert.ToString(intBranchId) + ". Error: " + e);
}
}

Is there anything wrong with my implementation?
 
C

Chris Ashley

Sorry, yes. My mistake.. My error in the log is:

System.Web.HttpUnhandledException: Exception of type
System.Web.HttpUnhandledException was thrown. --->
System.InvalidOperationException: Timeout expired. The timeout period
elapsed prior to obtaining a connection from the pool. This may have
occurred because all pooled connections were in use and max pool size
was reached.

I think I may have fixed it now. I had all my statements in 'using'
blocks creating a DataQuote object, but there was a rogue statement
which wasn't enclosed in a using block in the app. I believe that a
'using' block means my dispose method automatically gets called at the
end of the routine, so this should be okay?

Thanks for the advice. I may change the architecture at some point but
for now I just needed to fix the errors quickly.
 
M

Mythran

Chris Ashley said:
We're getting occasional pooling errors on a web app. It seems to
happen for about 30 seconds or so, and then be fine for several hours,
so it's very difficult to replicate the circumstances in which it
happens. However the app is quite high load (Several hundred users at a
time)...

I use a base connection class which has the following code:

public class DataConnection : IDisposable
{
private SqlConnection sqlConn;
protected SqlCommand sqlCmd;

protected DataConnection()
{
try
{
sqlConn = new SqlConnection(GetDBaseConnectionString());
sqlConn.Open();

sqlCmd = new SqlCommand();
sqlCmd.Connection = sqlConn;
}
catch(System.Data.SqlClient.SqlException eSql)
{
throw new ArgumentException("Error opening database: " +
eSql.Message);
}
}

/* Apparently blank destructor is bad
~DataConnection()
{

}
*/

/// <summary>
/// Our implementation of the Dispose funtion from IDisposable.
/// </summary>
public void Dispose()
{
// Dispose of our db connection, and return it to the pool
sqlConn.Close();
sqlConn.Dispose();
GC.SuppressFinalize(sqlConn);

// Dispose our command object (not as important as the conn, but we
may as well do it here anyway)
sqlCmd.Dispose();
GC.SuppressFinalize(sqlCmd);

/* NOTE: We do NOT dispose ourself here. We have disposed of the db
connection,
which is the most important thing, so we can leave ourselves to be
cleaned up by the GC.
*/
}


private string GetDBaseConnectionString()
{
return ConfigurationSettings.AppSettings["DBConnStr"];
}
}


Then my data retrieval classes inherit the connection class like so:

public class DataQuote : DataConnection
{

public DataQuote() : base()
{
}

public string InitiateNewQuote(int intBranchId)
{
sqlCmd.CommandType = CommandType.StoredProcedure;
sqlCmd.CommandText = "sproc_InitiateNewQuote";
sqlCmd.Parameters.Add("@branchId", intBranchId);

try
{
return sqlCmd.ExecuteScalar().ToString();
}
catch(Exception e)
{
// Fatal error
throw new Exception("Error initiating new quote for branch id " +
Convert.ToString(intBranchId) + ". Error: " + e);
}
}

Is there anything wrong with my implementation?

Something I see, just by skimming through the code, is that you are calling
Dispose on the SqlConnection object. I am not 100% positive, but I believe
if you explicitly call Dispose on the connection object, you are not
releasing the connection back to the pool, but instead ?destroying? it...if
you call Close() then you would be releasing it back into the pool...but
once again, I'm not positive.

Mythran
 
S

Sahil Malik [MVP C#]

Dispose will call close, so no worries there.

SM


Mythran said:
Chris Ashley said:
We're getting occasional pooling errors on a web app. It seems to
happen for about 30 seconds or so, and then be fine for several hours,
so it's very difficult to replicate the circumstances in which it
happens. However the app is quite high load (Several hundred users at a
time)...

I use a base connection class which has the following code:

public class DataConnection : IDisposable
{
private SqlConnection sqlConn;
protected SqlCommand sqlCmd;

protected DataConnection()
{
try
{
sqlConn = new SqlConnection(GetDBaseConnectionString());
sqlConn.Open();

sqlCmd = new SqlCommand();
sqlCmd.Connection = sqlConn;
}
catch(System.Data.SqlClient.SqlException eSql)
{
throw new ArgumentException("Error opening database: " +
eSql.Message);
}
}

/* Apparently blank destructor is bad
~DataConnection()
{

}
*/

/// <summary>
/// Our implementation of the Dispose funtion from IDisposable.
/// </summary>
public void Dispose()
{
// Dispose of our db connection, and return it to the pool
sqlConn.Close();
sqlConn.Dispose();
GC.SuppressFinalize(sqlConn);

// Dispose our command object (not as important as the conn, but we
may as well do it here anyway)
sqlCmd.Dispose();
GC.SuppressFinalize(sqlCmd);

/* NOTE: We do NOT dispose ourself here. We have disposed of the db
connection,
which is the most important thing, so we can leave ourselves to be
cleaned up by the GC.
*/
}


private string GetDBaseConnectionString()
{
return ConfigurationSettings.AppSettings["DBConnStr"];
}
}


Then my data retrieval classes inherit the connection class like so:

public class DataQuote : DataConnection
{

public DataQuote() : base()
{
}

public string InitiateNewQuote(int intBranchId)
{
sqlCmd.CommandType = CommandType.StoredProcedure;
sqlCmd.CommandText = "sproc_InitiateNewQuote";
sqlCmd.Parameters.Add("@branchId", intBranchId);

try
{
return sqlCmd.ExecuteScalar().ToString();
}
catch(Exception e)
{
// Fatal error
throw new Exception("Error initiating new quote for branch id " +
Convert.ToString(intBranchId) + ". Error: " + e);
}
}

Is there anything wrong with my implementation?

Something I see, just by skimming through the code, is that you are
calling Dispose on the SqlConnection object. I am not 100% positive, but
I believe if you explicitly call Dispose on the connection object, you are
not releasing the connection back to the pool, but instead ?destroying?
it...if you call Close() then you would be releasing it back into the
pool...but once again, I'm not positive.

Mythran
 
S

Sahil Malik [MVP C#]

Using block is enough.

I have a quick tip for you.

Don't do SuppressFinalize, instead let the finalizer run, and in the
finalizer, do a stackwalk and find out "who is holding your reference". That
is your culprit, thats the guy that should have called Dispose and didn't :)

It'll just help ya zero down on the errors quicker.

SM
 
W

William \(Bill\) Vaughn

This behavior can be expected--especially on a heavily loaded site. What's probably happening, is that when the flood of requests come in, the server is overwhelmed. Since ADO.NET and SS has to initialize each connection, that takes a bit more time than normal. The first time you execute any ADO.NET operation a number of DLLs have to be loaded and initialized. The first time a database is hit, none of its resources are in the cache so SPs have to be compiled, tables have to be read from disk etc. First time access is slow. After that, everything you need (to do about the same thing) is cached--the connection, the procedure, the data (for the most part). Once the system stabilizes, it can leverage this cached pool of stuff and run more smoothly.
Ok, how to solve this problem? I might suggest:
a.. Set the ConnectionString Min Pool Size to X where X=the number of connections you typically need at full capacity. Generally, a heavily loaded (but balanced) system won't need more than 25 or so connections in the pool.
b.. Run a "Startup" application before opening the site that opens a connection and performs a typical operation. This primes the ConnectionPool and the server proc and data caches.
c.. Monitor the pool to ensure that things are running smoothly. Consider adding more RAM to the system or making the queries more efficient (or both) to prevent overloading the box.
hth


--
William (Bill) Vaughn
President and Founder Beta V Corporation
Redmond, WA
(425) 556-9205
Microsoft MVP, Author, Mentor
Microsoft MVP
Chris Ashley said:
We're getting occasional pooling errors on a web app. It seems to
happen for about 30 seconds or so, and then be fine for several hours,
so it's very difficult to replicate the circumstances in which it
happens. However the app is quite high load (Several hundred users at a
time)...

I use a base connection class which has the following code:

public class DataConnection : IDisposable
{
private SqlConnection sqlConn;
protected SqlCommand sqlCmd;

protected DataConnection()
{
try
{
sqlConn = new SqlConnection(GetDBaseConnectionString());
sqlConn.Open();

sqlCmd = new SqlCommand();
sqlCmd.Connection = sqlConn;
}
catch(System.Data.SqlClient.SqlException eSql)
{
throw new ArgumentException("Error opening database: " +
eSql.Message);
}
}

/* Apparently blank destructor is bad
~DataConnection()
{

}
*/

/// <summary>
/// Our implementation of the Dispose funtion from IDisposable.
/// </summary>
public void Dispose()
{
// Dispose of our db connection, and return it to the pool
sqlConn.Close();
sqlConn.Dispose();
GC.SuppressFinalize(sqlConn);

// Dispose our command object (not as important as the conn, but we
may as well do it here anyway)
sqlCmd.Dispose();
GC.SuppressFinalize(sqlCmd);

/* NOTE: We do NOT dispose ourself here. We have disposed of the db
connection,
which is the most important thing, so we can leave ourselves to be
cleaned up by the GC.
*/
}


private string GetDBaseConnectionString()
{
return ConfigurationSettings.AppSettings["DBConnStr"];
}
}


Then my data retrieval classes inherit the connection class like so:

public class DataQuote : DataConnection
{

public DataQuote() : base()
{
}

public string InitiateNewQuote(int intBranchId)
{
sqlCmd.CommandType = CommandType.StoredProcedure;
sqlCmd.CommandText = "sproc_InitiateNewQuote";
sqlCmd.Parameters.Add("@branchId", intBranchId);

try
{
return sqlCmd.ExecuteScalar().ToString();
}
catch(Exception e)
{
// Fatal error
throw new Exception("Error initiating new quote for branch id " +
Convert.ToString(intBranchId) + ". Error: " + e);
}
}

Is there anything wrong with my implementation?
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top