Patchwork [1,of,1,STABLE,V2] merge: increase safety of parallel updating/removing on icasefs

login
register
mail settings
Submitter Katsunori FUJIWARA
Date April 30, 2013, 5:59 p.m.
Message ID <uobcvaohi.wl%foozy@lares.dti.ne.jp>
Download mbox | patch
Permalink /patch/1514/
State Superseded
Commit 9bfa86746c9c1f6ab51deb8f174ffc482417d09f
Headers show

Comments

Katsunori FUJIWARA - April 30, 2013, 5:59 p.m.
At Mon, 29 Apr 2013 12:32:24 +0200,
Mads Kiilerich wrote:
> 
> On 04/29/2013 09:07 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1367218695 -32400
> > #      Mon Apr 29 15:58:15 2013 +0900
> > # Branch stable
> > # Node ID b1dccec8c4a05fd7e8531fc8f4dcb954ab2aca78
> > # Parent  f01a351db79106ba96ac6d527cf69944fd98e665
> > merge: increase safety of parallel updating/removing on icasefs
> 
> I guess the same problem could happen if a file is replaced by a 
> directory. That could increase the severity from 'just' wrong casing on 
> iOS to random failures on Linux.
> 
> We have tests for that case, but it will of course never use workers 
> with the unfortunate partitioning of actions.
> 
> It would be nice if we could have a test for this case.
> 
> It seems like we don't have any test coverage of the new parallel update 
> in the test suite - is that really true?
> 
> /Mads

Thank you for your comment, Mads.

I only thought about case-insensitive environment, and didn't hit on
that parallel updating may cause incorrect behavior also on case
sensitive environment.

According to current "worker" implementation, at least 340 actions are
required for execution in parallel on dual processor environment.

So, I wrote the test below to reproduce your scenario easily (and I
confirmed that my patch fixes this issue). What about this kind test ?

You may have to change magic number "50" in newly added
"tests/test-worker.t" to reproduce this issue on your environment:
"40" is not enough on my environment (2.3GHz Core i5 Mac mini, 16G
memory).

========================================


----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

========================================
diff -r f01a351db791 tests/hghave.py
--- a/tests/hghave.py   Fri Apr 26 01:12:03 2013 +0900
+++ b/tests/hghave.py   Wed May 01 02:38:50 2013 +0900
@@ -274,6 +274,13 @@ 
 def has_msys():
     return os.getenv('MSYSTEM')

+def has_multiproc():
+    # ignore Windows environment
+    try:
+        return 1 < int(os.sysconf('SC_NPROCESSORS_ONLN'))
+    except (AttributeError, ValueError):
+        return False
+
 checks = {
     "true": (lambda: True, "yak shaving"),
     "false": (lambda: False, "nail clipper"),
@@ -296,6 +303,7 @@ 
     "inotify": (has_inotify, "inotify extension support"),
     "killdaemons": (has_killdaemons, 'killdaemons.py support'),
     "lsprof": (has_lsprof, "python lsprof module"),
+    "multiproc": (has_multiproc, "multi processor"),
     "mtn": (has_mtn, "monotone client (>= 1.0)"),
     "outer-repo": (has_outer_repo, "outer repo"),
     "p4": (has_p4, "Perforce server and client"),
diff -r f01a351db791 tests/test-worker.t
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-worker.t       Wed May 01 02:38:50 2013 +0900
@@ -0,0 +1,81 @@ 
+  $ "$TESTDIR/hghave" multiproc || exit 80
+
+  $ hg init update-collision
+  $ cd update-collision
+
+  $ cat > setupfiles.py <<EOF
+  > import os, sys
+  > os.mkdir(sys.argv[1])
+  > for x in xrange(50): # "50" may have to be bigger for reproductivity ?
+  >     f = open('%s/%04d' % (sys.argv[1], x), 'w')
+  >     f.write(str(x))
+  >     f.close()
+  > EOF
+
+  $ python setupfiles.py a
+  $ echo b > b
+  $ hg add -q a b
+  $ hg commit -m 'add b (0)'
+
+  $ hg remove -q a b
+  $ hg commit -m 'remove b (1)'
+
+  $ mkdir b
+  $ echo b/b > b/b
+  $ python setupfiles.py c
+  $ hg add -q b c
+  $ hg commit -m 'add b/b (2)'
+
+  $ hg remove -q b/b
+  $ mkdir d
+  $ echo d/d > d/d
+  $ hg add -q d
+  $ hg commit -m 'add d/d, remove b/b (3)'
+
+  $ hg -q remove c d
+  $ hg commit -m 'remove d/d (4)'
+
+  $ echo d > d
+  $ python setupfiles.py e
+  $ hg add -q d e
+  $ hg commit -m 'add d (5)'
+
+  $ hg update -q -C 0 --config worker.numcpus=1
+  $ hg status -A b
+  C b
+
+  $ cat > wrap.py <<EOF
+  > from mercurial import extensions, worker
+  >
+  > def worthwhile(orgfnc, ui, costperop, nops):
+  >     return True # to reproduce collision easily
+  > def uisetup(ui):
+  >     extensions.wrapfunction(worker, 'worthwhile', worthwhile)
+  > EOF
+
+| 'actions' should consist of below actions in this order:
+|
+|   - removing 'a' x N + 'b'
+|   - updating 'b/b' + 'c' x N
+|
+| this causes updating 'b/b' before removing 'b', because "numcpus=2"
+| locate the former at the top of the queue on the second worker.
+
+  $ hg update -q -C 2 --config extensions.wrap=wrap.py --config worker.numcpus$
+  $ hg status -A b/b
+  C b/b
+
+  $ hg update -q -C 3 --config worker.numcpus=1
+  $ hg status -A d/d
+  C d/d
+
+| 'actions' should consist of below in actions this order:
+|
+|   - removing 'c' x N + 'd/d'
+|   - updating 'd' + 'e' x N
+
+  $ hg update -q -C 5 --config extensions.wrap=wrap.py --config worker.numcpus$
+  $ hg status -A d
+  C d
+
+  $ cd ..