Summary:
debugcommand for receiving several lfs objects from server
Analog to the funtion `debuglfsreceive`, but for several objects
Will be used in replay traffic for Mononoke LFS.
Requires URL and an iterable with oids and sizes.
Format required:
url, (oid1, size1, oid2, size2, ... )
Write all files to the console.
Reviewed By: quark-zju
Differential Revision: D12956274
fbshipit-source-id: f83dd0636b2ad197cace9633222d0f1ed8191dab
Summary: Mostly empty lines removed and added. A few bugfixes on excessive line splitting.
Reviewed By: quark-zju
Differential Revision: D8199128
fbshipit-source-id: 90c1616061bfd7cfbba0b75f03f89683340374d5
Summary:
Turned on the auto formatter. Ran `arc lint --apply-patches --take BLACK **/*.py`.
Then run `arc lint` again so some other autofixers like spellchecker etc. looked
at the code base. Manually accept the changes whenever they make sense, or use
a workaround (ex. changing "dict()" to "dict constructor") where autofix is false
positive. Disabled linters on files that are hard (i18n/polib.py) to fix, or less
interesting to fix (hgsubversion tests), or cannot be fixed without breaking
OSS build (FBPYTHON4).
Conflicted linters (test-check-module-imports.t, part of test-check-code.t,
test-check-pyflakes.t) are removed or disabled.
Duplicated linters (test-check-pyflakes.t, test-check-pylint.t) are removed.
An issue of the auto-formatter is lines are no longer guarnateed to be <= 80
chars. But that seems less important comparing with the benefit auto-formatter
provides.
As we're here, also remove test-check-py3-compat.t, as it is currently broken
if `PYTHON3=/bin/python3` is set.
Reviewed By: wez, phillco, simpkins, pkaush, singhsrb
Differential Revision: D8173629
fbshipit-source-id: 90e248ae0c5e6eaadbe25520a6ee42d32005621b
After applying suggestions from https://phab.mercurial-scm.org/D1564
to catch all exceptions in the same way I actually broke the handling of
KeyboardInterrupt on windows. The reason is that KeyboardInterrupt doesn't
dervie from Exception, but BaseException:
https://docs.python.org/2/library/exceptions.html starting from python 2.5
Test Plan:
Run hg on windows and ctrl-c during a large update. No random
exceptions from threads surface in the shell. Previously we'd nearly always get
stack traces from some of threads
Run tests ./run-tests.py
[...]
Failed test-convert-svn-encoding.t: output changed
# Ran 622 tests, 41 skipped, 1 failed.
python hash seed: 2962682116
The test failing seems to have nothing to do with the change and fails on base
revision as well
Differential Revision: https://phab.mercurial-scm.org/D1718
The windows workers weren't daemons and were not correctly killed when ctrl-c'd from the terminal. Withi this change when the main thread is killed, all daemons get killed as well.
I also reduced the time we give to workers to cleanup nicely to not have people ctrl-c'ing when they get inpatient.
The output when threads clened up nicely:
PS C:\<dir>> hg.exe sparse --disable-profile SparseProfiles/<profile>.sparse
interrupted!
The output when threads don't clenup in 1 sec:
PS C:\<dir> hg.exe sparse --enable-profile SparseProfiles/<profile>.sparse
failed to kill worker threads while handling an exception
interrupted!
Exception in thread Thread-4 (most likely raised during interpreter shutdown):
PS C:\<dir>>
Test Plan:
Run hg command on windows (pull/update/sparse). Ctrl-C'd sparse --enable-profile command that was using threads and observed in proces explorer that all threads got killed.
ran tests on CentOS
Differential Revision: https://phab.mercurial-scm.org/D1564
This adds config to disable/enable workers with default being enabled.
Test Plan:
enabled profile without updaing .hg/hgrc (the default should be to use workers) and ran
hg sprase --enable-profile <profile>.sparse
Watched in the proces explorer that hg started 12 new threads for materializing files (this is my worker.numcpus) value
Added
[worker]
enabled = False
to the .hg/hgrc and re ran the command. This time hg didn't spawn any new threads for matreializing of files
Differential Revision: https://phab.mercurial-scm.org/D1460
This adds handling of exceptions from worker threads and resurfaces them as if the function ran without workers.
If any of the threads throws, the main thread kills all running threads giving them 5 sec to handle the interruption and raises the first exception received.
We don't have to join threads if is_alive() is false
Test Plan:
Ran multiple updates/enable/disable sparse profile and things worked well
Ran test on CentOS- all tests passing on @ passed here
Added a forged exception into the worker code and got it properly resurfaced and the rest of workers killed: P58642088
PS C:\open\<repo>> ..\facebook-hg-rpms\build\hg\hg.exe --config extensions.fsmonitor=! sparse --enable-profile <profile>
updating [==> ] 1300/39166 1m57sException in thread Thread-3:
Traceback (most recent call last):
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\threading.py", line 801, in __bootstrap_inner
self.run()
File "C:\open\facebook-hg-rpms\build\hg\mercurial\worker.py", line 244, in run
raise e
Exception: Forged exception
Exception in thread Thread-2:
Traceback (most recent call last):
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\threading.py", line 801, in __bootstrap_inner
self.run()
File "C:\open\facebook-hg-rpms\build\hg\mercurial\worker.py", line 244, in run
raise e
Exception: Forged exception
<...>
Traceback (most recent call last):
File "C:\open\facebook-hg-rpms\build\hg\hgexe.py", line 41, in <module>
dispatch.run()
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 85, in run
status = (dispatch(req) or 0) & 255
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 173, in dispatch
ret = _runcatch(req)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 324, in _runcatch
return _callcatch(ui, _runcatchfunc)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 332, in _callcatch
return scmutil.callcatch(ui, func)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\scmutil.py", line 154, in callcatch
return func()
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 314, in _runcatchfunc
return _dispatch(req)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 951, in _dispatch
cmdpats, cmdoptions)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\remotefilelog\__init__.py", line 415, in runcommand
return orig(lui, repo, *args, **kwargs)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\undo.py", line 118, in _runcommandwrapper
result = orig(lui, repo, cmd, fullargs, *args)
File "C:\open\facebook-hg-rpms\build\hg\hgext\journal.py", line 84, in runcommand
return orig(lui, repo, cmd, fullargs, *args)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\perftweaks.py", line 268, in _tracksparseprofiles
res = runcommand(lui, repo, *args)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\perftweaks.py", line 256, in _trackdirstatesizes
res = runcommand(lui, repo, *args)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\copytrace.py", line 144, in _runcommand
return orig(lui, repo, cmd, fullargs, ui, *args, **kwargs)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\fbamend\hiddenoverride.py", line 119, in runcommand
result = orig(lui, repo, cmd, fullargs, *args)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 712, in runcommand
ret = _runcommand(ui, options, cmd, d)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 959, in _runcommand
return cmdfunc()
File "C:\open\facebook-hg-rpms\build\hg\mercurial\dispatch.py", line 948, in <lambda>
d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\util.py", line 1183, in check
return func(*args, **kwargs)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\fbsparse.py", line 860, in sparse
disableprofile=disableprofile, force=force)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\fbsparse.py", line 949, in _config
len, _refresh(ui, repo, oldstatus, oldsparsematch, force))
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\hgext3rd\fbsparse.py", line 1116, in _refresh
mergemod.applyupdates(repo, typeactions, repo[None], repo['.'], False)
File "C:\open\facebook-hg-rpms\build\hg\hg-python\lib\site-packages\remotefilelog\__init__.py", line 311, in applyupdates
return orig(repo, actions, wctx, mctx, overwrite, labels=labels)
File "C:\open\facebook-hg-rpms\build\hg\mercurial\merge.py", line 1464, in applyupdates
for i, item in prog:
File "C:\open\facebook-hg-rpms\build\hg\mercurial\worker.py", line 286, in _windowsworker
raise t.exception
Exception: Forged exception
PS C:\open\ovrsource>
Differential Revision: https://phab.mercurial-scm.org/D1459
This change implements thread based worker on windows.
The handling of exception from within threads will happen in separate diff.
The worker is for now used in mercurial/merge.py and in lfs extension
After multiple tests and milions of files materiealized, thousands lfs fetched
it seems that neither merge.py nor lfs/blobstore.py is thread unsafe. I also
looked through the code and besides the backgroundfilecloser (handled in base
of this) things look good.
The performance boost of this on windows is
~50% for sparse --enable-profile
* Speedup of hg up/rebase - not exactly measured
Test Plan:
Ran 10s of hg sparse --enable-profile and --disable-profile operations on large profiles and verified that workers are running. Used sysinternals suite to see that all threads are spawned and run as they should
Run various other operations on the repo including update and rebase
Ran tests on CentOS and all tests that pass on @ pass here
Differential Revision: https://phab.mercurial-scm.org/D1458
Previously the worker error handling is like:
pid = os.fork() --+
if pid == 0: |
.... | problematic
.... --+
try: --+
.... | worker error handling
--+
If a signal arrives when Python is executing the "problematic" lines, an
external error handling (dispatch.py) will take over the control flow and
it's no longer guaranteed "os._exit" is called (see 560cc0db7f01 for why it
is necessary).
This patch rewrites the error handling so it covers all possible code paths
for a worker even during fork.
Note: "os.getpid() == parentpid" is used to test if the process is parent or
not intentionally, instead of checking "pid", because "pid = os.fork()" may
be not atomic - it's possible that that a signal hits the worker before the
assignment completes [1]. The newly added test replaces "os.fork" to
exercise that extreme case.
[1]: CPython compiles "pid = os.fork()" to 2 byte codes: "CALL_FUNCTION" and
"STORE_FAST", so it's probably not atomic:
def f():
pid = os.fork()
dis.dis(f)
2 0 LOAD_GLOBAL 0 (os)
3 LOAD_ATTR 1 (fork)
6 CALL_FUNCTION 0
9 STORE_FAST 0 (pid)
12 LOAD_CONST 0 (None)
15 RETURN_VALUE
a91c6275 introduces flushing ui buffers after a worker finished. If the ui was
not flushed before the worker was started, fork will copy the existing buffers
to the worker. This causes messages issued before the worker started to be
written to the terminal for each worker.
We are now flushing the ui before we start a worker and add an appropriate test
which will fail before this patch.
Before this patch, worker implementation assumes that os.waitpid()
with os.WNOHANG returns '(0, 0)' for still running child process. This
is explicitly specified as below in Python API document.
os.WNOHANG
The option for waitpid() to return immediately if no child
process status is available immediately. The function returns
(0, 0) in this case.
On the other hand, POSIX specification doesn't define the "stat_loc"
value returned by waitpid() with WNOHANG for such child process.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/waitpid.html
CPython implementation for os.waitpid() on POSIX doesn't take any care
of this gap, and this may cause unexpected "exit status indication"
even on POSIX conformance platform.
For example, os.waitpid() with os.WNOHANG returns non-zero "exit
status indication" on FreeBSD. This implies os.kill() with own pid or
sys.exit() with non-zero exit code, even if no child process fails.
To ignore meaningless exit status indication returned by os.waitpid(),
this patch skips subsequent steps forcibly, if os.waitpid() returns 0
as pid.
This patch also arranges examination of 'p' value for readability.
FYI, there are some issues below about this behavior reported for
CPython.
https://bugs.python.org/issue21791https://bugs.python.org/issue27808
os.fdopen() does not accepts bytes as its second argument which represent the
mode in which the file is to be opened. This patch makes sure unicodes are
passed in py3 by using pycompat.sysstr().
os.name returns unicodes on py3 and we have pycompat.osname which returns
bytes. This series of 2 patches will change every ocurrence of os.name with
pycompat.osname.
Like commandserver, the worker should never run other resource cleanup logic.
Previously this is not true for workers if they have exceptions other than
KeyboardInterrupt.
This actually caused a real-world deadlock with remotefilelog:
1. remotefilelog/fileserverclient creates a sshpeer. pipei/o/e get created.
2. worker inherits that sshpeer's pipei/o/e.
3. worker runs sshpeer.cleanup (only happens without os._exit)
4. worker closes pipeo/i, which will normally make the sshpeer read EOF from
its stdin and exit. But the master process still have pipeo, so no EOF.
5. worker reads pipee (stderr of sshpeer), which never completes because
the ssh process does not exit, does not close its stderr.
6. master waits for all workers, which never completes because they never
complete sshpeer.cleanup.
This could also be addressed by closing these fds after fork, which is not
easy because Python 2.x does not have an official "afterfork" hook. Hacking
os.fork is also ugly. Besides, sshpeer is probably not the only troublemarker.
The patch changes _posixworker so all its code paths will use os._exit to
avoid running unwanted resource clean-ups.
Since we now wait child processes in non-blocking way (changed by 6c7588a50638
and 13c3aefdee29), we don't have to kill them in the middle of the waitpid()
loop. This change will help solving a possible race of waitpid()-pids.discard()
sequence and another SIGCHLD.
waitforworkers() is called by cleanup(), in which case we do killworkers()
beforehand so we can remove killworkers() from waitforworkers().
killworkers() iterates over pids, which can be updated by SIGCHLD handler.
So we should either copy pids or prevent killworkers() from being interrupted
by SIGCHLD. I chose the latter as it is simpler and can make pids handling
more consistent.
This fixes a possible "set changed size during iteration" error at
killworkers() before cleanup().
Now that we have a SIGCHLD hander, and it could get executed when waiting
for I/O. It's no longer necessary to have a separated waitpid thread. So
just remove it.
As planned by previous patches, add a SIGCHLD handler to get notifications
about worker exits, and deals with worker failure immediately.
Note that the SIGCHLD handler gets unregistered before killworkers(), so
SIGCHLD won't interrupt "killworkers" - making it harder to send kill
signals to waited processes.
We are going to use it in the SIGCHLD handler. The handler will be executed
in the main thread with the non-blocking version of waitpid, while the
waitforworkers thread runs the blocking version. It's possible that one of
them collects a worker and makes the other error out (no child to wait).
This patch handles these errors: ECHILD is ignored. EINTR needs a retry.
The "pids" set is designed to be only modifiable by "waitforworkers". And we
only remove items after a successful waitpid. Since a child process can only
be "waitpid"-ed once. It's guaranteed that "pids.remove(p)" won't be called
with duplicated "p"s. And once a "p" is removed from "pids", that "p" does
not need to be killed or waited any more.
This patch adds a boolean flag to waitforworkers and makes it non-blocking
if set to True.
This is to make it possible that we can reap our workers while keep other
unrelated children untouched, after receiving SIGCHLD.
Before this patch, waitforworkers uses os.wait() to collect child workers, and
only wait len(pids) processes. This can have serious issues if other code
spawns new processes and does not reap them: 1. worker.py may get wrong exit
code and kill innocent workers. 2. worker.py may continue without waiting for
all workers to complete.
This patch fixes the issue by using waitpid to wait worker pid explicitly.
However, this patch introduces a new issue: worker failure may not be handled
immediately. The issue will be addressed in next patches.
mpm isn't a fan of the existing or previous partitioning scheme. He
provided a fantastic justification for why on the mailing list.
This patch adds his words to the code so they aren't forgotten.
The only consumer of the worker pool code today is `hg update`.
Previously, the algorithm to partition work to each worker process
preserved input list ordering. We'd take the first N elements, then
the next N elements, etc. Measurements on mozilla-central demonstrate
this isn't an optimal partitioning strategy.
I added debug code to print when workers were exiting. When performing
a working copy update on a previously empty working copy of
mozilla-central, I noticed that process lifetimes were all over the
map. One worker would complete after 7s. Many would complete after
12s. And another worker would often take >16s. This behavior occurred
for many worker process counts and was more pronounced on some than
others.
What I suspect is happening is some workers end up with lots of
small files and others with large files. This is because the update
code passes in actions according to sorted filenames. And, directories
under tend to accumulate similar files. For example, test directories
often consist of many small test files and media directories contain
binary (often larger) media files.
This patch changes the partitioning algorithm to select every Nth
element from the input list. Each worker thus has a similar composition
of files to operate on.
The result of this change is that worker processes now all tend to exit
around the same time. The possibility of a long pole due to being
unlucky and receiving all the large files has been mitigated. Overall
execution time seems to drop, but not by a statistically significant
amount on mozilla-central. However, repositories with directories
containing many large files will likely show a drop.
There shouldn't be any regressions due to partial manifest decoding
because the update code already iterates the manifest to determine
what files to operate on, so the manifest should already be decoded.
The home of 'Abort' is 'error' not 'util' however, a lot of code seems to be
confused about that and gives all the credit to 'util' instead of the
hardworking 'error'. In a spirit of equity, we break the cycle of injustice and
give back to 'error' the respect it deserves. And screw that 'util' poser.
For great justice.
This is a backout of 0f1a7b0ccb69. The stdlib implementation of
multiprocessing.cpu_count() attempts to invoke a process on BSD and
Darwin platforms (at least on 2.7). Under certain conditions (such as
cwd being removed) this could raise. Our old code was silently catching
the exception.
The old code was more robust, so restore it.
The multiprocessing package was added in Python 2.6.
The implementation of worker.countcpus() is very similar to
multiprocessing.cpu_count(). Ditch our one-off code.
multiprocessing does result in a number of imports. However,
the lazy importer ensures that we don't import anything until
cpu_count() is called. Furthermore, if we are doing something
with multiple cores, chances are the time of that operation
will dwarf the import time, so module bloat isn't a concern
here.
Python 2.6 introduced the "except type as instance" syntax, replacing
the "except type, instance" syntax that came before. Python 3 dropped
support for the latter syntax. Since we no longer support Python 2.4 or
2.5, we have no need to continue supporting the "except type, instance".
This patch mass rewrites the exception syntax to be Python 2.6+ and
Python 3 compatible.
This patch was produced by running `2to3 -f except -w -n .`.
_ is usually used for i18n markup but we also used it for I-don't-care
variables.
Instead, name don't-care variables in a slightly descriptive way but use the _
prefix to designate unused variable.
This will mute some pyflakes "import '_' ... shadowed by loop variable"
warnings.
If a large update triggered an abort, it was possible for the main
thread to still update the dirstate.
This fix is incomplete, as the failing worker now doesn't generate a
proper error message. This is difficult in the fork-based framework,
which relies on exceptions propagating to the top of the dispatcher
for formatting.
We now wait for worker processes in a separate thread, so that we can
spot failures in a timely way, wihout waiting for the progress pipe
to drain.
If a worker fails, we recover the pre-parallel-update behaviour of
failing early by killing its peers before propagating the failure.
