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

multiprocessing: allow Spack to run uninterrupted in background #14682

Merged
merged 4 commits into from
Mar 20, 2020

Conversation

becker33
Copy link
Member

Spack currently cannot run as a background process uninterrupted because some of the logging functions used in the install method (especially to create the dynamic verbosity toggle with the v key) cause the OS to issue a SIGTTOU to Spack when it's backgrounded.

This PR puts the necessary gatekeeping in place so that Spack doesn't do anything that will cause a signal to stop the process when operating as a background process.

@becker33 becker33 changed the title allow Spack to run uninterrupted in background multiprocessing: allow Spack to run uninterrupted in background Jan 30, 2020
@becker33 becker33 added the WIP label Jan 31, 2020
@scheibelp scheibelp self-requested a review March 4, 2020 00:05
Copy link
Member

@scheibelp scheibelp left a comment

Choose a reason for hiding this comment

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

I have a couple questions and a minor request about relocating import statements.

lib/spack/llnl/util/tty/log.py Outdated Show resolved Hide resolved
lib/spack/llnl/util/tty/log.py Show resolved Hide resolved
lib/spack/llnl/util/tty/log.py Show resolved Hide resolved
@tgamblin tgamblin added this to In progress in Spack v0.14.1 release via automation Mar 9, 2020
@tgamblin tgamblin moved this from In progress to Review in progress in Spack v0.14.1 release Mar 19, 2020
@tgamblin tgamblin added build Issues with general build capability and removed WIP labels Mar 19, 2020
lib/spack/llnl/util/tty/log.py Show resolved Hide resolved
lib/spack/llnl/util/tty/log.py Show resolved Hide resolved
lib/spack/llnl/util/tty/log.py Outdated Show resolved Hide resolved
Copy link
Member

@tgamblin tgamblin left a comment

Choose a reason for hiding this comment

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

Ok after further discussion with @becker33 and @scheibelp, we're going to merge this as-is, and revisit -v, the v toggle, and the CLI UI for install when we look at how we might do concurrent progress bars for parallel spack installs.

@becker33: fix the flake8 issues and we'll merge this.

@tgamblin tgamblin moved this from Review in progress to Reviewer approved in Spack v0.14.1 release Mar 20, 2020
@scheibelp scheibelp dismissed their stale review March 20, 2020 18:49

We've decided on another approach

@tgamblin tgamblin merged commit 75fafce into develop Mar 20, 2020
Spack v0.14.1 release automation moved this from Reviewer approved to Done Mar 20, 2020
tgamblin pushed a commit that referenced this pull request Mar 20, 2020
Spack currently cannot run as a background process uninterrupted because some of the logging functions used in the install method (especially to create the dynamic verbosity toggle with the v key) cause the OS to issue a SIGTTOU to Spack when it's backgrounded.

This PR puts the necessary gatekeeping in place so that Spack doesn't do anything that will cause a signal to stop the process when operating as a background process.
@tgamblin tgamblin deleted the bugfix/background-spack branch March 20, 2020 21:35
tgamblin added a commit that referenced this pull request Mar 28, 2020
With the addition of #13100 (parallel builds), people are going to try to
run Spack in the background more often.  This makes Spack handle that
situation gracefully, the way a good POSIX program should.

Specifically:

