Storing data into an SQL table using multiple threads and queues

Question

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?

Answer 1

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