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.
https://github.com/RamblingCookieMonster/Invoke-Parallel/blob/master/Invoke-Parallel/Invoke-Parallel.ps1
. \$\endgroup\$