views:

127

answers:

7

Hey,

I created a windows service, that is supposed to check a certain table in the db for new rows every 60 seconds. For every new row that was added, I need to do some heavy processing on the server that could sometimes take more than 60 seconds.

I created a Timer object in my service, that ticks every 60 seconds and invokes the wanted method.
Since I don't want this timer to tick while processing the new lines found, I wrapped the method in a lock { } block, so this won't be accessible by another thread.

It looks something like this :

Timer serviceTimer = new Timer();
serviceTimer.Interval = 60;
serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer_Elapsed);
serviceTimer.Start();

void serviceTimer_Elapsed(object sender, ElapsedEventArgs e)
{
    lock (this)
    {
        // do some heavy processing...
    }
}

Now, I'm wondering -
If my timer ticks, and finds a lot of new rows on the db, and now the processing will take more than 60 seconds, the next tick won't do any processing till the previous one finished. This is the effect I want.

But now, will the serviceTimer_Elapsed method go off immediatly once the first processing was finished, or will it wait for the timer to tick again.

What I want to happen is - if the processing requires more than 60 seconds, than the timer will notice the thread is locked, and wait another 60 seconds to check again so I will never get stuck in a situation where there are a queue of threads waiting for the previous one to finish.

How can i accomplish this result ?
What is the best practice for doing this ?

Thanks!

+12  A: 

You might try disabling the timer during processing, something like

try
{
  serviceTimer.Stop(); 

  lock (this) 
    { 
        // do some heavy processing... 
    } 
}
finally
{
  serviceTimer.Start(); 
}
nonnb
Don't use `lock (this)` - http://stackoverflow.com/questions/251391/why-is-lockthis-bad
Tim Robinson
+3  A: 

Put a quick check it see if the service is running. if it is running it will skip this event and wait for the next one to fire.

Timer serviceTimer = new Timer();
serviceTimer.Interval = 60;
serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer_Elapsed);
serviceTimer.Start();
bool isRunning = false;
void serviceTimer_Elapsed(object sender, ElapsedEventArgs e)
{
    lock (this)
    {
        if(isRunning)
            return;
        isRunning = true;
    }
    try
    {
    // do some heavy processing...
    }
    finally
    {
        isRunning = false;
    }
}
Scott Chamberlain
I like nonnb's solution more than mine but I will leave mine up to provide a example when you do not have the ability to stop the event from firing.
Scott Chamberlain
+2  A: 

Other options might be to use a BackGroundWorker class, or TheadPool.QueueUserWorkItem.

Background worker would easily give you the option check for current processing still occurring and process 1 item at a time. The ThreadPool will give you the ability to continue queueing items every tick (if necessary) to background threads.

From your description, I assume you are checking for items in a queue in a database. In this case, I would use the ThreadPool to push the work to the background, and not slow/stop your checking mechanism.

For a Service, I would really suggest you look at using the ThreadPool approach. This way, you can check for new items every 60 seconds with your timer, then Queue them up, and let .Net figure out how much to allocate to each item, and just keep pushing the items into the queue.

For Example: If you just use a timer and you have 5 new rows, which require 65 seconds of processing time total. Using the ThreadPool approach, this would be done in 65 seconds, with 5 background work items. Using the Timer approach, this will take 4+ minutes (the minute you will wait between each row), plus this may cause a back-log of other work that is queueing up.

Here is an example of how this should be done:

Timer serviceTimer = new Timer();
    void startTimer()
    {
        serviceTimer.Interval = 60;
        serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer_Elapsed);
        serviceTimer.AutoReset = false;
        serviceTimer.Start();
    }
    void serviceTimer_Elapsed(object sender, ElapsedEventArgs e)
    {
        try
        {
            // Get your rows of queued work requests

            // Now Push Each Row to Background Thread Processing
            foreach (Row aRow in RowsOfRequests)
            {
                ThreadPool.QueueUserWorkItem(
                    new WaitCallback(longWorkingCode), 
                    aRow);
            }
        }
        finally
        {
            // Wait Another 60 Seconds and check again
            serviceTimer.Stop();
        }
    }

    void longWorkingCode(object workObject)
    {
        Row workRow = workObject as Row;
        if (workRow == null)
            return;

        // Do your Long work here on workRow
    }
Ryan from Denver
+2  A: 

You don't need the lock in this case. Set timer.AutoReset=false before starting it. Restart the timer in the handler after you are done with your processing. This will ensure that the timer fires 60 seconds after each task.

Ani
+1 I never knew there was a property called Autoreset
Searock
+3  A: 

I recommend you don't let the timer tick at all while its processing.

Set the Timers AutoReset to false. And start it at the end. Here's a full answer you might be interested in http://stackoverflow.com/questions/3266420/needed-a-windows-service-that-executes-jobs-from-a-job-queue-in-a-db-wanted-ex/3266671#3266671

Conrad Frix
A: 

another posibility would be something like this:

void serviceTimer_Elapsed(object sender, ElapsedEventArgs e)
{   
    if (System.Threading.Monitor.IsLocked(yourLockingObject))
       return;
    else
       lock (yourLockingObject)
       // your logic  
           ;
}
Chris Ortiz
+1  A: 

A similar variation on other answers, that allows the timer to keep ticking and only do the work when the lock can be obtained, instead of stopping the timer.

Put this in the elapsed event handler:

if (Monitor.TryEnter(locker)
{
    try
    {
        // Do your work here.
    }
    finally
    {
        Monitor.Exit(locker);
    }
}
Andy
This is the approach I use as well. The elapsed event handler just falls through and the timer can be left alone.
ebpower