Storing data into an SQL table using multiple threads and queues

I have a process running in a separate thread which reads data over Ethernet and raises an event for each read. Then, this data is processed over a tree of classes. At the top of my class tree, I want to store the processed data to SQL tables. This read rate can be rather fast and often at times but on average its slower than my write rate to SQL. This is why I want a queue. Also I want my window not to freeze, so I want a separate thread.

The code below works but I'm fairly inexperienced in multi threading. Is this a good and robust way to accomplish my task?

Private addLogQueue As New Queue
Private dequeueing As Boolean = False
Public Sub addLogHandler(ByVal machineName As String, ByVal LogTime As Date, ByVal EventLog As String, ByVal EventValue As String)
addLogQueue.Enqueue("INSERT INTO ... some SQL CODE")
Dim Thread1 As New System.Threading.Thread(AddressOf addLog)
Thread1.Start
End Sub
Private Sub addLog()
If Not dequeueing Then
dequeueing = True
While addLogQueue.Count <> 0
Try
SQLCon.Open()
SQLCmd = New SqlCommand(addLogQueue.Dequeue(), SQLCon)
SQLCmd.ExecuteNonQuery()
SQLCon.Close()
Catch ex As Exception
MsgBox(ex.Message)
If SQLCon.State = ConnectionState.Open Then
SQLCon.Close()
End If
End Try
End While
dequeueing = False
End If
End Sub


After thinking a bit I realize I might not even need a queue or the dequeueing boolean, can I just start a new thread for every write maybe. I added the queue before I added the multi thread because at times I had to write before the SQLcon is even closed. Since I've added a thread, should I even keep the queue?

Single-Threading Style:

Managing resources:

You should be using using to manage your connection. It completely eliminates that... unsightly Try-Catch-Block. Compare following snippet from msdn with a similarly stripped version of your code:

Using connection As New SqlConnection(connectionString)
connection.Open()
' Do work here; connection closed on following line.
End Using


Try
SQLCon.Open()
' Do work here; connection closed on following line.
SQLCon.Close()
Catch ex As Exception
MsgBox(ex.Message)
If SQLCon.State = ConnectionState.Open Then
SQLCon.Close()
End If
End Try


Naming

While SQLCon and SQLCmd are ... conventional names, they are a little short and have the air of systems-hungarian... I'd prefer connection and command.

Also Thread1 is really non-speaking, InsertingThread may be the better option.

Whitespace

I like to keep separate Subs clearly outlined by an empty line before starting with [Modifier] Sub ...

Comments and documentation

You may have removed it, but ... this code is extremely undercommented and underdocumented. A comment here and there can help a little wit easing the reading. Then again this code is rather simple and obvious by the names...

Multi-Threading Style:

First off, you currently only allow a single Thread to run... well almost, because of possible interleaving and nonatomic comparison operations.

Consider the following:

Thread 1 Thread 2

• If Not deququeing

- If Not dequeuing
- dequeueing = True
- dequeueing = True


Suddenly you have 2 threads running and that when you could've had everything easy.

The queue docs mention ConcurrentQueue(Of T) when you "need to access the queue across multiple threads"

This makes the failed attempt at synchronizing with a shared boolean unnecessary and overall improves the code quality.

End result:

After applying my critiques I end up with following code:

Private addLogQueue As New ConcurrentQueue(Of String)

Public Sub addLogHandler(ByVal machineName As String, ByVal LogTime As Date, ByVal EventLog As String, ByVal EventValue As String)
addLogQueue.Enqueue("INSERT INTO ... some SQL CODE")
Dim InsertingThread As New System.Threading.Thread(AddressOf addLog)
InsertingThread.Start
End Sub

Private Sub addLog()
Using (connection As New SqlConnection(connstring))
connection.Open()
Dim statement As String
While addLogQueue.TryDequeue(out statement)
command = New SqlCommand(statement, connection)
command.ExecuteNonQuery()
End While
End Using
End Sub


StackExchange Code Review Q#96398, answer score: 3