views:

165

answers:

2

Hi,

I created a web service which is called from the client side to store the data into the database. These data are sent every 200 ms from a single user and each time the data are sent the database connection is opened and closed which I believe is not good for performance.

The data are stored by calling REST.StoreAcceleration() method and SQLWorks.StoreAcceleration() the following way:

public Response StoreAcceleration(string strSessionString, string strMeasurementTime, string strAccelerationX, string strAccelerationY, string strAccelerationZ)
    {
        SQLWorks sqlWorks = new SQLWorks();
        Response response = new Response();
        try
        {
            string strTime = strMeasurementTime.Replace("_", " ");
            DateTime measurementTime = DateTime.ParseExact(strTime, "yyyy-MM-dd HH:mm:ss:fff", null);
            double accelerationX = Convert.ToDouble(strAccelerationX.Replace(".", ","));
            double accelerationY = Convert.ToDouble(strAccelerationY.Replace(".", ","));
            double accelerationZ = Convert.ToDouble(strAccelerationZ.Replace(".", ","));

            sqlWorks.StoreAcceleration(strSessionString, measurementTime, accelerationX, accelerationY, accelerationZ);

            response.Successful = true;
            response.Comment = "Stored!";
        }
        catch(Exception ex)
        {
            string sDummy = ex.ToString();
            response.Comment = "an error occured!";
            response.Successful = false;
        }

        return response;
    }

public bool StoreAcceleration(string strStringSession, DateTime receivedTime, double accelerationX, double accelerationY, double accelerationZ)
    {
        bool result = false;
        string select =
            "INSERT INTO acceleration (session_id, measurement_time, acceleration_x, acceleration_y, acceleration_z) VALUES (@sessionID, @measurementTime, @accelerationX, @accelerationY, @accelerationZ)";
        SqlConnection conn = new SqlConnection(connectionString);
        SqlCommand cmd = new SqlCommand(select, conn);
        int sessionID = getSessionID(strStringSession);
        if(sessionID == 0)
            return false;
        updateSessions(sessionID);
        string strRecordTime = receivedTime.ToString("yyyy-MM-dd HH:mm:ss:fff");
        cmd.Parameters.AddWithValue("sessionID", sessionID.ToString());
        cmd.Parameters.AddWithValue("measurementTime", strRecordTime);
        cmd.Parameters.AddWithValue("accelerationX", accelerationX.ToString());
        cmd.Parameters.AddWithValue("accelerationY", accelerationY.ToString());
        cmd.Parameters.AddWithValue("accelerationZ", accelerationZ.ToString());
        try
        {
            conn.Open();
            cmd.ExecuteNonQuery();
            result = true;
        }
        catch(Exception ex)
        {
            string sDummy = ex.ToString();
        }
        finally
        {
            conn.Close();
        }
        return result;
    }

The problem here is that SqlConnection is opened and closed on every method call.

I would appreciate if anyone could suggest how to improve the solution in order to prevent frequent database connection opening/closing.

Thanks!

+5  A: 

If you have a connection pool set up, your database connection will not get closed. Never mind the conn.Close(). From MSDN:

The Close method rolls back any pending transactions. It then releases the connection to the connection pool, or closes the connection if connection pooling is disabled.

See here to set it up, if you're not already using it: SQL Server connection pooling (ADO.NET) and connection strings.

Basically, unless you have pooling=false or something similar in your connection string, it should already be active. However, you might want to set some MinPoolSize to always have a couple of connections ready to be used.


By the way, are you storing the received time as actual strings? Otherwise you can get rid of the whole ToString(..) thing. ADO.NET will make sure the date doesn't get misinterpreted. The same goes for the other values really; why are you converting them to strings?

Lastly, SqlCommand implements IDisposable, so you should be disposing it, much like the connection. I'd suggest rewriting to something like this:

public bool StoreAcceleration(string strStringSession, DateTime receivedTime, double accelerationX, double accelerationY, double accelerationZ)
{
    string select =
        "INSERT INTO acceleration (session_id, measurement_time, acceleration_x, acceleration_y, acceleration_z) VALUES (@sessionID, @measurementTime, @accelerationX, @accelerationY, @accelerationZ)";

    int sessionID = getSessionID(strStringSession);
    if (sessionID == 0)
        return false;
    updateSessions(sessionID);

    using (SqlConnection conn = new SqlConnection(connectionString))
    using (SqlCommand cmd = new SqlCommand(select, conn))
    {
        cmd.Parameters.AddWithValue("sessionID", sessionID);
        cmd.Parameters.AddWithValue("measurementTime", receivedTime);
        cmd.Parameters.AddWithValue("accelerationX", accelerationX);
        cmd.Parameters.AddWithValue("accelerationY", accelerationY);
        cmd.Parameters.AddWithValue("accelerationZ", accelerationZ);

        try
        {
            conn.Open();
            cmd.ExecuteNonQuery();
            return true;
        }
        catch (Exception ex)
        {
            return false;
        }
    }
}

You no longer need the Close() call in a finally block. The using block will implicitly call Dispose() when the variable conn falls out of scope. That will in turn call the Close() method internally.

Thorarin
+1 excellent advice - especially on using the `using` statements. Way too few ADO.NET programmers seems to know and embrace that...
marc_s
A: 

By default the connection pooling is enabled, so re-open a database connection is not that expensive like we thought. It is perfectly fine to open and close connection for each web service request.

codemeit