From 0760fe8b0c9d0d26b9f65522cc3239346ff562cb Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Wed, 24 Jul 2019 02:57:01 -0700 Subject: [PATCH] [pantsd] Remove dead code around prefork graph warming (#8095) **Implementation for #8002** Before removing forking from pantsd (#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 #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. --- src/python/pants/bin/daemon_pants_runner.py | 55 ++++++------------- src/python/pants/init/engine_initializer.py | 16 +----- .../pants/pantsd/service/scheduler_service.py | 24 ++++---- 3 files changed, 29 insertions(+), 66 deletions(-) diff --git a/src/python/pants/bin/daemon_pants_runner.py b/src/python/pants/bin/daemon_pants_runner.py index b0f52349dba..36b4defb1ae 100644 --- a/src/python/pants/bin/daemon_pants_runner.py +++ b/src/python/pants/bin/daemon_pants_runner.py @@ -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 @@ -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. @@ -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): @@ -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) @@ -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)) diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index e8f6aa1f508..a464493fd17 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -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, @@ -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. diff --git a/src/python/pants/pantsd/service/scheduler_service.py b/src/python/pants/pantsd/service/scheduler_service.py index 76ebfdc52e0..a595e0af606 100644 --- a/src/python/pants/pantsd/service/scheduler_service.py +++ b/src/python/pants/pantsd/service/scheduler_service.py @@ -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 @@ -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)` """ @@ -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) @@ -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, @@ -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())