Synchronized env update for install#14621
Conversation
…t; this removes the db read tx entirely for the env update since it doesn't help much
…ifficult because unlike Environment.install it doesn't concretize anything or otherwise change the underlying .lock/.yaml file)
|
@tldahlgren @tgamblin this is a work in progress. I expect it will be improved by the end of today. It should be usable now (at least for making #13100 work with environments) |
…g a write transaction; refactor redundant logic between _install and install_all
…logic that re-reads the environment's state from the filesystem for the transaction
… written before the package is actually installed now
…reate a deadlock for multiple processes trying to install at the same time
…ns along with 'spack install' will not result in an inconsistent state
|
Closing/reopening to redo python version check |
…ocking when all specs are already installed
|
|
||
| """ | ||
|
|
||
| # If "spack install" is invoked repeatedly for a large environment |
There was a problem hiding this comment.
Glad to see this comment.
tldahlgren
left a comment
tldahlgren
left a comment
There was a problem hiding this comment.
Overall this looks fine to me though I think it would be better, in install_all, to save off and only process (in the second loop) the uninstalled specs.
tgamblin
left a comment
tgamblin
left a comment
There was a problem hiding this comment.
This looks good! I have a few questions and comments but I think it's pretty close.
…mpts to install specs that it found were uninstalled
|
@tgamblin @tldahlgren docstrings are added and the other requests are now addressed at this point too IMO |
tgamblin
left a comment
tgamblin
left a comment
There was a problem hiding this comment.
@scheibelp: one more request: I think that uninstall also needs a write transaction around the parts that update the environment. Can you add that?
|
On second thought, let's do uninstall in a separate PR. @scheibelp: can you follow up with that? |
Yes: I was looking into taking care of that and I'll open up a PR for it by mid-day tomorrow (1/29) |
|
@scheibelp: thanks! |
|
@tldahlgren: FYI, this is merged |
Great! I'll do another update after I test a change I just made. |
Just pushed the merge. |
3 participants
This is a minimal set of edits to get parallel installs as implemented in #13100 working with environments.
What it currently does:
Environment.write_transactionEnvironment.write_transactionincmd.install(UPDATE: now passing)
One test is currently failing, which I think is an issue with the testing framwork vs. the actual logic (since I cannot manually reproduce the error when manually running through commands executed in the test).TODOs
spack add,spack rmetc.; this isn't strictly necessary for correcting parallel Spack installs but it should be done (perhaps later)Database.write_transactiontoEnvironment.write_transaction, which would include re-reading the environment (this would possibly allow moving the transaction logic fromcmd.installtoEnvironment