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.

Sign up for GitHub

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

Jump to bottom

[pantsd] Pants does not eagerly fail when the rule graph is invalid #10041

Closed
Eric-Arellano opened this issue Jun 12, 2020 · 1 comment · Fixed by #10035
Closed

[pantsd] Pants does not eagerly fail when the rule graph is invalid #10041

Eric-Arellano opened this issue Jun 12, 2020 · 1 comment · Fixed by #10035
Projects
Pants Daemon

Comments

@Eric-Arellano
Copy link
Contributor

Apply this diff:

diff --git a/src/python/pants/backend/project_info/dependees.py b/src/python/pants/backend/project_info/dependees.py
index a8e877274..46f893d11 100644
--- a/src/python/pants/backend/project_info/dependees.py
+++ b/src/python/pants/backend/project_info/dependees.py
@@ -82,7 +82,7 @@ async def dependees_goal(
     specified_addresses: Addresses, options: DependeesOptions, console: Console
 ) -> Dependees:
     # Get every target in the project so that we can iterate over them to find their dependencies.
-    all_targets = await Get[Targets](AddressSpecs([DescendantAddresses("")]))
+    all_targets = await Get[Targets](DependeesOptions([DescendantAddresses("")]))
     dependencies_per_target = await MultiGet(
         Get[Addresses](DependenciesRequest(tgt.get(Dependencies))) for tgt in all_targets
     )

Then run ./v2:

▶ ./v2
Rules with errors: 1

  @goal_rule(pants.backend.project_info.dependees:80:dependees_goal(Addresses, DependeesOptions, Console) -> Dependees, gets=[Get[Targets](DependeesOptions), Get[Addresses](DependenciesRequest)]):
    Ambiguous rules to compute Addresses with parameter types (Console, DependenciesRequest, FilesystemSpecs, OptionsBootstrapper):
      @rule(
DependenciesRequest,
UnionMembership,
GlobalOptions,
) -> Addresses,
gets=[
Get[WrappedTarget](Address)
Get[InferredDependencies](InferPythonDependencies),
]
pants.engine.target:1678:resolve_dependencies
for (DependenciesRequest, OptionsBootstrapper)
      @rule(AddressesWithOrigins) -> Addresses
pants.engine.internals.build_files:307:strip_address_origins
for (FilesystemSpecs, OptionsBootstrapper)
    Ambiguous rules to compute Addresses with parameter types (DependenciesRequest, FilesystemSpecs, OptionsBootstrapper):
      @rule(
DependenciesRequest,
UnionMembership,
GlobalOptions,
) -> Addresses,
gets=[
Get[WrappedTarget](Address)
Get[InferredDependencies](InferPythonDependencies),
]
pants.engine.target:1678:resolve_dependencies
for (DependenciesRequest, OptionsBootstrapper)
      @rule(AddressesWithOrigins) -> Addresses
pants.engine.internals.build_files:307:strip_address_origins
for (FilesystemSpecs, OptionsBootstrapper)
NoneType: None
18:06:21 [INFO] waiting for pantsd to start...
18:06:26 [INFO] waiting for pantsd to start...
18:06:31 [INFO] waiting for pantsd to start...
18:06:36 [INFO] waiting for pantsd to start...
18:06:41 [INFO] waiting for pantsd to start...
18:06:46 [INFO] waiting for pantsd to start...
18:06:51 [INFO] waiting for pantsd to start...
18:06:56 [INFO] waiting for pantsd to start...
18:07:01 [INFO] waiting for pantsd to start...
18:07:06 [INFO] waiting for pantsd to start...
18:07:11 [INFO] waiting for pantsd to start...
18:07:16 [ERROR] exceeded timeout of 60 seconds while waiting for pantsd to start
Traceback (most recent call last):
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/pants_exe.py", line 36, in main
    exit_code = runner.run(start_time)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/pants_runner.py", line 86, in run
    return RemotePantsRunner(self.args, self.env, options_bootstrapper).run(start_time)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/remote_pants_runner.py", line 209, in run
    return self._run_pants_with_retry(self._client.maybe_launch())
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/pants_daemon_client.py", line 35, in maybe_launch
    return self._launch()
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/pants_daemon_client.py", line 59, in _launch
    pantsd_pid = self.await_pid(60)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/process_manager.py", line 396, in await_pid
    caster=int,
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/process_manager.py", line 245, in await_metadata_by_name
    self._wait_for_file(file_path, ongoing_msg, completed_msg, timeout=timeout)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/process_manager.py", line 184, in _wait_for_file
    return cls._deadline_until(file_waiter, ongoing_msg, completed_msg, timeout=timeout)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/pantsd/process_manager.py", line 158, in _deadline_until
    timeout, ongoing_msg
pants.pantsd.process_manager.ProcessMetadataManager.Timeout: exceeded timeout of 60 seconds while waiting for pantsd to start
@stuhood
Copy link
Sponsor Member

stuhood commented Jun 12, 2020

Thanks for the report: relates to #9951.

@stuhood stuhood added this to TODO in Pants Daemon via automation Jun 12, 2020
@stuhood stuhood moved this from TODO to In Progress in Pants Daemon Jun 12, 2020
@stuhood stuhood mentioned this issue Jun 12, 2020
Merged
Pants Daemon automation moved this from In Progress to Done Jun 15, 2020
stuhood added a commit that referenced this issue Jun 15, 2020
@stuhood
Reinitialize pantsd for most configuration changes (#10035)
4b04464 
### 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
No open projects
Pants Daemon
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reinitialize pantsd for most configuration changes stuhood/pants
2 participants
@stuhood @Eric-Arellano