6

I have a windows in which i have added an infinite while loop in the OnStart() method .I have tested the service for 1 hour and it is running fine.But as this is my first Windows Service so have doubt on the performance with infinite loop. Here is the code..

 protected override void OnStart(string[] args)
    {
       while(true){

        string Query="";

        Query = "SELECT * FROM 'reportsetting` order by SendingTime;";

        MySqlConnection con = new MySqlConnection(conn);
        MySqlCommand comm = new MySqlCommand(Query, con);
        con.Open();
        MySqlDataReader dr = comm.ExecuteReader();
        while (dr.Read())
        {

            time = dr["SendingTime"].ToString();

            if ((str = DateTime.Now.ToString("HH:mm")).Equals(time))
            {

                //Execute Function and send reports based on the data from the database.

                Thread thread = new Thread(sendReports);
                thread.Start();

            }


        }


            //Halt for this Moment

            while ((str = DateTime.Now.ToString("HH:mm")).Equals(time))
            {


            }


         }

        }

    public void sendReports() { 



    }

So want to know if it will be Ok for long run.Thanks..

user3924730
  • 163
  • 1
  • 3
  • 14
  • 3
    This sounds like an immensely bad idea but you haven't described the details enough. Are there any break conditions from the loop? – James Cross Aug 30 '14 at 08:45
  • @JamesCross As per the condition i have to continuously read from the database and later based on condition have to do some work in thread – user3924730 Aug 30 '14 at 08:47
  • @JamesCross I have updated my post with full code.Please have alook – user3924730 Aug 30 '14 at 08:49
  • 1
    That code is highly inefficient. But to solve your while(true) issue take a look at using timers. Get the system to call you back every minute or so when you want the query again: http://stackoverflow.com/questions/10206000/how-to-have-a-function-run-inside-a-service-every-10-minutes – James Cross Aug 30 '14 at 08:52
  • @JamesCross Actually as per my condition i have to query on every `40 seconds` and execute the codes thereaftre.. – user3924730 Aug 30 '14 at 08:53
  • Oh, and you'll want to dispose that db-related objects. And use UTC time instead of local if you are using it for timing (although you need a timer actually). This would probably be better suited for [Code Review](http://codereview.stackexchange.com/). – vgru Aug 30 '14 at 08:54

4 Answers4

4

You almost certainly don't want to be querying a DB in an infinite loop. What you probably want to do is use a Timer to query the DB every so often (e.g. every 30 seconds) and do some work if some condition is matched e.g.

private static Timer timer;
private const int TimerInterval = 30000;

protected override void OnStart(string[] args)
{
    var callback = new TimerCallback(checkDatabase);
    this.timer = new Timer(callback, null, 0, TimerInterval);
}

private void checkDatabase()
{
    string query = "SELECT * FROM 'reportsetting` order by SendingTime;";
    using (var con = new MySqlConnection(conn))
    using (var cmd = new MySqlCommand(query, con))
    {
        con.Open();
        using (var dr = cmd.ExecuteReader())
        {
            while (dr.Read())
            {
                // do some work
            }
        }
    }
}

Depending on how important the speed of updates are, you could also be smart about things and introduce an back-off strategy. For example, here's a very simple incremental back-off:

private const int DefaultInterval = 40000;
private int interval = DefaultInterval;
...

while (dr.Read())
{
    if (someCondition)
    {
        // do some work
        timer.Change(DefaultInterval, DefaultInterval); // revert to 40 seconds 
    }
    else 
    {
        // no change? increase polling by 10 seconds
        interval += 10000;
        timer.Change(interval, interval);
    }
}
James
  • 80,725
  • 18
  • 167
  • 237
  • 1
    That backoff is incremental, not exponential. – ssmith May 11 '17 at 18:44
  • @ssmith ha, indeed it is - it's kinda irrelevant tbh, was just trying to give an example (will update for correctness though). Thanks – James May 12 '17 at 12:19
4

To re-run the query every 40 seconds:

private const string Query = "SELECT * FROM 'reportsetting` order by SendingTime;"

protected override void OnStart(string[] args)
{
    _timer = new Timer(40 * 1000); // every 40 seconds
    _timer.Elapsed += new System.Timers.ElapsedEventHandler(timer_Elapsed);
    _timer.Start(); // <- important
}

