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

add initial/experimental support for installing extensions in parallel #3667

Merged
merged 25 commits into from
Oct 27, 2021

Conversation

boegel
Copy link
Member

@boegel boegel commented Apr 28, 2021

WIP because needs enhancements in the test suite + some more testing with R

@boegel boegel force-pushed the install_extensions branch from 40e394f to 5f86294 Compare October 20, 2021 20:52
@boegel boegel force-pushed the install_extensions branch from 83a26c8 to 2e5c6ab Compare October 21, 2021 13:14
…Extension instances + checking for extensions to skip
@boegel
Copy link
Member Author

boegel commented Oct 21, 2021

Tests in latest commit are failing because update_progress_bar gets called without calling start_progress_bar first.

To fix that without jumping through lots of code hoops, I've proposed some trivial changes in #3867...

@easybuilders easybuilders deleted a comment from boegelbot Oct 22, 2021
Copy link
Member

@branfosj branfosj left a comment

Choose a reason for hiding this comment

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

My testing has been going well and this looks good.

easybuild/framework/easyblock.py Show resolved Hide resolved
easybuild/tools/output.py Show resolved Hide resolved
@easybuilders easybuilders deleted a comment from boegelbot Oct 25, 2021
@easybuilders easybuilders deleted a comment from boegelbot Oct 26, 2021
@boegel boegel force-pushed the install_extensions branch from 6d94d39 to 0dd9061 Compare October 26, 2021 18:51
@boegel boegel changed the title add support for installing extensions in parallel (WIP) add support for installing extensions in parallel Oct 26, 2021
@boegel boegel marked this pull request as ready for review October 26, 2021 20:58
@boegel
Copy link
Member Author

boegel commented Oct 26, 2021

Now that a test is in place that checks the support for installing extensions in parallel, I think this is ready to merge, in combination with easybuilders/easybuild-easyblocks#2408.

I've tested this extensively on a wide range of R easyconfigs, and I've yet to run into a failing installation because of installing extensions in parallel, so at least for R extensions this seems to work really well.

Quick summary of requirements to also support parallel installation of other types of extensions:

  • The run_async implementation should only run a single shell command, which should be run with run_cmd(..., asynchronous=True) via Extension.async_cmd_start. Any other stuff can be run via the existing Extension.prerun or Extension.postrun methods.
  • For all extensions involved, the required dependencies must be available via the required_deps property method.

The changes to the EB_toy and Toy_Extension easyblocks in the tests give a good idea of how easyblocks that derive from Extension or ExtensionEasyBlock need to be updated in order to support parallel installation of those type of extensions.

Note that this is marked as an experimental feature and it's opt-in, so it is only used when EasyBuild is configured with both --parallel-extensions-install and --experimental enabled.

@boegel boegel changed the title add support for installing extensions in parallel add experimental support for installing extensions in parallel Oct 27, 2021
@boegel boegel changed the title add experimental support for installing extensions in parallel add initial/experimental support for installing extensions in parallel Oct 27, 2021
Copy link
Member

@branfosj branfosj left a comment

Choose a reason for hiding this comment

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

lgtm

@branfosj
Copy link
Member

Going in, thanks @boegel!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants