2

I am trying to construct a simple class, which calls a reboot function depending on the machine type to be rebooted. The called methods refer to a library which contains public static methods. I want to asynchronously call these static methods using Task in order to call the reboot methods in parallel. Here is the code so far:

EDIT Following the community's request, this is now a version of the same question, with the code below compiling. Please not that you need the Renci.SshNet lib, and also need to set references to it in your project.

// libs
using System.IO;
using System.Threading.Tasks;
using System.Collections.Generic;

using Renci.SshNet;



namespace ConsoleApp
{
    class Program
    {


        // Simple Host class
        public class CHost
        {
            public string IP;
            public string HostType;

            public CHost(string inType, string inIP)
            {// constructor
                this.IP         = inIP;
                this.HostType   = inType;
            }
        }



        // Call test function
        static void Main(string[] args)
        {

            // Create a set of hosts
            var HostList = new List<CHost>();
            HostList.Add( new CHost("Machine1", "10.52.0.93"));
            HostList.Add( new CHost("Machine1", "10.52.0.30"));
            HostList.Add( new CHost("Machine2", "10.52.0.34"));


            // Call async host reboot call
            RebootMachines(HostList);
        }




        // Reboot method
        public static async void RebootMachines(List<CHost> iHosts)
        {
            // Locals
            var tasks = new List<Task>();


            // Build list of Reboot calls - as a List of Tasks
            foreach(var host in iHosts)
            {

                if (host.HostType == "Machine1")
                {// machine type 1
                    var task = CallRestartMachine1(host.IP);
                    tasks.Add(task);    // Add task to task list
                }
                else if (host.HostType == "Machine2")
                {// machine type 2
                    var task = CallRestartMachine2(host.IP);
                    tasks.Add(task);    // Add task to task list
                }   
            }


            // Run all tasks in task list in parallel
            await Task.WhenAll(tasks);
        }



        // ASYNC METHODS until here
        private static async Task CallRestartMachine1(string host)
        {// helper method: reboot machines of type 1

            // The compiler complains here (RebootByWritingAFile is a static method)
            // Error: "This methods lacks await operators and will run synchronously..."
            RebootByWritingAFile(@"D:\RebootMe.bm","reboot");

        }
        private static async Task CallRestartMachine2(string host)
        {// helper method: reboot machines of type 2

            // The compiler warns here (RebootByWritingAFile is a static method)
            // Error: "This methods lacks await operators and will run synchronously..."
            RebootByNetwork(host,"user","pwd");

        }




        // STATIC METHODS here, going forward
        private static void RebootByWritingAFile(string inPath, string inText)
        {// This method does a lot of checks using more static methods, but then only writes a file


            try
            {
                File.WriteAllText(inPath, inText); // static m
            }
            catch
            {
                // do nothing for now
            }
        }
        private static void RebootByNetwork(string host, string user, string pass)
        {
            // Locals
            string rawASIC = "";
            SshClient SSHclient;
            SshCommand SSHcmd;


            // Send reboot command to linux machine
            try
            {
                SSHclient = new SshClient(host, 22, user, pass);
                SSHclient.Connect();
                SSHcmd = SSHclient.RunCommand("exec /sbin/reboot");
                rawASIC = SSHcmd.Result.ToString();
                SSHclient.Disconnect();
                SSHclient.Dispose();
            }
            catch
            {
                // do nothing for now
            }
        }




    }
}

My only problem with this setup so far is that the static methods are called immediately (sequentially) and not assigned to a task. For example the line

        ...
        else if (host.HostType == "Machine2")
        {// machine type 2
            var task = CallRestartMachine2(host.IP);
            tasks.Add(task);    // Add task to task list
        }  
        ...

takes 20 seconds to execute if the host is unreachable. If 10 hosts are unreachable the sequential duration is 20*10 = 200 seconds.

I am aware of some seemingly similar questions such as

However, the cited lambda expressions still leave me with the same compiler error ["This methods lacks await operators..."]. Also, I do not want to spawn explicit threads (new Thread(() => ...)) due to high overhead if restarting a large number of machine in a cluster.

I may need to reboot a large number of machines in a cluster. Hence my question: How can I change my construct in order to be able to call the above static methods in parallel?

EDIT Thanks to the comments of @JohanP and @MickyD, I would like to elaborate that I have actually tried writing the async version of both static methods. However that sends me down a rabbit hole, where every time a static method is called within the async method I get the compiler warning that the call will be synchronous. Here is an example of how I tried to wrap the call to method as an async task, hoping to call the dependent methods in an async manner.

private static async Task CallRestartMachine1(string host)
{// helper method: reboot machines of type 1

    // in this version, compiler underlines '=>' and states that 
    // method is still called synchronously
    var test = await Task.Run(async () =>
    {
        RebootByWritingAFile(host);
    });

}

Is there a way to wrap the static method call such that all static child methods don't all need to rewritten as async?

