Skip to content

Commit

Permalink
Port the pantsd nailgun server to rust (#9722)
Browse files Browse the repository at this point in the history
### Problem

The setup and teardown of each request made to the nailgun server in `pantsd` had become quite complicated over time... and consequently, slower than it needed to be.

### Solution

Port `pantsd`'s nailgun server to rust using the `nails` crate. Additionally, remove the `Exiter` class, which had accumulated excess responsibilities that can instead be handled by returning `ExitCode` values. Finally, fix a few broken windows including: double logging to pantsd, double help output, closed file errors on pantsd shutdown, and redundant setup codepaths.

### Result

There is less code to maintain, and runs of `./pants --enable-pantsd help` take `~1.7s`, of which `~400ms` are spent in the server. Fixes #9448, fixes #8243, fixes #8206, fixes #8127, fixes #7653, fixes #7613, fixes #7597.
  • Loading branch information
Stu Hood authored May 13, 2020
1 parent bfb59ac commit 9506fbf
Show file tree
Hide file tree
Showing 51 changed files with 1,177 additions and 2,003 deletions.
1 change: 0 additions & 1 deletion build-support/ci_lists/integration_v2_blacklist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ tests/python/pants_test/backend/jvm/tasks:junit_run_integration
tests/python/pants_test/backend/jvm/tasks:junit_tests_concurrency_integration
tests/python/pants_test/backend/jvm/tasks:junit_tests_integration
tests/python/pants_test/backend/jvm/tasks:jvm_run_integration
tests/python/pants_test/base:exception_sink_integration
tests/python/pants_test/reporting:reporting_integration
tests/python/pants_test/tasks:bootstrap_jvm_tools_integration
133 changes: 20 additions & 113 deletions src/python/pants/base/exception_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
import threading
import traceback
from contextlib import contextmanager
from typing import Callable, Iterator, Optional
from typing import Optional

import setproctitle

from pants.base.exiter import Exiter
from pants.util.dirutil import safe_mkdir, safe_open
from pants.util.meta import classproperty
from pants.util.osutil import Pid

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -110,9 +110,6 @@ class ExceptionSink:
# NB: see the bottom of this file where we call reset_log_location() and other mutators in order
# to properly setup global state.
_log_dir = None
# We need an exiter in order to know what to do after we log a fatal exception or handle a
# catchable signal.
_exiter: Optional[Exiter] = None
# Where to log stacktraces to in a SIGUSR2 handler.
_interactive_output_stream = None
# Whether to print a stacktrace in any fatal error message printed to the terminal.
Expand Down Expand Up @@ -198,60 +195,6 @@ def reset_log_location(cls, new_log_location: str) -> None:
cls._pid_specific_error_fileobj = pid_specific_error_stream
cls._shared_error_fileobj = shared_error_stream

class AccessGlobalExiterMixin:
@property
def _exiter(self) -> Optional[Exiter]:
return ExceptionSink.get_global_exiter()

@classmethod
def get_global_exiter(cls) -> Optional[Exiter]:
return cls._exiter

@classmethod
@contextmanager
def exiter_as(cls, new_exiter_fun: Callable[[Optional[Exiter]], Exiter]) -> Iterator[None]:
"""Temporarily override the global exiter.
NB: We don't want to try/finally here, because we want exceptions to propagate
with the most recent exiter installed in sys.excepthook.
If we wrap this in a try:finally, exceptions will be caught and exiters unset.
"""
previous_exiter = cls._exiter
new_exiter = new_exiter_fun(previous_exiter)
cls._reset_exiter(new_exiter)
yield
cls._reset_exiter(previous_exiter)

@classmethod
@contextmanager
def exiter_as_until_exception(
cls, new_exiter_fun: Callable[[Optional[Exiter]], Exiter]
) -> Iterator[None]:
"""Temporarily override the global exiter, except this will unset it when an exception
happens."""
previous_exiter = cls._exiter
new_exiter = new_exiter_fun(previous_exiter)
try:
cls._reset_exiter(new_exiter)
yield
finally:
cls._reset_exiter(previous_exiter)

@classmethod
def _reset_exiter(cls, exiter: Optional[Exiter]) -> None:
"""Class state:
- Overwrites `cls._exiter`.
Python state:
- Overwrites sys.excepthook.
"""
logger.debug(f"overriding the global exiter with {exiter} (from {cls._exiter})")
# NB: mutate the class variables! This is done before mutating the exception hook, because the
# uncaught exception handler uses cls._exiter to exit.
cls._exiter = exiter
# NB: mutate process-global state!
sys.excepthook = cls._log_unhandled_exception_and_exit

@classmethod
def reset_interactive_output_stream(
cls, interactive_output_stream, override_faulthandler_destination=True
Expand All @@ -276,10 +219,14 @@ def reset_interactive_output_stream(
cls._interactive_output_stream = interactive_output_stream
except ValueError:
# Warn about "ValueError: IO on closed file" when the stream is closed.
cls.log_exception(
cls._log_exception(
"Cannot reset interactive_output_stream -- stream (probably stderr) is closed"
)

@classproperty
def should_print_exception_stacktrace(cls):
return cls._should_print_backtrace_to_terminal

@classmethod
def exceptions_log_path(cls, for_pid=None, in_dir=None):
"""Get the path to either the shared or pid-specific fatal errors log file."""
Expand All @@ -294,7 +241,7 @@ def exceptions_log_path(cls, for_pid=None, in_dir=None):
)

@classmethod
def log_exception(cls, msg):
def _log_exception(cls, msg):
"""Try to log an error message to this process's error log and the shared error log.
NB: Doesn't raise (logs an error instead).
Expand Down Expand Up @@ -446,34 +393,9 @@ def _format_unhandled_exception_log(cls, exc, tb, add_newline, should_print_back
maybe_newline=maybe_newline,
)

_EXIT_FAILURE_TERMINAL_MESSAGE_FORMAT = """\
{timestamp_msg}{terminal_msg}{details_msg}
"""

@classmethod
def _exit_with_failure(cls, terminal_msg):
timestamp_msg = (
f"timestamp: {cls._iso_timestamp_for_now()}\n"
if cls._should_print_backtrace_to_terminal
else ""
)
details_msg = (
""
if cls._should_print_backtrace_to_terminal
else "\n\n(Use --print-exception-stacktrace to see more error details.)"
)
terminal_msg = terminal_msg or "<no exit reason provided>"
formatted_terminal_msg = cls._EXIT_FAILURE_TERMINAL_MESSAGE_FORMAT.format(
timestamp_msg=timestamp_msg, terminal_msg=terminal_msg, details_msg=details_msg
)
# Exit with failure, printing a message to the terminal (or whatever the interactive stream is).
cls._exiter.exit_and_fail(msg=formatted_terminal_msg, out=cls._interactive_output_stream)

@classmethod
def _log_unhandled_exception_and_exit(
cls, exc_class=None, exc=None, tb=None, add_newline=False
):
"""A sys.excepthook implementation which logs the error and exits with failure."""
def log_exception(cls, exc_class=None, exc=None, tb=None, add_newline=False):
"""Logs an unhandled exception to a variety of locations."""
exc_class = exc_class or sys.exc_info()[0]
exc = exc or sys.exc_info()[1]
tb = tb or sys.exc_info()[2]
Expand All @@ -488,36 +410,21 @@ def _log_unhandled_exception_and_exit(
exception_log_entry = cls._format_unhandled_exception_log(
exc, tb, add_newline, should_print_backtrace=True
)
cls.log_exception(exception_log_entry)
cls._log_exception(exception_log_entry)
except Exception as e:
extra_err_msg = "Additional error logging unhandled exception {}: {}".format(exc, e)
logger.error(extra_err_msg)

# Generate an unhandled exception report fit to be printed to the terminal (respecting the
# Exiter's should_print_backtrace field).
if cls._should_print_backtrace_to_terminal:
stderr_printed_error = cls._format_unhandled_exception_log(
exc, tb, add_newline, should_print_backtrace=cls._should_print_backtrace_to_terminal
)
if extra_err_msg:
stderr_printed_error = "{}\n{}".format(stderr_printed_error, extra_err_msg)
else:
# If the user didn't ask for a backtrace, show a succinct error message without
# all the exception-related preamble. A power-user/pants developer can still
# get all the preamble info along with the backtrace, but the end user shouldn't
# see that boilerplate by default.
error_msgs = getattr(exc, "end_user_messages", lambda: [str(exc)])()
stderr_printed_error = "\n" + "\n".join(f"ERROR: {msg}" for msg in error_msgs)
cls._exit_with_failure(stderr_printed_error)
# Generate an unhandled exception report fit to be printed to the terminal.
logger.exception(exc)

_CATCHABLE_SIGNAL_ERROR_LOG_FORMAT = """\
Signal {signum} ({signame}) was raised. Exiting with failure.{formatted_traceback}
"""

@classmethod
def _handle_signal_gracefully(cls, signum, signame, traceback_lines):
"""Signal handler for non-fatal signals which raises or logs an error and exits with
failure."""
"""Signal handler for non-fatal signals which raises or logs an error."""
# Extract the stack, and format an entry to be written to the exception log.
formatted_traceback = cls._format_traceback(
traceback_lines=traceback_lines, should_print_backtrace=True
Expand All @@ -528,7 +435,7 @@ def _handle_signal_gracefully(cls, signum, signame, traceback_lines):
# TODO: determine the appropriate signal-safe behavior here (to avoid writing to our file
# descriptors re-entrantly, which raises an IOError).
# This method catches any exceptions raised within it.
cls.log_exception(signal_error_log_entry)
cls._log_exception(signal_error_log_entry)

# Create a potentially-abbreviated traceback for the terminal or other interactive stream.
formatted_traceback_for_terminal = cls._format_traceback(
Expand All @@ -538,19 +445,19 @@ def _handle_signal_gracefully(cls, signum, signame, traceback_lines):
terminal_log_entry = cls._CATCHABLE_SIGNAL_ERROR_LOG_FORMAT.format(
signum=signum, signame=signame, formatted_traceback=formatted_traceback_for_terminal
)
# Exit, printing the output to the terminal.
cls._exit_with_failure(terminal_log_entry)
# Print the output via standard logging.
logger.error(terminal_log_entry)


# Setup global state such as signal handlers and sys.excepthook with probably-safe values at module
# import time.
# Set the log location for writing logs before bootstrap options are parsed.
ExceptionSink.reset_log_location(os.getcwd())
# Sets except hook for exceptions at import time.
ExceptionSink._reset_exiter(Exiter(exiter=sys.exit))
# NB: Mutate process-global state!
sys.excepthook = ExceptionSink.log_exception
# Sets a SIGUSR2 handler.
ExceptionSink.reset_interactive_output_stream(sys.stderr.buffer)
# Sets a handler that logs nonfatal signals to the exception sink before exiting.
# Sets a handler that logs nonfatal signals to the exception sink.
ExceptionSink.reset_signal_handler(SignalHandler())
# Set whether to print stacktraces on exceptions or signals during import time.
# NB: This will be overridden by bootstrap options in PantsRunner, so we avoid printing out a full
Expand Down
89 changes: 4 additions & 85 deletions src/python/pants/base/exiter.py
Original file line number Diff line number Diff line change
@@ -1,89 +1,8 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import logging
import sys
import traceback
from typing import Callable, Union

from pants.util.strutil import ensure_binary

logger = logging.getLogger(__name__)


# Centralize integer return codes for the pants process. We just use a single bit for now.
# TODO: make these into an enum!
PANTS_SUCCEEDED_EXIT_CODE = 0
PANTS_FAILED_EXIT_CODE = 1

ExitCode = int


class Exiter:
"""A class that provides standard runtime exit behavior.
`pants.base.exception_sink.ExceptionSink` handles exceptions and fatal signals, delegating to an
Exiter instance which can be set process-globally with ExceptionSink.reset_exiter(). Call
.exit() or .exit_and_fail() on an Exiter instance when you wish to exit the runtime.
"""

def __init__(self, exiter: Callable[[Union[ExitCode, str, object]], None] = sys.exit):
"""
:param exiter: A function to be called to conduct the final exit of the runtime. (Optional)
"""
# Since we have some exit paths that run via the sys.excepthook,
# symbols we use can become garbage collected before we use them; ie:
# we can find `sys` and `traceback` are `None`. As a result we capture
# all symbols we need here to ensure we function in excepthook context.
# See: http://stackoverflow.com/questions/2572172/referencing-other-modules-in-atexit
self._exit = exiter

def __call__(self, *args, **kwargs):
"""Map class calls to self.exit() to support sys.exit() fungibility."""
return self.exit(*args, **kwargs)

def exit(self, result=PANTS_SUCCEEDED_EXIT_CODE, msg=None, out=None):
"""Exits the runtime.
:param result: The exit status. Typically either PANTS_SUCCEEDED_EXIT_CODE or
PANTS_FAILED_EXIT_CODE, but can be a string as well. (Optional)
:param msg: A string message to print to stderr or another custom file desciptor before exiting.
(Optional)
:param out: The file descriptor to emit `msg` to. (Optional)
"""
if msg:
out = out or sys.stderr
if hasattr(out, "buffer"):
out = out.buffer

msg = ensure_binary(msg)
try:
out.write(msg)
out.write(b"\n")
# TODO: Determine whether this call is a no-op because the stream gets flushed on exit, or
# if we could lose what we just printed, e.g. if we get interrupted by a signal while
# exiting and the stream is buffered like stdout.
out.flush()
except Exception as e:
# If the file is already closed, or any other error occurs, just log it and continue to
# exit.
if msg:
logger.warning(
"Encountered error when trying to log this message: {}, \n "
"exception: {} \n out: {}".format(msg, e, out)
)
# In pantsd, this won't go anywhere, because there's really nowhere for us to log if we
# can't log :(
# Not in pantsd, this will end up in sys.stderr.
traceback.print_stack()
logger.exception(e)
self._exit(result)

def exit_and_fail(self, msg=None, out=None):
"""Exits the runtime with a nonzero exit code, indicating failure.
:param msg: A string message to print to stderr or another custom file desciptor before exiting.
(Optional)
:param out: The file descriptor to emit `msg` to. (Optional)
"""
self.exit(result=PANTS_FAILED_EXIT_CODE, msg=msg, out=out)
# Centralize integer return codes for the pants process.
PANTS_SUCCEEDED_EXIT_CODE: ExitCode = 0
PANTS_FAILED_EXIT_CODE: ExitCode = 1
Loading

0 comments on commit 9506fbf

Please sign in to comment.