[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