Skip to content

Commit

Permalink
Reinitialize pantsd for most configuration changes (#10035)
Browse files Browse the repository at this point in the history
### Problem

Currently we restart pantsd for most configuration changes, and exclude a small set of bootstrap options (by marking them `daemon=False`) that should not trigger a restart. But in most cases, restarting is heavy-handed. We'd like to be able to keep more and more of our state alive over time as we continue to remove global mutable state (in order to allow us to tackle #7654, among other things).

Additionally, the pantsd client currently implements the fingerprinting that decides when the server should restart, which blocks moving the pantsd client to rust. We'd like the client to only need to interact with a small set of options to simplify its implementation.

### Solution

Move the nailgun server out of the `PantsService` model and directly into the `PantsDaemon` class. Share a `PantsDaemonCore` between the daemon and `DaemonPantsRunner` that encapsulates the current `Scheduler` and all live services. Finally, have the `PantsDaemonCore` implement fingerprinting to decide when to reinitialize/recreate the `Scheduler` (without restarting) and trim down the options that trigger a restart (`daemon=True`) to only those that are used to start the daemon itself (rather than to create the `Scheduler`).

### Result

`pantsd` will stay up through the vast majority of options changes (with the exception of a handful of "micro-bootstrap" options), and will instead reinitialize the `Scheduler` for bootstrap options changes with some useful output when it does so.

Example:
```
$ ./pants help
23:26:22 [INFO] initializing pantsd...
23:26:24 [INFO] pantsd initialized.
Pants 1.30.0.dev0 https://pypi.org/pypi/pantsbuild.pants/1.30.0.dev0

Usage:
<snip>

$ ./pants --no-v1 help
23:26:31 [INFO] initialization options changed: reinitializing pantsd...
23:26:32 [INFO] pantsd initialized.
Pants 1.30.0.dev0 https://pypi.org/pypi/pantsbuild.pants/1.30.0.dev0

Usage:
<snip>
```

This prepares to port the client to rust, and unblocks a fix for #8200, by having the `PantsDaemon` class tear down the nailgun server cleanly in the foreground if any services exit. Fixes #6114, fixes #7573, and fixes #10041.
  • Loading branch information
stuhood authored Jun 15, 2020
1 parent e319a4f commit 4b04464
Show file tree
Hide file tree
Showing 33 changed files with 507 additions and 632 deletions.
1 change: 1 addition & 0 deletions src/python/pants/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ python_library(
'src/python/pants/init',
'src/python/pants/option',
'src/python/pants/pantsd:pants_daemon_client',
'src/python/pants/pantsd:pants_daemon_core',
'src/python/pants/reporting',
'src/python/pants/scm/subsystems:changed',
'src/python/pants/subsystem',
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/bin/daemon_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
)
from pants.init.util import clean_global_runtime_state
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.pantsd.service.scheduler_service import SchedulerService
from pants.pantsd.pants_daemon_core import PantsDaemonCore
from pants.util.contextutil import argv_as, hermetic_environment_as, stdio_as

logger = logging.getLogger(__name__)
Expand All @@ -33,9 +33,9 @@ class ExclusiveRequestTimeout(Exception):
class DaemonPantsRunner(RawFdRunner):
"""A RawFdRunner (callable) that will be called for each client request to Pantsd."""

def __init__(self, scheduler_service: SchedulerService) -> None:
def __init__(self, core: PantsDaemonCore) -> None:
super().__init__()
self._scheduler_service = scheduler_service
self._core = core
self._run_lock = Lock()

@staticmethod
Expand Down Expand Up @@ -139,7 +139,7 @@ def _run(self, working_dir: str) -> ExitCode:
# Run using the pre-warmed Session.
with self._stderr_logging(global_bootstrap_options):
try:
scheduler = self._scheduler_service.prepare()
scheduler = self._core.prepare_scheduler(options_bootstrapper)
runner = LocalPantsRunner.create(
os.environ, options_bootstrapper, scheduler=scheduler
)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def _init_graph_session(
native = Native()
native.set_panic_handler()
graph_scheduler_helper = scheduler or EngineInitializer.setup_legacy_graph(
native, options_bootstrapper, build_config
options_bootstrapper, build_config
)

global_scope = options.for_global_scope()
Expand Down
19 changes: 15 additions & 4 deletions src/python/pants/engine/internals/native.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class Native(metaclass=SingletonMetaclass):
def __init__(self):
self.externs = Externs(self.lib)
self.lib.externs_set(self.externs)
self._executor = self.lib.PyExecutor()

class BinaryLocationError(Exception):
pass
Expand Down Expand Up @@ -188,11 +189,20 @@ def override_thread_logging_destination_to_just_stderr(self):
def match_path_globs(self, path_globs: PathGlobs, paths: Iterable[str]) -> bool:
return cast(bool, self.lib.match_path_globs(path_globs, tuple(paths)))

def nailgun_server_await_bound(self, scheduler, nailgun_server) -> int:
return cast(int, self.lib.nailgun_server_await_bound(scheduler, nailgun_server))
def nailgun_server_await_bound(self, nailgun_server) -> int:
"""Blocks until the server has bound a port, and then returns the port.
def new_nailgun_server(self, scheduler, port: int, runner: RawFdRunner):
return self.lib.nailgun_server_create(scheduler, port, runner)
Returns the actual port the server has successfully bound to, or raises an exception if the
server has exited.
"""
return cast(int, self.lib.nailgun_server_await_bound(self._executor, nailgun_server))

def new_nailgun_server(self, port: int, runner: RawFdRunner):
"""Creates a nailgun server with a requested port.
Returns the server and the actual port it bound to.
"""
return self.lib.nailgun_server_create(self._executor, port, runner)

def new_tasks(self):
return self.lib.PyTasks()
Expand Down Expand Up @@ -263,6 +273,7 @@ def new_scheduler(
)

return self.lib.scheduler_create(
self._executor,
tasks,
engine_types,
# Project tree.
Expand Down
16 changes: 0 additions & 16 deletions src/python/pants/engine/internals/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
PathGlobsAndRoot,
)
from pants.engine.interactive_runner import InteractiveProcessRequest, InteractiveProcessResult
from pants.engine.internals.native import RawFdRunner
from pants.engine.internals.nodes import Return, Throw
from pants.engine.rules import Rule, RuleIndex, TaskRule
from pants.engine.selectors import Params
Expand Down Expand Up @@ -286,21 +285,6 @@ def lease_files_in_graph(self, session):
def garbage_collect_store(self):
self._native.lib.garbage_collect_store(self._scheduler)

def nailgun_server_await_bound(self, nailgun_server) -> int:
"""Blocks until the server has bound a port, and then returns the port.
Returns the actual port the server has successfully bound to, or raises an exception if the
server has exited.
"""
return cast(int, self._native.nailgun_server_await_bound(self._scheduler, nailgun_server))

def new_nailgun_server(self, port_requested: int, runner: RawFdRunner):
"""Creates a nailgun server with a requested port.
Returns the server and the actual port it bound to.
"""
return self._native.new_nailgun_server(self._scheduler, port_requested, runner)

def new_session(
self,
zipkin_trace_v2: bool,
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,10 @@ def _make_goal_map_from_rules(rules):

@staticmethod
def setup_legacy_graph(
native: Native,
options_bootstrapper: OptionsBootstrapper,
build_configuration: BuildConfiguration,
options_bootstrapper: OptionsBootstrapper, build_configuration: BuildConfiguration,
) -> LegacyGraphScheduler:
"""Construct and return the components necessary for LegacyBuildGraph construction."""
native = Native()
build_root = get_buildroot()
bootstrap_options = options_bootstrapper.bootstrap_options.for_global_scope()
use_gitignore = bootstrap_options.pants_ignore_use_gitignore
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/init/options_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def add(absolute_path, include=False):
return pants_ignore

@staticmethod
def compute_pantsd_invalidation_globs(buildroot, bootstrap_options):
def compute_pantsd_invalidation_globs(buildroot, bootstrap_options, absolute_pidfile):
"""Computes the merged value of the `--pantsd-invalidation-globs` option.
Combines --pythonpath and --pants-config-files files that are in {buildroot} dir with those
Expand All @@ -141,6 +141,7 @@ def compute_pantsd_invalidation_globs(buildroot, bootstrap_options):
# Globs calculated from the sys.path and other file-like configuration need to be sanitized
# to relative globs (where possible).
potentially_absolute_globs = (
absolute_pidfile,
*sys.path,
*bootstrap_options.pythonpath,
*bootstrap_options.pants_config_files,
Expand Down
45 changes: 28 additions & 17 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,8 @@ class GlobalOptions(Subsystem):
def register_bootstrap_options(cls, register):
"""Register bootstrap options.
"Bootstrap options" are a small set of options whose values are useful when registering other
options. Therefore we must bootstrap them early, before other options are registered, let
alone parsed.
"Bootstrap options" are the set of options necessary to create a Scheduler. If an option is
not consumed during creation of a Scheduler, it should be in `register_options` instead.
Bootstrap option values can be interpolated into the config file, and can be referenced
programmatically in registration code, e.g., as register.bootstrap.pants_workdir.
Expand Down Expand Up @@ -223,6 +222,7 @@ def register_bootstrap_options(cls, register):
"--pants-version",
advanced=True,
default=pants_version(),
daemon=True,
help="Use this pants version. Note Pants code only uses this to verify that you are "
"using the requested version, as Pants cannot dynamically change the version it "
"is using once the program is already running. This option is useful to set in "
Expand Down Expand Up @@ -323,13 +323,15 @@ def register_bootstrap_options(cls, register):
advanced=True,
metavar="<dir>",
default=os.path.join(buildroot, ".pants.d"),
daemon=True,
help="Write intermediate output files to this dir.",
)
register(
"--pants-physical-workdir-base",
advanced=True,
metavar="<dir>",
default=None,
daemon=True,
help="When set, a base directory in which to store `--pants-workdir` contents. "
"If this option is a set, the workdir will be created as symlink into a "
"per-workspace subdirectory.",
Expand All @@ -352,6 +354,7 @@ def register_bootstrap_options(cls, register):
"--pants-subprocessdir",
advanced=True,
default=os.path.join(buildroot, ".pids"),
daemon=True,
help="The directory to use for tracking subprocess metadata, if any. This should "
"live outside of the dir used by `--pants-workdir` to allow for tracking "
"subprocesses that outlive the workdir data (e.g. `./pants server`).",
Expand All @@ -360,19 +363,30 @@ def register_bootstrap_options(cls, register):
"--pants-config-files",
advanced=True,
type=list,
daemon=False,
# NB: We don't fingerprint the list of config files, because the content of the config
# files independently affects fingerprints.
fingerprint=False,
default=[get_default_pants_config_file()],
help="Paths to Pants config files.",
)
# TODO: Deprecate the --pantsrc/--pantsrc-files options? This would require being able
# to set extra config file locations in an initial bootstrap config file.
register("--pantsrc", advanced=True, type=bool, default=True, help="Use pantsrc files.")
register(
"--pantsrc",
advanced=True,
type=bool,
default=True,
# NB: See `--pants-config-files`.
fingerprint=False,
help="Use pantsrc files.",
)
register(
"--pantsrc-files",
advanced=True,
type=list,
metavar="<path>",
daemon=False,
# NB: See `--pants-config-files`.
fingerprint=False,
default=["/etc/pantsrc", "~/.pants.rc"],
help=(
"Override config with values from these files, using syntax matching that of "
Expand All @@ -389,15 +403,15 @@ def register_bootstrap_options(cls, register):
"--spec-file",
type=list,
dest="spec_files",
daemon=False,
# NB: See `--pants-config-files`.
fingerprint=False,
help="Read additional specs from this file (e.g. target addresses or file names). "
"Each spec should be one per line.",
)
register(
"--verify-config",
type=bool,
default=True,
daemon=False,
advanced=True,
help="Verify that all config file values correspond to known options.",
)
Expand Down Expand Up @@ -457,7 +471,6 @@ def register_bootstrap_options(cls, register):
advanced=True,
type=list,
default=[],
daemon=False,
metavar="<regexp>",
help="Exclude target roots that match these regexes.",
)
Expand All @@ -477,6 +490,7 @@ def register_bootstrap_options(cls, register):
"--logdir",
advanced=True,
metavar="<dir>",
daemon=True,
help="Write logs to files under this directory.",
)

Expand All @@ -485,6 +499,7 @@ def register_bootstrap_options(cls, register):
advanced=True,
type=bool,
default=True,
daemon=True,
help=(
"Enables use of the pants daemon (pantsd). pantsd can significantly improve "
"runtime performance by lowering per-run startup cost, and by caching filesystem "
Expand All @@ -500,7 +515,6 @@ def register_bootstrap_options(cls, register):
advanced=True,
type=bool,
default=False,
daemon=False,
help="Enable concurrent runs of pants. Without this enabled, pants will "
"start up all concurrent invocations (e.g. in other terminals) without pantsd. "
"Enabling this option requires parallel pants invocations to block on the first",
Expand All @@ -527,7 +541,6 @@ def register_bootstrap_options(cls, register):
advanced=True,
type=float,
default=60.0,
daemon=False,
help="The maximum amount of time to wait for the invocation to start until "
"raising a timeout exception. "
"Because pantsd currently does not support parallel runs, "
Expand All @@ -551,14 +564,14 @@ def register_bootstrap_options(cls, register):
advanced=True,
default=None,
type=dir_option,
daemon=False,
help="A directory to write execution and rule graphs to as `dot` files. The contents "
"of the directory will be overwritten if any filenames collide.",
)
register(
"--print-exception-stacktrace",
advanced=True,
type=bool,
fingerprint=False,
help="Print to console the full exception stack trace if encountered.",
)

Expand All @@ -576,7 +589,6 @@ def register_bootstrap_options(cls, register):
type=int,
default=30,
advanced=True,
daemon=False,
help="Timeout in seconds for URL reads when fetching binary tools from the "
"repos specified by --baseurls.",
)
Expand Down Expand Up @@ -607,6 +619,7 @@ def register_bootstrap_options(cls, register):
advanced=True,
type=int,
default=0,
daemon=True,
help="The port to bind the pants nailgun server to. Defaults to a random port.",
)
# TODO(#7514): Make this default to 1.0 seconds if stdin is a tty!
Expand All @@ -622,13 +635,15 @@ def register_bootstrap_options(cls, register):
"--pantsd-log-dir",
advanced=True,
default=None,
daemon=True,
help="The directory to log pantsd output to.",
)
register(
"--pantsd-invalidation-globs",
advanced=True,
type=list,
default=[],
daemon=True,
help="Filesystem events matching any of these globs will trigger a daemon restart. "
"Pants' own code, plugins, and `--pants-config-files` are inherently invalidated.",
)
Expand Down Expand Up @@ -927,7 +942,6 @@ def register_options(cls, register):
type=bool,
default=sys.stdout.isatty(),
recursive=True,
daemon=False,
help="Set whether log messages are displayed in color.",
)

Expand All @@ -943,15 +957,13 @@ def register_options(cls, register):
"--dynamic-ui",
type=bool,
default=sys.stderr.isatty(),
daemon=False,
help="Display a dynamically-updating console UI as pants runs.",
)

register(
"--v2-ui",
default=False,
type=bool,
daemon=False,
removal_version="1.31.0.dev0",
removal_hint="Use --dynamic-ui instead.",
help="Whether to show v2 engine execution progress.",
Expand Down Expand Up @@ -1003,7 +1015,6 @@ def register_options(cls, register):
"--quiet",
type=bool,
recursive=True,
daemon=False,
passive=no_v1,
help="Squelches most console output. NOTE: Some tasks default to behaving quietly: "
"inverting this option supports making them noisier than they would be otherwise.",
Expand Down
Loading

0 comments on commit 4b04464

Please sign in to comment.