afiune / test-kitchen

Test Kitchen is an integration tool for developing and testing infrastructure code and software on isolated target platforms.
http://kitchen.ci
Other
10 stars 7 forks source link

Use a single winrm shell for uploads #10

Closed mwrock closed 9 years ago

mwrock commented 9 years ago

This leverages some of the lower level methods exposed by the winrm library to use a single session for all commands in an upload. Although winrm still has to make multiple http requests. It only has t od oauth negotiation once and I believe it can reuse the same guest conhost.exe process on each call.

I witnessed a 3x perf boost 30 minutes to 10 minutes on a cookbook that included the dependencies of the windows and dsc cookbooks.

I think this is just one in several perf improvements we can make and I will try to keep them coming.

mwrock commented 9 years ago

Got this down to 1 minute. :) It needs a bit of cleanup and the dirty check is buggy but I'll take care of that over the weekend. This essentially zips the directories and base64s them. We are able to get around 4x compression and its mostly a contiguous block so much fewer round trips.

So this should hopefully avoid the need for any driver specific optimizations like my vsphere driver or @jdmundrawala vagrant work. We can likely get this even much faster but I think it is now usable for normal sized payloads.

Again, its a mess right now but I'll get it cleaned up.

mwrock commented 9 years ago

just got this working using the winrm arguments which has no size restriction. So we can break up large zips into much larger chunks. I take back all the mean things i said about winrm.

sneal commented 9 years ago

@mwrock Great work! BTW - I would like to see the winrm gem natively support:

  1. Remote file management/uploads
  2. Elevated shell via a scheduled task.

This would allow Vagrant and Test-Kitchen to share the same functionality, and any performance improvements ;) This is something I've been wanting to implement for the last few months.

jaym commented 9 years ago

awesome, ill pull it today and play around.

mwrock commented 9 years ago

@sneal yes. I was thinking of submitting a winrm pr with an upload method that includes this work.

Sent from my Windows Phone


From: Shawn Nealmailto:notifications@github.com Sent: ‎10/‎11/‎2014 6:36 AM To: afiune/test-kitchenmailto:test-kitchen@noreply.github.com Cc: Matt Wrockmailto:matt@mattwrock.com Subject: Re: [test-kitchen] Use a single winrm shell for uploads (#10)

@mwrock Great work! BTW - I would like to see the winrm gem natively support:

  1. Remote file management/uploads
  2. Elevated shell via a scheduled task.

This would allow Vagrant and Test-Kitchen to share the same functionality, and any performance improvements ;) This is something I've been wanting to implement for the last few months.


Reply to this email directly or view it on GitHub: https://github.com/afiune/test-kitchen/pull/10#issuecomment-58749760

mwrock commented 9 years ago

The dirty check on the transferred zip files is fixed and things have been cleaned up a bit. I think if it is deemed acceptable by others, its ok to be merged as it stands. I'll add the argument work either as a separate branch or add to this one if it is not merged first.

I set the arguments work aside this weekend due to some trouble I was having. The basic winrm argument mechanism works but its not as straightforward as I thought. I'm gonna pick that up next since I think that stands to leap frog performance. I started testing with a cookbook that contains a 4mb cookbook file. The cookbook is here: https://github.com/mwrock/dsc_nugetserver. You can really feel the 8k limit when it busses over the cookbooks. The default winrm input bytes limit is 500k which should bring round trips down significantly.

mwrock commented 9 years ago

Its looking like even breaking the command up with arguments is still susceptible to the 8191 character limit. The only approach to overcome that from what I can tell is to implement the powershell remoting protocol which is outside the scope of this PR.

Still, the perf boost from sharing a shell and zipping directories is significant.

mwrock commented 9 years ago

Just an update: The file copy logic here was ported into the WinRM gem and merged into the current dev branch yesterday https://github.com/WinRb/WinRM/tree/v1.3. Its has a couple more optimizations over what is here but I'm gonna hold off until that gem is released before retrofiting those changes back here. I'm not sure what @sneal's release plans are but certainly dont see why this could not go as is and be retrofitted later since it wont have any public interface impact.