Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put Temp on RAM Disk for Azure Pipelines tests #7263

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

chrahunt
Copy link
Member

@chrahunt chrahunt commented Oct 28, 2019

Profiling on Azure Pipelines indicates that the majority of our time is
spent waiting for filesystem operations to complete. As a quick way to
improve our test speed in this area, we can do operations on a RAM disk
instead of the default SSDs.

Related to #4497, #6791.

@chrahunt chrahunt added C: tests Testing and related things skip news Does not need a NEWS file entry (eg: trivial changes) labels Oct 28, 2019
@chrahunt chrahunt force-pushed the maint/speedup-azure-pipelines branch from d653fa8 to 1979fe0 Compare October 28, 2019 04:20
@chrahunt
Copy link
Member Author

chrahunt commented Oct 28, 2019

Nice, this saves about 10 minutes for integration tests, 30 seconds for unit tests, and only takes 1 minute extra for setup. It may be worth putting the tox environment in the same filesystem.

Compare:

  1. this build
  2. previous CI build 1
  3. previous CI build 2

@chrahunt chrahunt marked this pull request as ready for review October 28, 2019 04:55
@xavfernandez
Copy link
Member

Nice speedup !
It would be even better if it was built-in in the pipelines ^^
You might want to create an issue in https://github.com/Microsoft/azure-pipelines-image-generation/issues (not sure it is the right place) to ask for an easier integration ?

@chrahunt
Copy link
Member Author

Not sure what could be improved there, maybe FS-iSCSITarget-Server could be installed by default?

I can clean up our config at least by moving it to a separate script and then invoke it from the yml file.

@xavfernandez
Copy link
Member

maybe FS-iSCSITarget-Server could be installed by default?

Ideally a RAM disk would be available by default ? 😛

@xavfernandez
Copy link
Member

cc @brcrista (as author of #5785) since he might have more familiarity on what could possibly be suggested/improved :)

@brcrista
Copy link
Contributor

this saves about 10 minutes for integration tests, 30 seconds for unit tests, and only takes 1 minute extra for setup

Well done!

From looking at the logs, it looks like about 30 seconds of that setup is spent on Install-WindowsFeature -Name FS-iSCSITarget-Server.

cc @vtbassmatt -- something to consider pre-installing on the Windows image, depending on how much storage it eats up.

@chrahunt chrahunt force-pushed the maint/speedup-azure-pipelines branch 7 times, most recently from e66cb6d to 9be075a Compare October 29, 2019 02:44
Profiling on Azure Pipelines indicates that the majority of our time is
spent waiting for filesystem operations to complete. As a quick way to
improve our test speed in this area, we can do operations on a RAM disk
instead of the default SSDs.
@chrahunt chrahunt force-pushed the maint/speedup-azure-pipelines branch from 9be075a to ceaf75b Compare October 29, 2019 02:50
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hurray!

@chrahunt
Copy link
Member Author

Thanks for reviewing @pradyunsg and @xavfernandez, and @brcrista for initiating the agent image update request.

I tested and Install-WindowsFeature will succeed regardless of whether the feature is already installed, so I'll merge this and we can remove the install step if it gets added by default.

@chrahunt chrahunt merged commit 3ff414b into pypa:master Oct 29, 2019
@chrahunt chrahunt deleted the maint/speedup-azure-pipelines branch October 29, 2019 04:55
@pradyunsg
Copy link
Member

@chrahunt if you haven't already, it might be worth investigating if this is a good idea on Appveyor as well, since that's our bottleneck CI service currently.

@chrahunt
Copy link
Member Author

We could also remove it entirely. Given 1 PR is submitted, currently we would have 4 idle runners on Azure Pipelines which could handle the 3 builds we do on Appveyor in parallel.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Nov 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 28, 2019
@pradyunsg
Copy link
Member

I followed up on Azure Pipelines + GitHub Actions installing FS-iSCSITarget-Server in their images at actions/runner-images#708.

@pradyunsg
Copy link
Member

They've added this to their VM images now.

@chrahunt
Copy link
Member Author

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: tests Testing and related things skip news Does not need a NEWS file entry (eg: trivial changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants