Merge lp:~facundo/ubuntuone-client/lr-dont-delete-partial-nopartialfile into lp:ubuntuone-client

Proposed by Facundo Batista on 2010-10-19
Status: Merged
Approved by: dobey on 2010-10-22
Approved revision: 736
Merged at revision: 738
Proposed branch: lp:~facundo/ubuntuone-client/lr-dont-delete-partial-nopartialfile
Merge into: lp:ubuntuone-client
Diff against target: 216 lines (+151/-6)
2 files modified
tests/syncdaemon/test_fsm.py (+125/-2)
ubuntuone/syncdaemon/filesystem_manager.py (+26/-4)
To merge this branch: bzr merge lp:~facundo/ubuntuone-client/lr-dont-delete-partial-nopartialfile
Reviewer Review Type Date Requested Status
Lucio Torre (community) Approve on 2010-10-22
Guillermo Gonzalez 2010-10-19 Approve on 2010-10-20
Review via email: mp+38834@code.launchpad.net

Commit message

Handle better errors on partial creation. (LP: #657195, LP: #662660)

Description of the change

Handle better errors on partial creation.

This has two sides:

- Leave the node in a better state if *any* error happens when touching disk: in this case, I moved the MD handling before the filesystem handling. So, from now, if something bad happens when creating the partial in the disk, the node will still be in SERVER state, and LR will re-download it.

- Support too long filenames: for this, specific code were added to the filename handling when the file is created: we trim the filename until the filesystem stops giving that error to us.

As the filename may be trimmed according to filesystem errors, it's not fully predictable any more. So, when that happens, it's stored in the metadata object.

Tests included for everything.

To post a comment you must log in.
Guillermo Gonzalez (verterok) wrote :

looks good, tests pass.

review: Approve
736. By Facundo Batista on 2010-10-20

Make the filename length platform agnostic in the test

Lucio Torre (lucio.torre) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'tests/syncdaemon/test_fsm.py'
--- tests/syncdaemon/test_fsm.py 2010-09-15 19:02:52 +0000
+++ tests/syncdaemon/test_fsm.py 2010-10-20 20:57:45 +0000
@@ -20,10 +20,11 @@
2020
21from __future__ import with_statement21from __future__ import with_statement
2222
23import errno
23import os24import os
24import shutil25import shutil
26import time
25import unittest27import unittest
26import time
2728
28from contrib.testing.testcase import (29from contrib.testing.testcase import (
29 FakeVolumeManager,30 FakeVolumeManager,
@@ -1584,7 +1585,6 @@
1584 # it has no partial!1585 # it has no partial!
1585 self.fsm.remove_partial("uuid", "share")1586 self.fsm.remove_partial("uuid", "share")
15861587
1587
1588 def test_create_dir_previous(self):1588 def test_create_dir_previous(self):
1589 """Test create .partial for a dir when the dir existed."""1589 """Test create .partial for a dir when the dir existed."""
1590 testdir = os.path.join(self.share_path, "path")1590 testdir = os.path.join(self.share_path, "path")
@@ -1697,6 +1697,129 @@
1697 self.fsm.create_partial("uuid1", "share")1697 self.fsm.create_partial("uuid1", "share")
1698 self.assertTrue(self.fsm.get_by_mdid(mdid).info.is_partial)1698 self.assertTrue(self.fsm.get_by_mdid(mdid).info.is_partial)
16991699
1700 def test_disk_problem(self):
1701 """On any disk problem, the node should be left in partial state."""
1702 testfile = os.path.join(self.share_path, "path")
1703 mdid = self.fsm.create(testfile, "share")
1704 self.fsm.set_node_id(testfile, "uuid")
1705
1706 # ugly problem not handled
1707 os.rmdir(self.partials_dir)
1708 try:
1709 self.fsm.create_partial("uuid", "share")
1710 except IOError, e:
1711 if e.errno == errno.ENOENT:
1712 # expected
1713 pass
1714 else:
1715 raise
1716
1717 # the node should still be in partial internally
1718 # for LR to handle it ok
1719 mdobj = self.fsm.get_by_mdid(mdid)
1720 self.assertTrue(mdobj.info.is_partial)
1721
1722 def test_filename_too_long(self):
1723 """Handle the filename being too long."""
1724 # find a almost too long file
1725 repeat = 300
1726 while True:
1727 testfile = os.path.join(self.share_path, "x"*repeat)
1728 try:
1729 fh = open(testfile, 'w')
1730 except IOError, e:
1731 if e.errno == errno.ENAMETOOLONG:
1732 repeat -= 10
1733 else:
1734 fh.close()
1735 os.unlink(testfile)
1736 break
1737
1738 mdid = self.fsm.create(testfile, "share")
1739 self.fsm.set_node_id(testfile, "uuid")
1740
1741 # create partial ok, even knowing that "full partial path" won't fit
1742 self.fsm.create_partial("uuid", "share")
1743 self.assertTrue(self.fsm.get_by_mdid(mdid).info.is_partial)
1744
1745 # check the path
1746 partial_path = self.fsm._get_partial_path(self.fsm.fs[mdid])
1747 self.assertTrue(os.path.exists(partial_path))
1748
1749 def test_get_partial_path_notrim(self):
1750 """Create the partial path."""
1751 testfile = os.path.join(self.share_path, "foo")
1752 mdid = self.fsm.create(testfile, "share")
1753 partial_path = self.fsm._get_partial_path(self.fsm.fs[mdid])
1754 partial_name = os.path.basename(partial_path)
1755 self.assertEqual(partial_name, mdid + ".u1partial.foo")
1756
1757 def test_get_partial_path_trim1(self):
1758 """Create the partial path trimmed some chars."""
1759 longname = "longnamethatistoolong"
1760 testfile = os.path.join(self.share_path, longname)
1761 mdid = self.fsm.create(testfile, "share")
1762 partial_path = self.fsm._get_partial_path(self.fsm.fs[mdid], trim=1)
1763 partial_name = os.path.basename(partial_path)
1764 self.assertEqual(partial_name, mdid + ".u1partial." + longname[:-10])
1765
1766 def test_get_partial_path_trim2(self):
1767 """Create the partial path trimmed more chars."""
1768 longname = "longnamethatistoolong"
1769 testfile = os.path.join(self.share_path, longname)
1770 mdid = self.fsm.create(testfile, "share")
1771 partial_path = self.fsm._get_partial_path(self.fsm.fs[mdid], trim=2)
1772 partial_name = os.path.basename(partial_path)
1773 self.assertEqual(partial_name, mdid + ".u1partial." + longname[:-20])
1774
1775 def test_get_partial_path_dontcache_when_notrim(self):
1776 """Normal behaviour, no partial path cached."""
1777 testfile = os.path.join(self.share_path, "longnamethatistoolong")
1778 mdid = self.fsm.create(testfile, "share")
1779 self.fsm._get_partial_path(self.fsm.fs[mdid])
1780
1781 # check
1782 mdobj = self.fsm.get_by_mdid(mdid)
1783 self.assertFalse(hasattr(mdobj.info, 'partial_path'))
1784
1785 def test_get_partial_path_caches_when_trim(self):
1786 """If trimming is necessary, it will cache the name."""
1787 testfile = os.path.join(self.share_path, "longnamethatistoolong")
1788 mdid = self.fsm.create(testfile, "share")
1789 partial_path = self.fsm._get_partial_path(self.fsm.fs[mdid], trim=1)
1790
1791 # check
1792 mdobj = self.fsm.get_by_mdid(mdid)
1793 self.assertEqual(mdobj.info.partial_path, partial_path)
1794
1795 def test_get_partial_path_cached_normal(self):
1796 """Return the cached partial path if there."""
1797 testfile = os.path.join(self.share_path, "foo")
1798 mdid = self.fsm.create(testfile, "share")
1799
1800 # fake the cache
1801 mdobj = self.fsm.fs[mdid]
1802 mdobj['info']['partial_path'] = "bar"
1803
1804 # check
1805 partial_path = self.fsm._get_partial_path(mdobj)
1806 partial_name = os.path.basename(partial_path)
1807 self.assertEqual(partial_name, "bar")
1808
1809 def test_get_partial_path_cached_trimming(self):
1810 """Do not return the cached partial path if there when trimming."""
1811 testfile = os.path.join(self.share_path, "foobarlongone")
1812 mdid = self.fsm.create(testfile, "share")
1813
1814 # fake the cache
1815 mdobj = self.fsm.fs[mdid]
1816 mdobj['info']['partial_path'] = "bar"
1817
1818 # check
1819 partial_path = self.fsm._get_partial_path(mdobj, trim=1)
1820 partial_name = os.path.basename(partial_path)
1821 self.assertEqual(partial_name, mdid + ".u1partial.foo")
1822
17001823
1701class FileHandlingTests(FSMTestCase):1824class FileHandlingTests(FSMTestCase):
1702 """Test the file handling services."""1825 """Test the file handling services."""
17031826
=== modified file 'ubuntuone/syncdaemon/filesystem_manager.py'
--- ubuntuone/syncdaemon/filesystem_manager.py 2010-09-15 19:02:52 +0000
+++ ubuntuone/syncdaemon/filesystem_manager.py 2010-10-20 20:57:45 +0000
@@ -876,11 +876,19 @@
876 raise InconsistencyError(msg)876 raise InconsistencyError(msg)
877 return partial_in_md877 return partial_in_md
878878
879 def _get_partial_path(self, mdobj):879 def _get_partial_path(self, mdobj, trim=None):
880 """Gets the path of the .partial file for a given mdobj"""880 """Gets the path of the .partial file for a given mdobj"""
881 if trim is None and "partial_path" in mdobj["info"]:
882 return mdobj["info"]["partial_path"]
883
881 path = self.get_abspath(mdobj['share_id'], mdobj['path'])884 path = self.get_abspath(mdobj['share_id'], mdobj['path'])
882 partial_path = os.path.join(self.partials_dir, mdobj['mdid'] + '.u1partial')885 partial_path = os.path.join(self.partials_dir, mdobj['mdid'] + '.u1partial')
883 dirname, filename = os.path.split(path)886 dirname, filename = os.path.split(path)
887
888 if trim is not None:
889 filename = filename[:-10*trim]
890 mdobj["info"]["partial_path"] = partial_path + '.' + filename
891
884 return partial_path + '.' + filename892 return partial_path + '.' + filename
885893
886 def create_partial(self, node_id, share_id):894 def create_partial(self, node_id, share_id):
@@ -896,19 +904,33 @@
896 mdobj = self.fs[mdid]904 mdobj = self.fs[mdid]
897 is_dir = mdobj["is_dir"]905 is_dir = mdobj["is_dir"]
898 path = self.get_abspath(mdobj['share_id'], mdobj['path'])906 path = self.get_abspath(mdobj['share_id'], mdobj['path'])
899 partial_path = self._get_partial_path(mdobj)
900 with self._enable_share_write(share_id, os.path.dirname(path)):907 with self._enable_share_write(share_id, os.path.dirname(path)):
901 # if we don't have the dir yet, create it908 # if we don't have the dir yet, create it
902 if is_dir and not os.path.exists(path):909 if is_dir and not os.path.exists(path):
903 self.eq.add_to_mute_filter("FS_DIR_CREATE", path)910 self.eq.add_to_mute_filter("FS_DIR_CREATE", path)
904 os.mkdir(path)911 os.mkdir(path)
905912
906 # don't alert EQ, partials are in other directory, not watched
907 open(partial_path, "w").close()
908 mdobj["info"]["last_partial_created"] = time.time()913 mdobj["info"]["last_partial_created"] = time.time()
909 mdobj["info"]["is_partial"] = True914 mdobj["info"]["is_partial"] = True
910 self.fs[mdid] = mdobj915 self.fs[mdid] = mdobj
911916
917 # create the partial path, trimming the name until fits
918 # in the filesystem
919 partial_path = self._get_partial_path(mdobj)
920 trim = 0
921 while True:
922 try:
923 # don't alert EQ, partials are in other directory, not watched
924 open(partial_path, "w").close()
925 except IOError, e:
926 if e.errno == errno.ENAMETOOLONG:
927 trim += 1
928 partial_path = self._get_partial_path(mdobj, trim=trim)
929 else:
930 raise
931 else:
932 break
933
912 def get_partial_for_writing(self, node_id, share_id):934 def get_partial_for_writing(self, node_id, share_id):
913 """Get a write-only fd to a partial file"""935 """Get a write-only fd to a partial file"""
914 mdid = self._idx_node_id[(share_id, node_id)]936 mdid = self._idx_node_id[(share_id, node_id)]

Subscribers

People subscribed via source and target branches