HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Database threading design

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasethreadingdesign

Problem

I am new to threading and I am a junior developer, so I guess there are many mistakes. My scenario is this:

  • look into the database



  • if there are data which should be sent



  • add the data to the queue



  • if queue is not empty



  • send the next message and remove it from the queue



  • sleep 10 seconds



  • if a message from another thread arrived



  • stop waiting and go to next message in queue



  • if no message from another thread arrived



  • go back to the first step, but do this only 2 times



  • if a message still hasn't arrived from another thread



  • pass to the next message until queue is empty



  • return to the first step



I am trying to do this like this:

private Thread ReceiveThread;
private Thread SendThread;
internal static Thread ServiceThread;


These 3 threads:

ReceiveThread = new Thread(ReceiveTask);
ReceiveThread.Start();
ServiceThread = new Thread(SerAutoThread.SendServiceMsg);
ServiceThread.Start();
SendThread = new Thread(SendTask);
SendThread.Start();


```
class SerAutoThread
{
internal static object[] NextService;
public static readonly object _locker = new object();
internal static Queue Services;
internal static int sendingTime = 0;
private static DatabaseFirebird DB;
internal static void SendServiceMsg()
{
DB = new DatabaseFirebird();
DB.Open(ConnectionStr);
Services = new Queue();
while (true)
{
if (Services.Count != 0)
{
SetNextSerAndSend();
}
else
{
CheckAndSetServices();
}
}
}

private static void SetNextSerAndSend()
{
NextService = Services.Dequeue();
for (int j = 0; j < 4; j++)
{
if (sendingTime == TRANSMITTED)
{
//pass to next msg
sendingTime = 0;
j = NEXTMSG;
}
else if (sendingTime < 3)
{
sendi

Solution

This looks pretty good to me. I am not that good at threads myself, so I won't comment on the thread safety, but these are a few things I noticed.

This is fine as it is, but you might want to consider making this a ternary:

if (Services.Count != 0)
{
    SetNextSerAndSend();
}
else
{
    CheckAndSetServices();
}


Here it is in ternary form:

Services.Count == 0 ? CheckAndSetServices() : SetNextSerAndSend();


I'm not sure how this would affect the performance of your code, or whether it is important that the variable stay the same, but it is good to always keep your variables in as tight a scope as possible:

private void SendTask()
{
    SendMessage msg;
    while (true)
    {
        msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}


This could possibly become:

private void SendTask()
{
    while (true)
    {
        SendMessage msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}


The same principle can be applied to ReceiveTask.

Code Snippets

if (Services.Count != 0)
{
    SetNextSerAndSend();
}
else
{
    CheckAndSetServices();
}
Services.Count == 0 ? CheckAndSetServices() : SetNextSerAndSend();
private void SendTask()
{
    SendMessage msg;
    while (true)
    {
        msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}
private void SendTask()
{
    while (true)
    {
        SendMessage msg = MessageSendQueue.GetItem(-1);
        String rtrn = PushData(msg);
    }
}

Context

StackExchange Code Review Q#44146, answer score: 2

Revisions (0)

No revisions yet.