1. When `spack install` is running, we disable echo and canonical input
   so that users can type `v` to toggle build output.  We do that in a
   safe way now, so that it does not generate `SIGTTOU` in the background
   (#14682 did this).

2. We properly disable keyboard input mode when Spack is placed in the
   background, and re-enable it when Spack is in the foreground.  This
   means that if you Ctrl-Z a spack install, your terminal won't be left
   in a weird state.

3. We'll continue writing verbose output when Spack is in the background.
   If you have `stty +tostop` on, it'll end up stopping the build when
   you try to run in the background, unless you redirect output. This is
   normal behavior and it lets you easily do thingslike `spack install &>
   log.txt &`

4. Spack works fine when stopped in the background or when running in the
   background.

(2) is handled mostly with signal handlers (the way things like `vi` and
`emacs` do it) -- see the code for how that's done -- it's a bit tricky
in Python, as Python did not support blocking signals until 3.8.  It
turns out we can still make it work.
tgamblin added a commit that referenced this pull request Mar 28, 2020
With the addition of #13100 (parallel builds), people are going to try to
run Spack in the background more often.  This makes Spack handle that
situation gracefully, the way a good POSIX program should.

Specifically:

1. When `spack install` is running, we disable echo and canonical input
   so that users can type `v` to toggle build output.  We do that in a
   safe way now, so that it does not generate `SIGTTOU` in the background
   (#14682 did this).

2. We properly disable keyboard input mode when Spack is placed in the
   background, and re-enable it when Spack is in the foreground.  This
   means that if you Ctrl-Z a spack install, your terminal won't be left
   in a weird state.

3. We'll continue writing verbose output when Spack is in the background.
   If you have `stty +tostop` on, it'll end up stopping the build when
   you try to run in the background, unless you redirect output. This is
   normal behavior and it lets you easily do things like this:

       spack install -v &> log.txt &

4. Spack works fine when stopped in the background or when running in the
   background.

(2) is handled mostly with signal handlers (the way things like `vi` and
`emacs` do it) -- see the code for how that's done -- it's a bit tricky
in Python, as Python did not support blocking signals until 3.8.  It
turns out we can still make it work.
tgamblin added a commit that referenced this pull request Mar 28, 2020
With the addition of #13100 (parallel builds), people are going to try to
run Spack in the background more often.  This makes Spack handle that
situation gracefully, the way a good POSIX program should.

Specifically:

1. When `spack install` is running, we disable echo and canonical input
   so that users can type `v` to toggle build output.  We do that in a
   safe way now, so that it does not generate `SIGTTOU` in the background
   (#14682 did this).

2. We properly disable keyboard input mode when Spack is placed in the
   background, and re-enable it when Spack is in the foreground.  This
   means that if you Ctrl-Z a spack install, your terminal won't be left
   in a weird state.

3. We'll continue writing verbose output when Spack is in the background.
   If you have `stty +tostop` on, it'll end up stopping the build when
   you try to run in the background, unless you redirect output. This is
   normal behavior and it lets you easily do things like this:

       spack install -v &> log.txt &

4. Spack works fine when stopped in the background or when running in the
   background.

(2) is handled mostly with signal handlers (the way things like `vi` and
`emacs` do it) -- see the code for how that's done -- it's a bit tricky
in Python, as Python did not support blocking signals until 3.8.  It
turns out we can still make it work.
tgamblin added a commit that referenced this pull request Mar 28, 2020
With the addition of #13100 (parallel builds), people are going to try to
run Spack in the background more often.  This makes Spack handle that
situation gracefully, the way a good POSIX program should.

Specifically:

1. When `spack install` is running, we disable echo and canonical input
   so that users can type `v` to toggle build output.  We do that in a
   safe way now, so that it does not generate `SIGTTOU` in the background
   (#14682 did this).

2. We properly disable keyboard input mode when Spack is placed in the
   background, and re-enable it when Spack is in the foreground.  This
   means that if you Ctrl-Z a spack install, your terminal won't be left
   in a weird state.

3. We'll continue writing verbose output when Spack is in the background.
   If you have `stty +tostop` on, it'll end up stopping the build when
   you try to run in the background, unless you redirect output. This is
   normal behavior and it lets you easily do things like this:

       spack install -v &> log.txt &

4. Spack works fine when stopped in the background or when running in the
   background.

(2) is handled mostly with signal handlers (the way things like `vi` and
`emacs` do it) -- see the code for how that's done -- it's a bit tricky
in Python, as Python did not support blocking signals until 3.8.  It
turns out we can still make it work.
tgamblin added a commit that referenced this pull request Apr 1, 2020
With the addition of #13100 (parallel builds), people are going to try to
run Spack in the background more often.  This makes Spack handle that
situation gracefully, the way a good POSIX program should.

Specifically:

1. When `spack install` is running, we disable echo and canonical input
   so that users can type `v` to toggle build output.  We do that in a
   safe way now, so that it does not generate `SIGTTOU` in the background
   (#14682 did this).

2. We properly disable keyboard input mode when Spack is placed in the
   background, and re-enable it when Spack is in the foreground.  This
   means that if you Ctrl-Z a spack install, your terminal won't be left
   in a weird state.

3. We'll continue writing verbose output when Spack is in the background.
   If you have `stty +tostop` on, it'll end up stopping the build when
   you try to run in the background, unless you redirect output. This is
   normal behavior and it lets you easily do things like this:

       spack install -v &> log.txt &

4. Spack works fine when stopped in the background or when running in the
   background.

(2) is handled mostly with signal handlers (the way things like `vi` and
`emacs` do it) -- see the code for how that's done -- it's a bit tricky
in Python, as Python did not support blocking signals until 3.8.  It
turns out we can still make it work.
likask pushed a commit to likask/spack that referenced this pull request Apr 7, 2020
…upstream_master

* commit 'e2b1737a42c9c0c796671f9dd0c39f623e4c91c0': (1343 commits)
  update CHANGELOG.md for 0.14.1
  version bump: 0.14.1
  multiprocessing: allow Spack to run uninterrupted in background (spack#14682)
  Cray bugfix: TERM missing while reading default target (spack#15381)
  Upstreams: don't write metadata directory to upstream DB (spack#15526)
  Creating versions from urls doesn't modify class attributes (spack#15452)
  bugfix: fix install_missing_compilers option bug from v0.14.0 (spack#15416)
  bugfix: installer.py shouldn't be executable (spack#15386)
  Add function replace_prefix_nullterm for use on mach-o rpaths. (spack#15347)
  ArchSpec: fix semantics of satisfies when not concrete and strict is true (spack#15319)
  suite-sparse: fix installation for v5.X (spack#15326)
  testing:  increase installer coverage (spack#15237)
  bugfix: resolve undefined source_pkg_dir failure (spack#15339)
  Bugfix: resolve StopIteration message attribute failure (spack#15341)
  Recover coverage from subprocesses during unit tests (spack#15354)
  Correct pytest.raises matches to match (spack#15346)
  bugfix:  Add dependents when initializing spec from yaml (spack#15220)
  Uniquify suffixes added to module names (spack#14920)
  bugfix: ensure proper dependency handling for package-only installs (spack#15197)
  Fix for being able to 'spack load' packages that have been renamed. (spack#14348)
  ...

# Conflicts:
#	.travis.yml
#	lib/spack/spack/modules/common.py
#	var/spack/repos/builtin/packages/mofem-cephas/package.py
#	var/spack/repos/builtin/packages/mofem-fracture-module/package.py
#	var/spack/repos/builtin/packages/mofem-users-modules/package.py
#	var/spack/repos/builtin/packages/python/package.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues with general build capability
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants