[PATCH 1 of 2 STABLE] worker: be silent if killed by the main process

Yuya Nishihara yuya at tcha.org
Fri Apr 21 07:43:48 EDT 2017


On Thu, 20 Apr 2017 09:18:01 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1492704007 25200
> #      Thu Apr 20 09:00:07 2017 -0700
> # Node ID f0bd43ca0d5659a1e49bf4948a9d3d0f30a69e34
> # Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r f0bd43ca0d56
> worker: be silent if killed by the main process
> 
> Previously, worker.py will send SIGTERM to remaining workers once a failure
> of another worker is noticed. Workers receiving SIGTERM will go through
> scmutil.callcatch, and may have extra outputs like "killed!" and a
> traceback, which breaks test-worker.t.

Can't we simply move the SignalInterrupt handling to dispatch? SignalInterrupt
is a sub-type of KeyboardInterrupt, so it doesn't make sense to handle only
SignalInterrupt in scmutil.callcatch().

I noticed "interrupted!" message isn't printed when worker is involved, but
that is a minor issue we don't have to fix in the stable release.

> Besides, it's hard to predicate how many "killed!" or traceback should be
> printed if we keep that behavior.

Well, too many "killed!" messages are bad, but traceback is for developers.
I don't think it should be suppressed just for test stability.

> Since the main process will notice a worker failure and handle it
> accordingly, it makes sense to kill the remaining workers silently.
> Therefore this patch adds a backdoor signal - SIGUSR2 to allow a silent
> SIGTERM kill.
> 
> This makes test-worker.t less flaky. The next patch will make test-worker.t
> stronger (and easier to break without this patch).
> 
> diff --git a/mercurial/worker.py b/mercurial/worker.py
> --- a/mercurial/worker.py
> +++ b/mercurial/worker.py
> @@ -100,4 +100,6 @@ def _posixworker(ui, func, staticargs, a
>          for p in pids:
>              try:
> +                # SIGUSR2 makes a worker silent for the upcoming SIGTERM
> +                os.kill(p, signal.SIGUSR2)
>                  os.kill(p, signal.SIGTERM)
>              except OSError as err:
> @@ -135,4 +137,11 @@ def _posixworker(ui, func, staticargs, a
>      oldchldhandler = signal.signal(signal.SIGCHLD, sigchldhandler)
>      ui.flush()
> +    # when the main process notices an error (non-0 exit code), remaining
> +    # workers will be killed without causing noisy output ("killed!" and a
> +    # traceback). this is done by a special SIGUSR2.
> +    def silentsigterm(signum, frame):
> +        # avoid scmutil.callcatch code path which could be noisy
> +        signal.signal(signal.SIGTERM, signal.SIG_DFL)

The default handler of SIGTERM kills the process, which shouldn't be used.


More information about the Mercurial-devel mailing list