Copied to clipboard

Flag this post as spam?

This post will be reported to the moderators as potential spam to be looked at


  • Tom Madden 252 posts 454 karma points MVP 2x c-trib
    Sep 16, 2009 @ 22:27
    Tom Madden
    0

    Has anyone else had SQL queries causing back-end errors in Umbraco 3.0.5?

    I've been maintaining an Umbraco site which has been giving errors since some new functionality was added. I've narrowed it down do a SQL query executed in a call to a seperate class file file from an XSLT macro. The problem only occurs intermittently when the server is under a small amount of load and results in

    No node exists with id '2877'

    The ID is the ID of a content node which does exist and the error can happen on any content node. If I leave all the C# code intact and remove the call to the Query, the problem goes away, proving that executing the quert, or the ExecuteReader statement which calls it is the culprit.

    The Query retreives a single value from a join between 2 tables and the tables contain data which doesn't relate to Umbraco itslef so it stored in 2 separate tables.

    I would need to ask for permission before posting the code as I didn't write it. I will ask for permission tomorrow and post the code, but in the meantime, has anyone else experienced a similar problem in 3.0.5?

    TIA

    Tom Madden
    Quix Ltd

     

     

  • Tom Madden 252 posts 454 karma points MVP 2x c-trib
    Sep 16, 2009 @ 23:33
    Tom Madden
    0

    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();
                    }
  • Jameskn 64 posts 78 karma points
    Sep 17, 2009 @ 01:38
    Jameskn
    0

     

    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

     

     


     

     

  • Tom Madden 252 posts 454 karma points MVP 2x c-trib
    Sep 17, 2009 @ 08:18
    Tom Madden
    0

    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

     

  • Tom Madden 252 posts 454 karma points MVP 2x c-trib
    Sep 17, 2009 @ 09:27
    Tom Madden
    0

    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

  • Jameskn 64 posts 78 karma points
    Sep 17, 2009 @ 11:09
    Jameskn
    0

    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

  • Tom Madden 252 posts 454 karma points MVP 2x c-trib
    Sep 17, 2009 @ 12:03
    Tom Madden
    0

    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

  • Jameskn 64 posts 78 karma points
    Sep 17, 2009 @ 13:05
    Jameskn
    0

    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.

Please Sign in or register to post replies

Write your reply to:

Draft