How would you change my Heartbeat process written in C#?

Question

I'm looking at implementing a "Heartbeat" process to do a lot of repeated cleanup tasks throughout the day.

This seemed like a good chance to use the Command pattern, so I have an interface that looks like:

   public interface ICommand
   {
       void Execute();
       bool IsReady();
   }

I've then created several tasks that I want to be run. Here is a basic example:

public class ProcessFilesCommand : ICommand
{
    private int secondsDelay;
    private DateTime? lastRunTime;

    public ProcessFilesCommand(int secondsDelay)
    {
        this.secondsDelay = secondsDelay;
    }

    public void Execute()
    {
        Console.WriteLine("Processing Pending Files...");
        Thread.Sleep(5000); // Simulate long running task
        lastRunTime = DateTime.Now;
    }

    public bool IsReady()
    {
        if (lastRunTime == null) return true;

        TimeSpan timeSinceLastRun = DateTime.Now.Subtract(lastRunTime.Value);
        return (timeSinceLastRun.TotalSeconds > secondsDelay);
    }

}

Finally, my console application runs in this loop looking for waiting tasks to add to the ThreadPool:

class Program
{
    static void Main(string[] args)
    {

        bool running = true;

        Queue<ICommand> taskList = new Queue<ICommand>();
        taskList.Enqueue(new ProcessFilesCommand(60)); // 1 minute interval
        taskList.Enqueue(new DeleteOrphanedFilesCommand(300)); // 5 minute interval

        while (running)
        {
            ICommand currentTask = taskList.Dequeue();
            if (currentTask.IsReady())
            {
                ThreadPool.QueueUserWorkItem(t => currentTask.Execute());
            }
            taskList.Enqueue(currentTask);
            Thread.Sleep(100);
        }

    }
}

I don't have much experience with multi-threading beyond some work I did in Operating Systems class. However, as far as I can tell none of my threads are accessing any shared state so they should be fine.

Does this seem like an "OK" design for what I want to do? Is there anything you would change?

Answer 1

This is a great start. We've done a bunch of things like this recently so I can offer a few suggestions.

1. Don't use thread pool for long running tasks. The thread pool is designed to run lots of tiny little tasks. If you're doing long running tasks, use a separate thread. If you starve the thread pool (use up all the tasks), everything that gets queued up just waits for a threadpool thread to become available, significantly impacting the effective performance of the threadpool.

2. Have the Main() routine keep track of when things ran and how long till each runs next. Instead of each command saying "yes I'm ready" or "no I'm not" which will be the same for each command, just have LastRun and Interval fields which Main() can then use to determine when each command needs to run.

3. Don't use a Queue. While it may seem like a Queue type operation, since each command has it's own interval, it's really not a normal Queue. Instead put all the commands in a List and then sort the list by shortest time to next run. Sleep the thread until the first command is needed to run. Run that command. Resort the list by next command to run. Sleep. Repeat.

4. Don't use multiple threads. If each command's interval is a minute or few minutes, you probably don't need to use threads at all. You can simplify by doing everything on the same thread.

5. Error handling. This kind of thing needs extensive error handling to make sure a problem in one command doesn't make the whole loop fail, and so you can debug a problem when it occurs. You also may want to decide if a command should get immediately retried on error or wait until it's next scheduled run, or even delay it more than normal. You may also want to not log an error in a command if the error happens every time (an error in a command that runs often can easily create huge log files).