Thank you all in advance.

supersausage007
  • 93
  • 1
  • 10
  • How can you run several methods in parallel? By running them in different threads. Don't like threads? Re-write the methods to be `async` and await them. – Joker_vD Oct 25 '18 at 22:21
  • 1
    This code wouldn't even compile so it's hard for us to help you since this is clearly not what your code looks like – Dave Oct 25 '18 at 22:30
  • @Dave, the code now compiles once the [Renci.SHH](https://github.com/sshnet/SSH.NET) lib is installed and referenced. – supersausage007 Oct 26 '18 at 14:15

3 Answers3

0

Your code has a strange mix of async with continuations and it won't even compile. You need to make it async all the way up. When you call RebootMachines(...) and that call can't be awaited, you can schedule continuations on that i.e. RebootMachines(...).ContinueWith(t=> Console.WriteLine('All Done'))

public static async Task RebootMachines(List<CHost> iHosts)
{
    var tasks = new List<Task>();

    // Build list of Reboot calls - as a List of Tasks
    foreach(var host in iHosts)
    {
        if (host.HostType == "Machine1")
        {// machine type 1
             task = CallRestartMachine1(host.IP);
        }
        else if (host.HostType == "Machine2")
        {// machine type 2
            task = CallRestartMachine2(host.IP);
        }

         // Add task to task list - for subsequent parallel processing
         tasks.Add(task);
    }


    // Run all tasks in task list in parallel
    await Task.WhenAll(tasks);
}

private static async Task CallRestartMachine1(string host)
{// helper method: reboot machines of type 1

    //RebootByWritingAFile is method that returns a Task, you need to await it
    // that is why the compiler is warning you
    await RebootByWritingAFile(host);

}

private static async Task CallRestartMachine2(string host)
{// helper method: reboot machines of type 2

    await RebootByNetwork(host);

}
JohanP
  • 5,252
  • 2
  • 24
  • 34
  • 1
    Good answer. The only thing I would add is that now the methods are async (or you have made additional async methods) it is generally a best practice to rename them with an `Async` suffix. https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/ –  Oct 25 '18 at 23:28
  • Thank you everyone for your input. I edited my question to show what I have tried additionally to the simple code construct. Essentially, If I called the methods on a new thread for each invocation, I would end up with a quasi-async execution but with explicit thread switching overhead. – supersausage007 Oct 26 '18 at 13:12
  • @JohanP, the code now compiles once the Renci.SHH lib is installed and referenced. – supersausage007 Oct 26 '18 at 14:17
0

Everyone, thanks for your input and your help. I have played around with his over the last couple of days, and have come up with the following way to asynchronously call a static method:

// libs
using System.IO;
using System.Threading.Tasks;
using System.Collections.Generic;

using Renci.SshNet;



namespace ConsoleApp
{
    class Program
    {


        // Simple Host class
        public class CHost
        {
            public string IP;
            public string HostType;

            public CHost(string inType, string inIP)
            {// constructor
                this.IP         = inIP;
                this.HostType   = inType;
            }
        }



        // Call test function
        static void Main(string[] args)
        {

            // Create a set of hosts
            var HostList = new List<CHost>();
            HostList.Add( new CHost("Machine1", "10.52.0.93"));
            HostList.Add( new CHost("Machine1", "10.52.0.30"));
            HostList.Add( new CHost("Machine2", "10.52.0.34"));


            // Call async host reboot call
            RebootMachines(HostList);
        }




        // Reboot method
        public static async void RebootMachines(List<CHost> iHosts)
        {
            // Locals
            var tasks = new List<Task>();


            // Build list of Reboot calls - as a List of Tasks
            foreach(var host in iHosts)
            {

                if (host.HostType == "Machine1")
                {// machine type 1
                     var task = CallRestartMachine1(host.IP);
                    tasks.Add(task);    // Add task to task list
                }
                else if (host.HostType == "Machine2")
                {// machine type 2
                    var task = CallRestartMachine2(host.IP);
                    tasks.Add(task);    // Add task to task list
                }   
            }


            // Run all tasks in task list in parallel
            await Task.WhenAll(tasks);
        }



        // ASYNC METHODS until here
        private static async Task CallRestartMachine1(string host)
        {// helper method: reboot machines of type 1

            await Task.Run(() =>
            {
                RebootByWritingAFile(@"D:\RebootMe.bm", "reboot");
            });

        }
        private static async Task CallRestartMachine2(string host)
        {// helper method: reboot machines of type 2

            await Task.Run(() =>
            {
                RebootByNetwork(host, "user", "pwd");
            });

        }




        // STATIC METHODS here, going forward
        private static void RebootByWritingAFile(string inPath, string inText)
        {// This method does a lot of checks using more static methods, but then only writes a file


            try
            {
                File.WriteAllText(inPath, inText); // static m
            }
            catch
            {
                // do nothing for now
            }
        }
        private static void RebootByNetwork(string host, string user, string pass)
        {
            // Locals
            string rawASIC = "";
            SshClient SSHclient;
            SshCommand SSHcmd;


            // Send reboot command to linux machine
            try
            {
                SSHclient = new SshClient(host, 22, user, pass);
                SSHclient.Connect();
                SSHcmd = SSHclient.RunCommand("exec /sbin/reboot");
                rawASIC = SSHcmd.Result.ToString();
                SSHclient.Disconnect();
                SSHclient.Dispose();
            }
            catch
            {
                // do nothing for now
            }
        }




    }
}

This setup calls my static methods asynchronously. I hope this helps someone who was stuck with a similar problem. Thanks again for all your input.

supersausage007
  • 93
  • 1
  • 10
-1

I think you should reconsider the "high overhead" of threads: This overhead is IMHO negligable when compared to the network roundtrips and waiting times on every RPC call. I am pretty sure, that the resources needed to create N threads is marginal against the resources needed to run N networked requests (be they http[s], RPC or whatever).

My approach would be to queue the tasks into a threadsafe collection (ConcurrentQueue, ConcurrentBag and friends), then spawn a finite number of threads looping through those work items until the collection is empty and just end.

This would not only allow you to run the tasks in parallel, it would allow you to run them in a controlled parallel way.

Eugen Rieck
  • 64,175
  • 10
  • 70
  • 92
  • _"This overhead is IMHO negligable "_ - OP is talking about `new Thread` as opposed to thread pool thread. There is a measureable cost of spawning explicit threads, about [1MB per thread](https://stackoverflow.com/questions/2744442/whats-the-memory-overhead-for-a-single-windows-threads). –  Oct 25 '18 at 23:42
  • Also, these threads *are doing nothing useful*, whether you use explicit threads or thread pool threads. So in addition to the fact that the overhead is *not* in fact negligible, they're still not accomplishing anything. – Servy Oct 26 '18 at 13:18
  • @Eugen Rieck, thanks for your input. I would use this approach as a last resort. However, general guidelines advise against using more than 50 threads per CPU core due to amassing too much thread switching overhead. I have another method, where I scan tens of thousands of IP addresses on our local server network within less than 5 seconds, all in one async call. This is what I am trying to achieve, here but calling the lib static methods continues to force sync execution only. – supersausage007 Oct 26 '18 at 13:23
  • Another good distinction why I should not use multiple threads but instead multitasking is explained nicely [here](https://stackoverflow.com/questions/34680985/what-is-the-difference-between-asynchronous-programming-and-multithreading) – supersausage007 Oct 26 '18 at 14:29
  • @MickyD 1MB per thread is negligable, if you compare it to the 12MB needed for an RPC call over SMB. – Eugen Rieck Oct 26 '18 at 21:49
  • @Servy If they do nothing usefull, then they are never scheduled, so use no CPU cycles. The C# runtime (i.e. dotnet and needed libs) allone need over 100MB, of which ca. 90MB do "nothing usefull" – Eugen Rieck Oct 26 '18 at 21:51
  • @supersausage007 multitasking can be achieved by many means, one of them being threads. `async` e.g. uses nothing else then ... thread pool threads. – Eugen Rieck Oct 26 '18 at 21:52
  • @EugenRieck Threads have more of an overhead than just their stack memory. There is also the time spent context switching between threads, for example. Additionally while the cost of *one* thread might not be unreasonably high, when you're constantly creating new threads to have them do nothing every time you want to do something asynchronously it adds up, fairly quickly. If you actually had some benefit out of using threads, these costs could be justified, but you're not using them to do anything productive. – Servy Oct 26 '18 at 21:56
  • @Servy If a thread has nothing to do, it will not be scheduled, so no context switch happens. I recommend you read up on how thread pools actually work! – Eugen Rieck Oct 27 '18 at 10:25
  • @EugenRieck When you're allocating a thread to have it do nothing but wait for an IO operation it will need to be scheduled for a short period of time to start the asynchronous operation and to complete it, both of which involve context switches. Additionally, if you're constantly blocking lots of thread pool threads you can end up starving the thread pool of workers, even when no actual work is being done by them. I recommend you read up on how thread pools actually work! – Servy Oct 29 '18 at 13:20
  • @Servy When scheduling a thread to start and finish an IO, it does something very useful. You really should read up on how thread pools actually work! – Eugen Rieck Oct 29 '18 at 13:34
  • @EugenRieck It does something useful *that doesn't require creating an entire thread* (or consuming a thread pool thread) to accomplish, and that can much more effectively be accomplished by just not using any new threads at all. You really should read up on how thread pools actually work! – Servy Oct 29 '18 at 13:34
  • Using a thread pool thread accomplishes exactly that. SInce you are ignorant on how thread pools work and not willing to learn, this discussion is fruitless and so ends here. – Eugen Rieck Oct 29 '18 at 13:35