private void timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
{
    MySqlConnection con = new MySqlConnection(conn);
    MySqlCommand comm = new MySqlCommand(Query, con);
    con.Open();
    MySqlDataReader dr = comm.ExecuteReader();
    while (dr.Read())
    {

        time = dr["SendingTime"].ToString();

        if ((str = DateTime.Now.ToString("HH:mm")).Equals(time))
        {

            //Execute Function and send reports based on the data from the database.

            Thread thread = new Thread(sendReports);
            thread.Start();
        }
    }
}

Something like that. As Groo mentioned though, you might want to dispose of the connection every time so you don't have that hanging around in memory.

James Cross
  • 7,799
  • 8
  • 24
  • 30
  • 1
    Yeah, just showing where 40 seconds comes into it. The compiler sorts it all out for us anyway. There is no inefficiency there. – James Cross Aug 30 '14 at 09:04
  • @JamesCross How much time i need to give to `1 minute`.I mean what value i have to add for .Is it.`60*1000`? – user3924730 Aug 30 '14 at 09:05
  • 2
    Yep exactly. And for 10 minutes go one more time `10 * 60 * 1000` – James Cross Aug 30 '14 at 09:09
  • @JamesCross Ok I have one more doubt as in my `while (dr.Read())` if the `if ((str = DateTime.Now.ToString("HH:mm")).Equals(time))` condition meets then will `Thread thread = new Thread(sendReports);` execute in background and next `time` value we will get or first the thread will execute then it will go for while(dr) loop – user3924730 Aug 30 '14 at 09:13
  • Yup the `sendReports` method will run in a new thread but the `while(dr.Read())` will still be running in the same thread as before. – James Cross Aug 30 '14 at 09:16
  • Instead of pulling all the records from database, why don't you filter all the unsent email and the datetime is in say `Query = "SELECT * FROM reportsetting where [your tag if already sent] = false and SendingTime <= [current date and time] order by SendingTime;";`. you may also used `Thread.Sleep(1000) //1000= 1 second` – Jade Aug 30 '14 at 09:19
  • @JamesCross It means that i will get the new `time` value from database while `sendReports()` will be running in the background..Am i right? – user3924730 Aug 30 '14 at 09:20
  • @Jade Is it that i will get the new time value from database while sendReports() will be running in the background..Am i right? – user3924730 Aug 30 '14 at 09:28
2

Or you can just start it in new thread without using timer

var threadStart = new ThreadStart(<your method delegate>);
var thread = new Thread(threadStart);
thread.Start();
Alexandr Sargsyan
  • 656
  • 1
  • 6
  • 21
0

Try this

bool _IsStop = false;

protected override void OnStart(string[] args)
{
        base.OnStart(args);

        while (!this._IsStop)
        {

            this.Process();

            //40 seconds time out
            Thread.Sleep(1000*40); //1000 is 1 second
        }

        //If we reach here
        //Do some clean up here

    }


    protected override void OnStop()
    {
        //Service is stop by the user on service or by OS
        this._IsStop = true;

    }


    private void Process()
    {
        string Query = "";

        // -->>>  if you use something like this on your query and little bit setup on your db
        // this will give you a more efficient memory usage
        Query = "SELECT * FROM reportsetting where [your tag if already sent] = false and SendingTime <= [current date and time]  order by SendingTime;";

        MySqlConnection con = new MySqlConnection(conn);
        MySqlCommand comm = new MySqlCommand(Query, con);
        con.Open();
        MySqlDataReader dr = comm.ExecuteReader();
        while (dr.Read())
        {
                //Execute Function and send reports based on the data from the database.

                Thread thread = new Thread(sendReports);
                thread.Start();

                //sleep for half a second before proceeding to the next record if any
                Thread.Sleep(500); 
        }
    }
Jade
  • 2,972
  • 1
  • 11
  • 9
  • Is it that i will get the new time value from database while sendReports() will be running in the background..Am i right? – user3924730 Aug 30 '14 at 09:30
  • Once the the cycle for querying is executed the you can pass the date as your criteria which is computed from c# or in the database (sorry but i'm not familiar in MySQL dateAdd function). – Jade Aug 30 '14 at 09:41
  • Yes it will, since you spawn threads per record for sendReports – Jade Aug 30 '14 at 09:47
  • Am sorry but i am not getting you.I have doubt that will my function `sendReport()` will execute in the background as thread (or whatever i am not familiar of these Am sorry) and i will move to the `While(dr.Read())` and will get new `time` value and subsequently new thread will get started – user3924730 Aug 30 '14 at 09:51
  • the present case will run in background since you create a thread per iteration of record, meaning they will work independently until the process is done. – Jade Aug 30 '14 at 09:52
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/60286/discussion-between-user3924730-and-jade). – user3924730 Aug 30 '14 at 09:53