Search code examples
c#multithreadingtask-parallel-librarybackgroundworker

C# Background Workers - How many should I use simultaneously?


I'm writing an MVVM (Caliburn.Micro) application in C# which uses PowerShell to run WMI queries on remote computers. The computers are loaded from a selected Active Directory OU, so there could be any number of them. The results from the WMI queries will be displayed on the UI and I want to run multiple queries simultaneously and display each one as soon as its query has completed. I'm using multiple background workers to achieve this and at the moment it's working. However my current code will create one background worker for each computer in the OU without any form of queue or limit.

private void QueryComputers()
{
    foreach (RemoteComputer computer in Computers)
    {
        BackgroundWorker bw = new BackgroundWorker();
        bw.WorkerReportsProgress = true;
        bw.DoWork += BackgroundWorker_DoWork;
        bw.ProgressChanged += BackgroundWorker_ProgressChanged;
        bw.RunWorkerCompleted += BackgroundWorker_RunWorkerCompleted;
        bw.RunWorkerAsync(computer.DNSHostName);
    }

}

I imagine if there was enough computers in the selected OU that this could have a large performance impact. How many simultaneous background workers should I limit this to? Would you use a static number or base it on the number of CPU cores?

Also, how would you implement the queue for this? I thought about doing something like this:

private int bwCount = 0;
private int bwLimit = 5; // 10, 20, 200??

private void QueryComputers()
{
    int stopAt = lastIndex + (bwLimit - bwCount);
    if (stopAt > Computers.Count - 1) stopAt = Computers.Count - 1;
    if (stopAt > lastIndex)
    {
        for (int i = lastIndex; i <= lastIndex + (bwLimit - bwCount); i++) {
            BackgroundWorker bw = new BackgroundWorker();
            bw.WorkerReportsProgress = true;
            bw.DoWork += BackgroundWorker_DoWork;
            bw.ProgressChanged += BackgroundWorker_ProgressChanged;
            bw.RunWorkerCompleted += BackgroundWorker_RunWorkerCompleted;
            bw.RunWorkerAsync(Computers[i].DNSHostName);

            lastIndex = i;
            bwCount++;
        }
    }
}

private void BackgroundWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    // Handle Result etc...

    bwCount--;
    QueryComputers();
}

EDIT:

Attempt at using Task Parallel Library

I've taken one method from my application which retrieves the logged on user from the remote machine and attempted to use TPL instead of a background worker. The problem is it's not running asynchronously and the UI is hanging while it runs.

private void GetLoggedOnUsersTPL()
{
    Parallel.ForEach(Computers, (computer) =>
    {
        using (PowerShell ps = PowerShell.Create())
        {

            computer.Status = RemoteComputer.ComputerStatus.UpdatingStatus;

            // Ping the remote computer to check if it's available to connect to
            ps.AddScript($"Test-Connection -ComputerName {computer.DNSHostName} -Count 1 -Quiet");
            Collection<PSObject> psOutput = ps.Invoke();
            if ((bool)psOutput[0].BaseObject) // If the computer responded to the Ping
            {
                ps.Commands.Clear(); // Remove the Test-Connection (Ping) command

                // Use a WMI query to find out who is logged on to the remote computer
                ps.AddScript($"Get-CimInstance -ComputerName {computer.DNSHostName} -Class Win32_ComputerSystem -Property UserName");
                psOutput = ps.Invoke();

                if (psOutput.Count < 1) // If there are no results we will try using a DCOM connection instead of WSMAN
                {
                    ps.Commands.Clear();
                    ps.AddScript("$opt = New-CimSessionOption -Protocol DCOM");
                    ps.AddScript($"$cims = New-CimSession -ComputerName {computer.DNSHostName} -SessionOption $opt");
                    ps.AddScript($"Get-CimInstance -Class Win32_ComputerSystem -Property UserName -CimSession $cims");
                    psOutput = ps.Invoke();
                }

                if (psOutput.Count > 0) // Check if we had any results
                {
                    string userName = psOutput[0].Members["UserName"].Value.ToString();
                    if (userName == null || userName == "")
                    {
                        computer.LoggedOnUser = "Nobody is logged on...";
                        computer.Status = RemoteComputer.ComputerStatus.Online;
                    }
                    else
                    {
                        computer.LoggedOnUser = userName;
                        computer.Status = RemoteComputer.ComputerStatus.Online;

                    }
                }
                else
                {
                    computer.Status = RemoteComputer.ComputerStatus.Blocked;
                }

            }
            else
            { 
                computer.Status = RemoteComputer.ComputerStatus.Offline;
            }
        }
    });
}

