23
\$\begingroup\$

I came up with the below code to improve the peformance of pinging a large number of machines. At present it's fairly basic, but thought I should see what people thought before proceeding further.

NB: This is my first time playing with workflows, so I've probably committed a few faux-pas there.

cls
workflow Test-ConnectionQuickly {
    [cmdletbinding()]
    param(
        [Parameter(Mandatory = $true)]
        [string[]]$computers
    )
    $computerGroups = InlineScript {
        #group computers to throttle the number of threads
        [psobject]$itemCounter = [pscustomobject]@{itemNo=0;groupSize=10} #alter groupSize per your preference
        $itemCounter | Add-Member -MemberType ScriptMethod -Name 'groupNo' -Value {
            [math]::Floor($this.itemNo++ / $this.groupSize)
        }
        write-output $using:computers | Group-Object -Property {$itemCounter.groupNo()}
    }
    ForEach -parallel ($computerGroup in $computerGroups) {
        $computerGroup | select -ExpandProperty group | %{
            $pingable = (Test-Connection $_ -Count 2 -Quiet -ErrorAction SilentlyContinue)
            write-output (new-object -type psobject -property @{Name=$_;Online=$pingable})
        }
    } 
}

[string[]]$computers = (1..100 | %{("Server{0:000}" -f $_)}) #here's where we'd read in the computere from file
Test-ConnectionQuickly $computers | select Name, Online #here's where we'd pipe the output to file

Design Notes:

  • The InlineScript is used to group servers into small sets (of 10; an arbitrary size) in the hope of balancing serial's performance for small sets against parallel's performance for large sets of computers.
  • The $itemCounter variable's an attempt at making grouping into defined set sizes simpler to read
  • -count 2 is specified on the test-connection so that we have some tolerence for network glitches without too much affect on performance.
  • -Quiet is specified because we don't need output / and not having output will help performance.
\$\endgroup\$
  • 3
    \$\begingroup\$ Have you had a look at ThrottleLimit? Eg: ForEach -Parallel -ThrottleLimit 10 (x In y) {...} \$\endgroup\$ – xXhRQ8sD2L7Z Sep 1 '15 at 14:04
  • \$\begingroup\$ I hadn't been aware; will look into it; thank-you. \$\endgroup\$ – JohnLBevan Sep 1 '15 at 18:19
  • 1
    \$\begingroup\$ I was looking for a fast ping sweeper and came across this question. I settled on this script: gallery.technet.microsoft.com/scriptcenter/… \$\endgroup\$ – YetAnotherRandomUser Jul 29 '17 at 13:57
7
+150
\$\begingroup\$

Want to start of with I am new to workflows as well. For the most part your code is solid. You should see a couple of suggestions that would actually improve performance especially if you are running this against 100's of computers.

Grouping logic

This is more of a compliment. I thought, from what I read, that -ThrottleLimit would be a simple answer to replace your grouping logic. Likely from my inexperience but most of my testing shows that what you have there outperforms some simple tests that use just -ThrottleLimit. I know that it does have its place but here I think what you have is actually better.

Use a parameter for grouping/ throttling

You have a necessary property for $computers but I think it would make just as much sense to have a parameter in the workflow for GroupSize as well.

Use of ScriptMethod

While this is a neat way to get the computers group together I think it is overkill for a simple array of computers. You can avoid group-object all together really since you already know how to group the computers. Instead consider just using a simple foreach-object loop that extracts the chunks of computers each as there own array. In practice there is no performance gain here. It is easier to understand if it was simple array splitting.

Many different ways of splitting the array into smaller chunks as well. I was trying to avoid a simple counter that reset when it reached the $groupsize

As an aside I'm am disappointed that you still need Add-Member if you want to make something other than a note property easily. I read about another approach here but did not think it was useful enough to suggest in your case.

New-Object and [pscustomobject]

You use both approaches for creating object. Might as well just stick to one. When I changed your final object creation to just a [pscustomobject] cast on the hashtable it actually improved performance!

Testing the connection twice

I like the idea of hardening the test. It is possible that a computer might not respond to the first ping and could be recorded as down when it is indeed not. So I am fine with doing the test twice.

However when you are testing a computer that is up you would ping it twice. Would be better to do two separate tests and do it in such a way that the first, if successful, will prevent checking again as it would be redundant.

Consider the following very simple If statement.

if(1 -eq 1 -or 1/0 ){"Yo"}

At first glance you should see the divide by 0 for the second condition and expect failure. That will never be checked since 1 -eq 1 will always be true. Since the first condition is true then PowerShell will ignore the right hand condition as it would not change the outcome. This concept is covered under about_logical_operators in the last paragraph.

So I propose something like this:

$pingable = (Test-Connection $_ -Count 1 -Quiet -ErrorAction SilentlyContinue) -or 
                        (Test-Connection $_ -Count 1 -Quiet -ErrorAction SilentlyContinue)

Same command is called twice but only if the first one fails. So it is still hardened but should be efficient as well. I suppose you could splat the commands since they are the same but I was trying to keep extra logic out of the loop for performance.


You by no means need to use this code but I implore you to try both yours against mine to see if there is at least a performance improvement.

workflow Test-ConnectionQuickly {
    [cmdletbinding()]
    param(
        [Parameter(Mandatory = $true)]
        [string[]]$computers,
        [Parameter(Mandatory = $false)]
        [int]$GroupSize = 10
    )
    $computerGroups = InlineScript {
        #group computers to throttle the number of threads
        $numberofGroups = [math]::Ceiling($using:Computers.Count / $using:groupSize)
        1..$numberofGroups | ForEach-Object{
            # Arrays are zero based which is why you will see -1 
            ,($using:computers | Select -Index ((($_ - 1) * $using:GroupSize)..($_ * $using:GroupSize - 1)))
            # Use the unary operator to ensure an array of computers is returned and not unrolled. 
    }

    }
    ForEach -parallel ($computerGroup in $computerGroups) {
        $computerGroup | ForEach-Object{
            $pingable = (Test-Connection $_ -Count 1 -Quiet -ErrorAction SilentlyContinue) -or 
                        (Test-Connection $_ -Count 1 -Quiet -ErrorAction SilentlyContinue)
            [pscustomobject]@{Name=$_;Online=$pingable}
        }
    } 
} 

The only points I would recommend, if nothing else, is [pscustomobject]@{Name=$_;Online=$pingable} and how I broke up the ping test into 2 separate tests.

Most of my testing was done with a $GroupSize of 5 and 80 computers.

Measure-Command -Expression {Test-ConnectionQuickly $computers -GroupSize 5}

You definitely need to experiment with $GroupSize to find that sweet spot.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.