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

daemon state invalidation strategy for e.g. pants.ini modification #3941

Closed
kwlzn opened this issue Oct 7, 2016 · 6 comments
Closed

daemon state invalidation strategy for e.g. pants.ini modification #3941

kwlzn opened this issue Oct 7, 2016 · 6 comments
Assignees
Milestone

Comments

@kwlzn
Copy link
Member

kwlzn commented Oct 7, 2016

currently, modification to pants.ini in a given repo doesn't end up invalidating anything in the daemon. if pertinent new configuration is added (e.g. new ignore paths), stale data (e.g. errors in previously unignored paths) can be cached and served by the daemon.

we should probably implement specialized handling for modifications to pants.ini to improve this situation. the same might also be true for PANTS_* environment variables.

@kwlzn kwlzn added the pantsd label Oct 7, 2016
@kwlzn kwlzn added this to the v2 engine/daemon adoption milestone Oct 7, 2016
@kwlzn kwlzn added the engine label Oct 7, 2016
@stuhood
Copy link
Member

stuhood commented Feb 17, 2017

There are two categories here I think.

  1. "bootstrap" options. The daemon's own options live in this category, and probably call for the daemon to restart itself.
  2. all other options (scope/task/subsystem, etc). We should likely port parsing of these options to the new engine, so that "options for these scopes" are a thing you can request in the daemon pre-fork.

stuhood pushed a commit that referenced this issue Mar 29, 2017
### Problem

The "rule triple" / "task triple" syntax generally requires ~30 minutes worth of explanation for every new user. In particular, the triples _look_ like they are decoupled from individual functions, when they are generally defined 1-to-1.

### Solution

Switch the majority of rule declarations to a new `@rule` decorator which creates and annotates the function with a `TaskRule`. The decorator is usable in all cases where constraints are known statically... in about three cases (where constraints are computed dynamically: ie, types from the `SymbolTable`) it is necessary to explicitly create a `TaskRule`.

Additionally, removed `SelectLiteral` in favor of usage of `SingletonRule`s in all cases. When a `SingletonRule` is declared, callers can directly use `Select` to request the value for the singleton. Just like `SelectLiteral`, these continue to be an escape hatch for values which are not constructed from the ground up using files on disk (see #3941).

### Result

Fixes #4289; "task triples" are gone: long live the `@rule` decorator!
@stuhood stuhood modified the milestones: v2 engine: daemon-by-default beta, 1.3.0, 1.4.0 Apr 7, 2017
@stuhood
Copy link
Member

stuhood commented Apr 7, 2017

Perhaps a little bit controversial, but I think that we can target this to 1.4.0, and call it a known-caveat of experimenting with the daemon in 1.3.0.

lenucksi pushed a commit to lenucksi/pants that referenced this issue Apr 25, 2017
### Problem

The "rule triple" / "task triple" syntax generally requires ~30 minutes worth of explanation for every new user. In particular, the triples _look_ like they are decoupled from individual functions, when they are generally defined 1-to-1.

### Solution

Switch the majority of rule declarations to a new `@rule` decorator which creates and annotates the function with a `TaskRule`. The decorator is usable in all cases where constraints are known statically... in about three cases (where constraints are computed dynamically: ie, types from the `SymbolTable`) it is necessary to explicitly create a `TaskRule`.

Additionally, removed `SelectLiteral` in favor of usage of `SingletonRule`s in all cases. When a `SingletonRule` is declared, callers can directly use `Select` to request the value for the singleton. Just like `SelectLiteral`, these continue to be an escape hatch for values which are not constructed from the ground up using files on disk (see pantsbuild#3941).

### Result

Fixes pantsbuild#4289; "task triples" are gone: long live the `@rule` decorator!
@kwlzn kwlzn changed the title daemon state invalidation strategy for pants.ini modification daemon state invalidation strategy for e.g. pants.ini modification May 23, 2017
@kwlzn
Copy link
Member Author

kwlzn commented May 23, 2017

options parsing stuff makes sense. would like to chat about your thoughts on the implementation strategy for that a bit more whenever you have time @stuhood.

it also seems like there's a blocking chicken&egg situation here in #4618, so I'll probably focus on that aspect for now.

additionally, runtime imports will hold unwanted state in the form of stale pants sources which could lead to confusion when actively developing on pants. we'll likely need to devise a fingerprinting strategy to ensure the daemon stays in sync when it's underlying sources are modified - or else advocate for a development process that either excludes pantsd or emphasizes manual lifecycle control.

@stuhood
Copy link
Member

stuhood commented Jun 16, 2017

.@benjyw volunteered to do the portion of this that is related to computing configuration using the engine. Thanks Benjy!

@kwlzn kwlzn self-assigned this Jul 21, 2017
@kwlzn kwlzn modified the milestones: 1.4.x, Daemon Beta Oct 3, 2017
@kwlzn
Copy link
Member Author

kwlzn commented Oct 27, 2017

now that pantsd's transitive options dependencies are in bootstrap options and the daemon launches from a clean memory state, I think the core problem here should be as easy as fingerprinting a specific set of bootstrap options and comparing that to a resident fingerprint in the running daemon.

the fingerprint would include:

  • options that affect pantsd (workdir, processdir, etc)
  • options that affect pantsd services (pailgun host/port, fs event workers, etc)
  • options that affect any transitive components of pantsd services (like the scheduler services scheduler or watchman)
  • the pants version

I /think/ we'll only have to worry about task and subsystem options as we begin to move their execution/initialization/storage into the v2 scheduler.

@kwlzn
Copy link
Member Author

kwlzn commented Oct 27, 2017

(or it may be simpler to just fingerprint all bootstrap options)

kwlzn added a commit that referenced this issue Nov 4, 2017
Problem

Currently, if options (from cli args, pants.ini, env vars, rc files et al) affecting the daemon change between runs the daemon does not detect this and will happily run with trapped, stale state.

Solution

Fingerprint the relevant subset of bootstrap options at startup time in the thin client and compare that fingerprint to the one used at launch time for the current pantsd process to determine if the daemon needs to be restarted in advance of the run. This piggy-backs on top of the daemon externalization work.

Result

Fixes #3941.
kwlzn added a commit to twitter/pants that referenced this issue Nov 27, 2017
Problem

Currently, if options (from cli args, pants.ini, env vars, rc files et al) affecting the daemon change between runs the daemon does not detect this and will happily run with trapped, stale state.

Solution

Fingerprint the relevant subset of bootstrap options at startup time in the thin client and compare that fingerprint to the one used at launch time for the current pantsd process to determine if the daemon needs to be restarted in advance of the run. This piggy-backs on top of the daemon externalization work.

Result

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

No branches or pull requests

3 participants