I tried making the method async I.e. private async void GetLoggedOnUsersTPL() but that told me I need to use await, and I'm not sure where to use that in this example.

EDIT 2:

Second attempt at using Task Parallel Library

I'm now trying to use Task.Run instead of Parallel.ForEach which is working mostly. The tasks are executing and the UI is NOT hanging, but if I select a new OU from the TreeView before all of the tasks have finished executing the debugger breaks on the token.ThrowIfCancellationRequested(); lines, so they are not being caught. Is anybody able to point out what I've done wrong here please?

public override bool IsSelected // << AD OU IsSelected in TreeView
{
    get { return isSelected; }
    set
    {
        if (isSelected != value)
        {
            isSelected = value;

            if (getLoggedOnUsersTokenSource != null) // If any 'GetLoggedOnUsers' tasks are still running, cancel them
            {
                getLoggedOnUsersTokenSource.Cancel(); 
            }

            LoadComputers(); // Load computers from the selected OU
            GetLoggedOnUsersTPL();
        }
    }
}

private CancellationTokenSource getLoggedOnUsersTokenSource;
private async void GetLoggedOnUsersTPL()
{
    getLoggedOnUsersTokenSource = new CancellationTokenSource();
    CancellationToken token = getLoggedOnUsersTokenSource.Token;

    List<Task> taskList = new List<Task>();
    foreach (RemoteComputer computer in Computers)
    {
        taskList.Add(Task.Run(() => GetLoggedOnUsersTask(computer, token), token));

    }

    try
    {
        await Task.WhenAll(taskList);
    } catch (OperationCanceledException) // <<<< Not catching all cancelled exceptions
    {
        getLoggedOnUsersTokenSource.Dispose();
    }

}

private void GetLoggedOnUsersTask(RemoteComputer computer, CancellationToken token)
{
    using (PowerShell ps = PowerShell.Create())
    {
        if (token.IsCancellationRequested)
        {
            token.ThrowIfCancellationRequested();
        }

        // Ping remote computer to check if it's online

        if ((bool)psOutput[0].BaseObject) // If the computer responded to the Ping
        {
            if (token.IsCancellationRequested)
            {
                token.ThrowIfCancellationRequested();
            }

            // Run WMI query to get logged on user using WSMAN

            if (psOutput.Count < 1) // If there were no results, try DCOM
            {

                if (token.IsCancellationRequested)
                {
                    token.ThrowIfCancellationRequested();
                }

                // Run WMI query to get logged on user using DCOM

                // Process results
            }
        }
    }
}

Solution

  • I'm using multiple background workers to achieve this and at the moment it's working.

    BackgroundWorker is a rather outdated type that doesn't handle dynamic requirements well. Parallel is a better approach if your workload is synchronous (which it appears to be).

    The problem is it's not running asynchronously and the UI is hanging while it runs.

    The Parallel.ForEach is a great solution. To unblock the UI, just push it onto a thread pool thread. So this parallel method:

    private void GetLoggedOnUsersTPL()
    {
      Parallel.ForEach(Computers, (computer) =>
      {
        ...
      });
    }
    

    should be called as such:

    await Task.Run(() => GetLoggedOnUsersTPL());