Merge lp://qastaging/~facundo/ubuntuone-client/plt-fixpath into lp://qastaging/ubuntuone-client

Proposed by Facundo Batista
Status: Merged
Approved by: Facundo Batista
Approved revision: 1311
Merged at revision: 1321
Proposed branch: lp://qastaging/~facundo/ubuntuone-client/plt-fixpath
Merge into: lp://qastaging/ubuntuone-client
Diff against target: 512 lines (+385/-12)
5 files modified
contrib/testing/testcase.py (+1/-0)
tests/syncdaemon/test_pathlockingtree.py (+273/-0)
tests/syncdaemon/test_sync.py (+6/-5)
ubuntuone/syncdaemon/action_queue.py (+100/-7)
ubuntuone/syncdaemon/sync.py (+5/-0)
To merge this branch: bzr merge lp://qastaging/~facundo/ubuntuone-client/plt-fixpath
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Manuel de la Peña (community) Approve
Review via email: mp+124518@code.qastaging.launchpad.net

Commit message

Fix the PathLockTree elements on a move operation.

Description of the change

Fix the PathLockTree elements on a move operation.

Tests included.

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) wrote :

Here is some background for the other reviewers:

<facundobatista> so... the situation is the following, the PathLockTree locks operations on other operations regarding the path
<facundobatista> if you touch a file 'foo', a MakeFile and Upload are queued, the Makefile will be executed, taking the lock on 'foo', the Upload will be locked because of the same path
<facundobatista> when Makefile finishes, it releases the lock, the Upload jumps in, all happy
<facundobatista> so, if you then do something like "ls > foo; mv foo bar"
* pedronis has quit (Ping timeout: 246 seconds)
<facundobatista> syncdaemon will get the FILE_CREATE and CLOSE_WRITE and will queue the Makefile and actually send HQ to hash the file
<facundobatista> the Makefile will be executed, taking the lock on 'foo'
<facundobatista> of course, there's a move really fast after the CREATE and CLOSE
<facundobatista> internal stuff is adjusted because of that move
<facundobatista> milliseconds later, the HQ jumps in, and tries to hash 'foo', it gets an error because the file moved! so after that error the correct path is taken (MOVE adjusted internal stuff!), and finally HQ hashes 'bar'
<facundobatista> when HQ finishes hashing bar, it queues an Upload
<facundobatista> the Upload tries to get the pathlock on 'bar' and it succeeds!!!
<facundobatista> because the Makefile was holding a lock on 'foo'
<facundobatista> so the Upload jumps in, but out of order
<facundobatista> so
<facundobatista> the fix does the following
<facundobatista> when the MOVE is processed by syncdaemon
<facundobatista> it adjusts the path of PathLockTree, changing 'foo' to 'bar', there
<facundobatista> so, the Upload will be locked
<facundobatista> and that's all
<facundobatista> it made me to change a little the internals of PathLockTree, but that's another more complex story

Revision history for this message
Manuel de la Peña (mandel) wrote :

Code looks good, here is a small comment regarding the tests but I don't think it should block the landing of the branch. I have noticed the following code repeated several times:

134 + # release
135 + releases.pop(0)()
136 + release = yield d
137 + release()
138 + self.assertEqual(len(self.plt.root['children_nodes']), 0)

It might be a good idea to reuse the code by creating a method, maybe assert_release, and use it in all the diff tests that need it. As I said, this should not block the branch.

review: Approve
Revision history for this message
Guillermo Gonzalez (verterok) :
review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (407.6 KiB)

The attempt to merge lp:~facundo/ubuntuone-client/plt-fixpath into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.69
checking for automake >= 1.10...
  testing automake-1.12... not found.
  testing automake-1.11... found 1.11.6
checking for libtool >= 1.5...
  testing libtoolize... found 2.4.2
checking for intltool >= 0.30...
  testing intltoolize... found 0.50.2
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.26
checking for gtk-doc >= 1.0...
  testing gtkdocize... found 1.18
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running gtkdocize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --enable-gtk-doc --enable-debug ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking how to create a ustar tar archive... gnutar
checking whether make supports nested variables... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking how to print strings... printf
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell unde...

1311. By Facundo Batista

Merged trunk in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches