proxb / PoshRSJob

Provides an alternative to PSjobs with greater performance and less overhead to run commands in the background, freeing up the console and allowing throttling on the jobs.
MIT License
542 stars 87 forks source link

Feature Request: Add RunspaceID handling to Start-RSJob for better throttling support #75

Closed MVKozlov closed 8 years ago

MVKozlov commented 8 years ago

for now 1..25 | Start-RSJob -throttle 5 can work as expected and 1..25 | foreach { $_ | Start-RSJob -throttle 5 } is not because latest creates it own RunspaceID for each new job

Will be better if Start-RSJob can have a possibility to add new job to already existing RunSpacePool may be with -AddToExistingRunspace switch or -RunspacePoolID parameter

that will need existing pool checking instead of $RunspacePoolID = [guid]::NewGuid().ToString() [....] i.e. invoke new RunspacePool code only if selected with -RunspaceID parameter is not found

proxb commented 8 years ago

I think I was looking at this at one point in my initial module build but ended up shelving it for one reason or another. This is something that I will definitely revisit and see about adding in a future release.

MVKozlov commented 8 years ago

After some thinking, I realize that this feature will look better if it will be implemented as -LinkThreadPoolToBatchName (or may be this behaviour should be default ?) Because thread throttling as usual needed for similar tasks that already grouped by -Batch name and threadpool management out of user control.

proxb commented 8 years ago

That makes sense to me. I'll explore this probably in the coming weeks once I get a couple of other things knocked that I am working on.

proxb commented 8 years ago

I think what I will do is automatically generate a Batch name (as a guid or something like that) if one is not given and link that to the runspacepool and if the user decides to use a batch name, then that will be used to create the runspacepoolID as well. I'll need to update the DLL file to make the runspacepoolid a string as opposed to a guid.

MVKozlov commented 8 years ago

Good - lets runspaceID will be equal to batch name.

then that will be used to create the runspacepoolID

Not just create, but add new job to existing runspace with the same name/id

proxb commented 8 years ago

Not just create, but add new job to existing runspace with the same name/id

Assuming the existing runspacepool hasn't been disposed of by that time, in which case a new runspacepool would be created. This reminds me that I should look at adjusting the current time before a runspacepool is disposed of. Currently it is after a minute of inactivity but I may bump it to 5 minutes with this addition.

proxb commented 8 years ago

Made some progress on this with the linking of Batch to RunspacePoolID and most of my testing showed that this was working great. The only thing holding me back now is providing some extra code for those who use $_ within the script under a ForEach (or similiar) loop which currently only works when piping directly to the function. I'm probably going to have to some sort of scriptblock inspection to see if the variable exists outside of any other loop and treat it as though it came from the pipeline.

1..5|ForEach {
Start-RSJob -ScriptBlock {$_}
}
proxb commented 8 years ago

I may have been overthinking my last comment example. The example you showed works great and I am just leave it at that although I did find a way to detect if the function is part of a ForEach command loop... I might test that just a bit and if it doesn't work like I want it to then I will scrap it and publish the current update that I have this weekend.

proxb commented 8 years ago

@MVKozlov If you get a chance, try out the latest version update and let me know if it meets what you are looking for.

MVKozlov commented 8 years ago

Thanks :) it works exactly as needed

bonus: Test-RSJob.ps1 https://gist.github.com/MVKozlov/a093877010c213564dd4f638724ff9d4 Test-RSJob.Tests.ps1 https://gist.github.com/MVKozlov/f3754975db518a9b096b32d8503e588b

proxb commented 8 years ago

Cool! I'll get that added to my existing Pester tests.

proxb commented 8 years ago

I'm leaving this open until I can add the Pester test.

proxb commented 8 years ago

Pester test added to existing tests

proxb commented 8 years ago

Reopening as this brought on a breaking change (https://github.com/proxb/PoshRSJob/issues/108). I am still looking to include this functionality but need to spend more time researching and testing to ensure it doesn't break core functionality of the module.

MVKozlov commented 8 years ago

I do not have Issues as in https://github.com/proxb/PoshRSJob/issues/108 (leave comment about it) Can help with any further testing (excl. weekends :)

AspenForester commented 8 years ago

@MVKozlov is your Pester test from August 15 supposed to pass on the "Full Pipe input", and fail on the "OneByOne Pipe input"?

MVKozlov commented 8 years ago

@AspenForester , my Pester test works as intendent (all pass) on post-1.6.1.0 and pre-1.7.2.9 or on my 1.7.2,9 fork - #109