Further investigation has revealed that the query is somehow leaving connections open to the database, so it's not the query, but hte calling code.

The code does have a close conenction statement in the finally part of the try catch, but somehow it's leaving connections open. Does anyone know why this would leave connections open?

                try
                {

                    SqlCommand sqlCmd = new SqlCommand(ProcedureNames.LocationLookup_GetLocation, connDBConnection);
                    sqlCmd.CommandType = CommandType.StoredProcedure;
                    sqlCmd.Parameters.AddWithValue("@IpAddressAsDecimal", ipAddress.IpAsDecimal);

                    SqlDataReader reader = sqlCmd.ExecuteReader();

                    if (reader.Read())
                    {
                        location = Convert.ToString(reader["location"]);
                    }
                    else location = "Unknown";

                }
                catch (Exception ex)
                {
                    location = "Unknown";
                }
                finally
                {
             if (connDBConnection.State == ConnectionState.Open)
                connDBConnection.Close();
                }

Connections can sometimes stay open because of connection pooling. This may not be a bad thing but..

I would also be inclined to close the reader and dispose the connection.

              try
                {

                    SqlCommand sqlCmd = new SqlCommand(ProcedureNames.LocationLookup_GetLocation, connDBConnection);
                    sqlCmd.CommandType = CommandType.StoredProcedure;
                    sqlCmd.Parameters.AddWithValue("@IpAddressAsDecimal", ipAddress.IpAsDecimal);

                    SqlDataReader reader = sqlCmd.ExecuteReader();
                    location = "Unknown"; 
                   

                   if (reader.Read())
                    {

                       // Think we should check for null .  Assuming also column 0 is the right one for location. Not great for maintence hence a scaler might better.

                       if (reader.IsDBNull(0)) {
                        location = reader.GetString(0)

                        }
                    }
                   

                     reader.Close();

                }// Taken out exception handling lets it go up the chain.
                finally
                {
             if (connDBConnection.State == ConnectionState.Open)
                connDBConnection.Close();

                connDBConnection.Dispose();
                }

Also catching the exception and handling with location="Unknown" is very bad idea imho you dont know if you have now problems with memory as you are handling better to let this bubble. If you have issues in this bit of code you have much larger issues also.

Also if I am going to picky if the statement only returns one value out of the stored procedure why not use Scaler ? Faster, and cleaner with a using statement to boot you would be sorted.

http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlcommand.executescalar.aspx

Sorry for mass edit hope this helps.  Simple change at first would be just to close the reader but I would be worried about handling exceptions at this level. My guess is this was done to handle null values in the reader?

tencents,

James

Thanks James, I'll give this a try.

I didn't write the code and I've used the DAAB for years which handles disposing of the connections so I never thought of that. I would have used executescalar myself, though I'm happy with setting the value to "unknown" if no record is found in the data. It's querying a list of know IP address block to determine if the location is a specific area, the rest of Britain, or neither.

Thanks

Tom

James,

that didn't solve the problem, I'll look into it further. It is related to connections being kept in the connection pool and I saw something previously about being able to reset the conenction pool, but I'm not sure if that's a good idea. Within a minute of testing, I can have 100 open connections to the database, though the errors starts much sonner than than. I'll post the solution when I find one.

Thansk

Tom

Strange, might be worth looking at the stored proc, Anything else in there?

Dont think resetting the connection pool is a good idea. Any chance this could be mixed in with some caching of the control by umbraco ?

Will have a think about this today,

James

James,

I tried changing this element from being xslt to a .net usercontrol and it made to difference.

I tried a quick and dirty totally new stored proc created from a View and it made no difference.

If I run the MS Web Application Stress Tool on the page, it has 50 open connections to the DB within 20-30 seconds, though this doesn't seem to happen if I manually refresh the page (though I maybe didn't try refreshing enough times fast enough). Within a minute with WAST the connection pool actually reaches 100 and fails totally, giving a time out on the page and a message that the conenction pool might be full.

I've actually reduced the test page to only showing the text returned from the function and nothing else (not even an html tag) and the problem still occurs.

My next test is create an aspx page utilising the Usercontrol, bypassing Umbraco and see if the error still happens, though that won't be until this evening before I get to try that.

Tom

Good plan to move the usercontrol out of umbraco and give it a whirl. Have you tried just putting the control on to a vanilla template and checking it from there.

is working on a reply...