SQL and Locking

[aquinas]

Hello,

I've been put in charge of making a lot of old ASP.NET 2.0 code thread safe after we started experiencing concurrency problems, particularly with database inserts. I've spent a lot of time searching and reading tutorials on the topic, but most of it is still going over my head. Nevertheless, I've put together a few functions to handle database updates, ex)

Public Class Podcast
    . . .

    Public Sub Insert_Podcast()
        Dim stick As New Object()
        SyncLock stick
            If me.ID = 0 Then
                Using conn As New SqlConnection(strConnection)
                    conn.Open()
                    Dim cmd As SqlCommand = conn.CreateCommand()
                    Dim trans As SqlTransaction = conn.BeginTransaction("Add_Transaction")
                    cmd.Connection = conn
                    cmd.Transaction = trans
                    Try
                        cmd.CommandText = Add_Podcast
                        cmd.Parameters.AddWithValue("@TITLE", me.title)
                        cmd.Parameters.AddWithValue("@FILENAME", me.filename)
                        cmd.Parameters.AddWithValue("@DESCRIPTION", me.description)
                        cmd.Parameters.AddWithValue("@VIEWS", me.views)
                        cmd.Parameters.AddWithValue("@LENGTH", me.length)
                        cmd.Parameters.AddWithValue("@SIZE", me.size)
                        cmd.Parameters.AddWithValue("@FORMAT", me.format)
                        cmd.Parameters.AddWithValue("@AUTHOR", me.author)
                        cmd.Parameters.AddWithValue("@RECORDED", me.recorded)
                        If cmd.ExecuteNonQuery() > 0 Then
                            cmd.CommandText = Get_New_ID
                            me.ID = CType(cmd.ExecuteScalar(), Integer)
                        End If
                        trans.Commit()
                    Catch Ex As Exception
                        trans.RollBack()
                    End Try
                End Using
            End If
        End SyncLock
    End Sub
    . . .
End Class

What I'd like to know is will this code be thread-safe as it is? Will it solve the concurrent database write problems? Am I right to be putting the locking object in an instance class?

Or am I horribly misinterpreting what I've read?

 

[jmcilhinney]

Thread-safety and data concurrency have very little to do with each other. What exactly is your issue? Are you getting incorrect IDs after inserting records?

 

[aquinas]

I'm sorry if I've got the ideas muddled together here.

The old code seemed to have both thread-safety and data concurrency issues. Under load, portions of users records were getting overwritten and mixed together and occasionally there would be problems with incorrect id's after inserting records. Occasionally, it seemed like sections of the code were executed entirely out of order. Generally, a giant mess.

Here, I'm trying to fix the data concurrency issue. Does this ensure that only one person at a time can insert into the database? I'd also read that the lock prevents instruction reordering, which seemed to be part of my problems.

 

[jmcilhinney]

There aren't any data concurrency issues with insertion. Where data concurrency issues occur is when two people retrieve the same data, edit it and then both try to save their changes. In that case, the second save will overwrite the first unless you use some sort of concurrency scheme. In that case you need to check before you save that what is currently in the database is what you retrieved in the first place. If it's not then you know someone else has saved changes since so you need to determine, usually with the user's involvement, how best to merge the two sets of changes.

In your case, it's not really a concurrency issue but rather just bad timing. You're telling the system to give you the last ID generated but you're waiting for a period of time before doing so. It may be a short period of time but it's still a period of time. What you should be doing is inserting the new record and retrieving the specific ID generated for that record in one command. You can then specify that you want the last ID generated within that scope and, even if a thousand other users are inserting data at the same time, it won't matter because all the other IDs are generated in different scopes. Here's an example of inserting a record into SQL Server and getting the ID generated:

Using connection As New SqlConnection("connection string here"),
      command As New SqlCommand("INSERT INTO Person (GivenName, FamilyName) VALUES (@GivenName, @FamilyName); SELECT @ID = SCOPE_IDENTITY()",
                                connection)
    With command.Parameters
        .AddWithValue("@GivenName", "Joe")
        .AddWithValue("@FamilyName", "Bloggs")
        .Add("@ID", SqlDbType.Int).Direction = ParameterDirection.InputOutput
    End With

    connection.Open()
    command.ExecuteNonQuery()
    connection.Close()

    Dim id = CInt(command.Parameters("@ID").Value)

    'Use id here.
End Using

Note the query in the same command as the insertion and the use of SCOPE_IDENTITY in order to ignore any IDs generated outside the current scope. That is basically the same way that you refresh the IDs in a DataTable when using a data adapter to save new records.

 

[aquinas]

Ah!

I'd seen code like that before, but had no idea what it was for.

Thank you for your help.