Skip to content

Commit

Permalink
[pantsd] Remove dead code around prefork graph warming (pantsbuild#8095)
Browse files Browse the repository at this point in the history
**Implementation for pantsbuild#8002**
Before removing forking from pantsd (pantsbuild#7596 ), we needed to warm up the v2 product graph of the daemon before forking into a pantsd-runner process (relevant code here).

Now, however, this warming is not necessary (because we don't fork a process per run of pants anymore), so we can do some cleanup to that code:

We can remove the concept of "warming" (here), which is invoked from scheduler_service.prefork (here). This probably means that we can remove the entire warm_product_graph function.
We can reword the code around scheduler_service.prefork so that it doesn't mention forking at all. It was called prefork because that's what we did before pantsbuild#7596, but it can be called something different now.
We can merge the try:except block in DaemonPantsRunner.create() (here) into the try:except block in DaemonPantsRunner.run() (here). The run() function aims to centralize and correctly handle all the exceptions that happen on a pants run, so the "prefork" logic should be integrated there as well.
This will help a lot in correctly diagnosing pantsd problems, as this split and the wrangled exception handling that happen as a result have caused numerous issues.
  • Loading branch information
wisechengyi authored and blorente committed Jul 24, 2019
1 parent 570fab8 commit 0760fe8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 66 deletions.
55 changes: 17 additions & 38 deletions src/python/pants/bin/daemon_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import time
from contextlib import contextmanager

from pants.base.build_environment import get_buildroot
from pants.base.exception_sink import ExceptionSink
from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE, Exiter
from pants.bin.local_pants_runner import LocalPantsRunner
Expand Down Expand Up @@ -110,38 +109,16 @@ class DaemonPantsRunner:

@classmethod
def create(cls, sock, args, env, services, scheduler_service):
maybe_shutdown_socket = MaybeShutdownSocket(sock)

try:
# N.B. This will redirect stdio in the daemon's context to the nailgun session.
with cls.nailgunned_stdio(maybe_shutdown_socket, env, handle_stdin=False) as finalizer:
options, _, options_bootstrapper = LocalPantsRunner.parse_options(args, env)
subprocess_dir = options.for_global_scope().pants_subprocessdir
graph_helper, target_roots, exit_code = scheduler_service.prefork(options, options_bootstrapper)
finalizer()
except Exception:
graph_helper = None
target_roots = None
options_bootstrapper = None
# TODO: this should no longer be necessary, remove the creation of subprocess_dir
subprocess_dir = os.path.join(get_buildroot(), '.pids')
exit_code = 1
# TODO This used to raise the _GracefulTerminationException, and maybe it should again, or notify in some way that the prefork has failed.

return cls(
maybe_shutdown_socket,
args,
env,
graph_helper,
target_roots,
services,
subprocess_dir,
options_bootstrapper,
exit_code
maybe_shutdown_socket=MaybeShutdownSocket(sock),
args=args,
env=env,
services=services,
exit_code=0,
scheduler_service=scheduler_service
)

def __init__(self, maybe_shutdown_socket, args, env, graph_helper, target_roots, services,
metadata_base_dir, options_bootstrapper, exit_code):
def __init__(self, maybe_shutdown_socket, args, env, services, exit_code, scheduler_service):
"""
:param socket socket: A connected socket capable of speaking the nailgun protocol.
:param list args: The arguments (i.e. sys.argv) for this run.
Expand All @@ -155,16 +132,14 @@ def __init__(self, maybe_shutdown_socket, args, env, graph_helper, target_roots,
:param OptionsBootstrapper options_bootstrapper: An OptionsBootstrapper to reuse.
"""
self._name = self._make_identity()
self._metadata_base_dir = metadata_base_dir
self._maybe_shutdown_socket = maybe_shutdown_socket
self._args = args
self._env = env
self._graph_helper = graph_helper
self._target_roots = target_roots
self._services = services
self._options_bootstrapper = options_bootstrapper
self.exit_code = exit_code
self._exiter = DaemonExiter(maybe_shutdown_socket)
self._scheduler_service = scheduler_service

self.exit_code = exit_code

# TODO: this should probably no longer be necesary, remove.
def _make_identity(self):
Expand Down Expand Up @@ -275,6 +250,10 @@ def run(self):
hermetic_environment_as(**self._env), \
encapsulated_global_logger():
try:
options, _, options_bootstrapper = LocalPantsRunner.parse_options(self._args, self._env)
graph_helper, target_roots, exit_code = self._scheduler_service.prepare_v1_graph_run_v2(options, options_bootstrapper)
self.exit_code = exit_code

# Clean global state.
clean_global_runtime_state(reset_subsystem=True)

Expand All @@ -286,9 +265,9 @@ def run(self):
PantsRunFailCheckerExiter(),
self._args,
self._env,
self._target_roots,
self._graph_helper,
self._options_bootstrapper,
target_roots,
graph_helper,
options_bootstrapper,
)
runner.set_start_time(self._maybe_get_client_start_time_from_env(self._env))

Expand Down
16 changes: 1 addition & 15 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
from pants.engine.goal import Goal
from pants.engine.isolated_process import create_process_rules
from pants.engine.legacy.address_mapper import LegacyAddressMapper
from pants.engine.legacy.graph import (LegacyBuildGraph, TransitiveHydratedTargets,
create_legacy_graph_tasks)
from pants.engine.legacy.graph import LegacyBuildGraph, create_legacy_graph_tasks
from pants.engine.legacy.options_parsing import create_options_parsing_rules
from pants.engine.legacy.parser import LegacyPythonCallbacksParser
from pants.engine.legacy.structs import (JvmAppAdaptor, JvmBinaryAdaptor, PageAdaptor,
Expand Down Expand Up @@ -168,19 +167,6 @@ def __init__(self, invalid_goals):
)
self.invalid_goals = invalid_goals

def warm_product_graph(self, target_roots):
"""Warm the scheduler's `ProductGraph` with `TransitiveHydratedTargets` products.
This method raises only fatal errors, and does not consider failed roots in the execution
graph: in the v1 codepath, failed roots are accounted for post-fork.
:param TargetRoots target_roots: The targets root of the request.
"""
logger.debug('warming target_roots for: %r', target_roots)
subjects = [target_roots.specs]
request = self.scheduler_session.execution_request([TransitiveHydratedTargets], subjects)
self.scheduler_session.execute(request)

def run_console_rules(self, options_bootstrapper, goals, target_roots):
"""Runs @console_rules sequentially and interactively by requesting their implicit Goal products.
Expand Down
24 changes: 11 additions & 13 deletions src/python/pants/pantsd/service/scheduler_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ class SchedulerService(PantsService):
"""The pantsd scheduler service.
This service holds an online Scheduler instance that is primed via watchman filesystem events.
This provides for a quick fork of pants runs (via the pailgun) with a fully primed ProductGraph
in memory.
"""

QUEUE_SIZE = 64
Expand Down Expand Up @@ -162,8 +160,11 @@ def product_graph_len(self):
"""
return self._scheduler.graph_len()

def prefork(self, options, options_bootstrapper):
"""Runs all pre-fork logic in the process context of the daemon.
def prepare_v1_graph_run_v2(self, options, options_bootstrapper):
"""For v1 (and v2): computing TargetRoots for a later v1 run
For v2: running an entire v2 run
The exit_code in the return indicates whether any issue was encountered
:returns: `(LegacyGraphSession, TargetRoots, exit_code)`
"""
Expand All @@ -178,21 +179,21 @@ def prefork(self, options, options_bootstrapper):
session = self._graph_helper.new_session(zipkin_trace_v2, v2_ui)

if options.for_global_scope().loop:
prefork_fn = self._prefork_loop
fn = self._loop
else:
prefork_fn = self._prefork_body
fn = self._body

target_roots, exit_code = prefork_fn(session, options, options_bootstrapper)
target_roots, exit_code = fn(session, options, options_bootstrapper)
return session, target_roots, exit_code

def _prefork_loop(self, session, options, options_bootstrapper):
def _loop(self, session, options, options_bootstrapper):
# TODO: See https://github.com/pantsbuild/pants/issues/6288 regarding Ctrl+C handling.
iterations = options.for_global_scope().loop_max
target_roots = None
exit_code = PANTS_SUCCEEDED_EXIT_CODE
while iterations and not self._state.is_terminating:
try:
target_roots, exit_code = self._prefork_body(session, options, options_bootstrapper)
target_roots, exit_code = self._body(session, options, options_bootstrapper)
except session.scheduler_session.execution_error_type as e:
# Render retryable exceptions raised by the Scheduler.
print(e, file=sys.stderr)
Expand All @@ -202,7 +203,7 @@ def _prefork_loop(self, session, options, options_bootstrapper):
continue
return target_roots, exit_code

def _prefork_body(self, session, options, options_bootstrapper):
def _body(self, session, options, options_bootstrapper):
global_options = options.for_global_scope()
target_roots = TargetRootsCalculator.create(
options=options,
Expand All @@ -214,9 +215,6 @@ def _prefork_body(self, session, options, options_bootstrapper):

v1_goals, ambiguous_goals, v2_goals = options.goals_by_version

if v1_goals or (ambiguous_goals and global_options.v1):
session.warm_product_graph(target_roots)

if v2_goals or (ambiguous_goals and global_options.v2):
goals = v2_goals + (ambiguous_goals if global_options.v2 else tuple())

Expand Down

0 comments on commit 0760fe8

Please sign in to comment.