0

I have three different job class for different work, each of them needs to run in the background in a specified interval.

Is the following code is good in performance and clean in code style?

if not, how to fix it? I am new to c#, my dev environment is net core 3.1

class Program  
{  
    static void Main(string[] args)  
    {  
        Method1();  
        Method2();  
        Console.ReadKey();  
    }  

    public static async Task Method1()  
    {  
        await Task.Run(() =>  
        {  
            var run = true;
            for (run)  
            {   
                var c = await fetchConfigFromDBAsync()
                run = c.run;
                var interval = c.interval
                await JobClass1.RunAsync() 
                Thread.Sleep(interval)
            }  
        });  
    }  


    public static async Task Method2()  
    {  
        await Task.Run(() =>  
        {  
            for (true)  
            {  
                await JobClass2.RunAsync() 
                Thread.Sleep(new TimeSpan(0, 10, 4))
            }  
        });  
    }  
    public static async Task Method3()  
    {  
        await Task.Run(() =>  
        {  
            for (true)  
            {  
                await JobClass3.RunAsync() 
                Thread.Sleep(new TimeSpan(1, 0, 3))
            }  
        });  
    } 
}

Panic
  • 325
  • 2
  • 10

1 Answers1

2

You must avoid Thread.Sleep for theses reasons. You can use Task.Delay() instead.

Then, you don't need to encapsulate your loops into Task.Run() if your async call is really an awaitable function, like sending data, commucicating with database, etc.. If your async function is more like heavy calculations, you can keep it. More explainations here.

And then, it's cleaner to wait all the infinites tasks rather than use Console.ReadLine()

static async Task Main(string[] args)
{
    Task m1 = Method1();
    Task m2 = Method2();

    await Task.WhenAll(new[] { m1, m2 });
}

public static async Task Method2()
{
    while (true)
    {
        await JobClass.RunAsync();
        await Task.Delay(3);
    }
}

public static async Task Method1()
{
    while (true)
    {
        await JobClass.RunAsync();
        await Task.Delay(3);
    }
}
Thibaut
  • 342
  • 1
  • 7
  • You are welcome, if the response is good enough for you, you can accept it. (you can also read theses rules: https://stackoverflow.com/help/someone-answers). Thanks! – Thibaut Jun 12 '20 at 13:37
  • 1
    Wrapping the loop in `await Task.Run(async () =>` doesn't hurt, and it's actually [advantageous](https://stackoverflow.com/questions/38739403/await-task-run-vs-await-c-sharp/58306020#58306020), if your app is WinForms or WPF and you are going to launch these tasks from the UI thread. Also there is no mechanism in place for the graceful completion of the `m1` and `m2` tasks, so they'll never complete unless they fail. Why not adding a `CancellationTokenSource` to the mix, so that their lifetime can be controlled? – Theodor Zoulias Jun 12 '20 at 13:59
  • @TheodorZoulias thanks, I am writing a console app that runs silently, and CancellationTokenSource is an advanced usage, may use it when I want to receive a remote call to close. currently, I get config from DB to control interval time and start-stop each time before await JobClass.RunAsync() begins – Panic Jun 12 '20 at 14:07
  • @PageLarry how are you closing the console app? By just clicking the close button? Are you OK with the possibility of your jobs been silently aborted at some random point during their execution? – Theodor Zoulias Jun 12 '20 at 14:13
  • 1
    @TheodorZoulias actually do by kill PID, perhaps I need to do catch kill signal and set all CancellationTokenSource to all async methods, this seems more responsibly, but maybe left it to a later time – Panic Jun 12 '20 at 14:32
  • @TheodorZoulias and maybe need implement send email logic when catch some random point – Panic Jun 12 '20 at 14:34
  • @PageLarry you could consider handling the [`Console.CancelKeyPress`](https://learn.microsoft.com/en-us/dotnet/api/system.console.cancelkeypress) event, canceling there the `CancellationTokenSource`, and waiting the tasks to complete (which will happen instantaneously most of the time). Then the hard part will be to remember to close the console app with Ctrl+C or Ctrl+Break instead of killing the process. :-) – Theodor Zoulias Jun 12 '20 at 14:56
  • 1
    @TheodorZoulias, yes, you are right, Most time I run the program with `nohup xxx &>/dev/null &` , and read the log in program dir. keep app foreground waiting for ctrl-C or ctrl-Break is not friendly to jumpserver user – Panic Jun 12 '20 at 15:09