Merge lp:~verterok/ubuntuone-client/safe-md-v5-migration into lp:ubuntuone-client

[Guillermo Gonzalez]

Backup volume manager metadata before doing the migration to v6 and restore it in case of error.

This branch make the migration from volumemanager metadata v5 to v6 a bit more robust by doing a backup of current metadata and restoring it in case of error.

[Natalia Bidart (nataliabidart)]

Past tense for break is broke, without a 'd'. Could you remove the d from the test case names?
Approving nevertheless.

review: Approve

[Guillermo Gonzalez (verterok)]

good catch
fixed and pushed

On Fri, Mar 26, 2010 at 2:22 PM, Naty Bidart
<email address hidden>wrote:

> Review: Approve
> Past tense for break is broke, without a 'd'. Could you remove the d from
> the test case names?
> Approving nevertheless.
> --
>
> https://code.launchpad.net/~verterok/ubuntuone-client/safe-md-v5-migration/+merge/22234
> You are the owner of lp:~verterok/ubuntuone-client/safe-md-v5-migration.
>

[lp:~verterok/ubuntuone-client/safe-md-v5-migration]

449. By Guillermo Gonzalez on 2010-03-26

fix typo

[Facundo Batista (facundo)]

You're doing:

1. Move MD to BKUP.
2. Migrate BKUP to MD.

If something bad happens in 2, you grab the exception, and fix it. But what about if something *very* bad happens? (like if energy is lost in the computer)

I think that in that case, the MD will be broken.

If I understood the problem correctly, this is what you should do:

1. Migrate MD to NEWMD
2. Move MD to BKUP
3. Move NEWMD to MD

The 2 and 3 moves are atomic, the only problem could be energy going off in the middle of 2 and 3 (you can fix that by checking BKUP at startup if you don't find MD).

Also, please, fix the docstring of iteritems().

review: Needs Fixing

[Guillermo Gonzalez (verterok)]

Point!

fixed and pushed (revno 450).

Thanks!

[lp:~verterok/ubuntuone-client/safe-md-v5-migration]

450. By Guillermo Gonzalez on 2010-03-29

migrate the metadata in a temporary dir and move it once we are done.

[Facundo Batista (facundo)]

Looks ok now!

review: Approve

=== modified file 'tests/syncdaemon/test_fileshelf.py'
--- tests/syncdaemon/test_fileshelf.py	2010-02-10 17:35:26 +0000
+++ tests/syncdaemon/test_fileshelf.py	2010-03-29 17:42:32 +0000
@@ -133,7 +133,7 @@
         self.assertTrue(('foo', 'bar') and ('foo1', 'bar1') in \
                         [(k, v) for k, v in shelf.items()])
 
-    def test_broked_metadata_without_backup(self):
+    def test_broken_metadata_without_backup(self):
         """test the shelf behavior when it hit a broken metadata file without
         backup.
         """
@@ -148,7 +148,7 @@
             f.write(BROKEN_PICKLE)
         self.assertRaises(KeyError, self.shelf.__getitem__, 'broken_pickle')
 
-    def test_broked_metadata_with_backup(self):
+    def test_broken_metadata_with_backup(self):
         """test that each time a metadata file is updated a .old is kept"""
         self.shelf['bad_file'] = {'value':'old'}
         path = self.shelf.key_file('bad_file')
@@ -254,6 +254,42 @@
         self.assertEquals(shelf.values['foo'],
                           cPickle.dumps('bar', protocol=2))
 
+    def test_broken_metadata_iteritems(self):
+        """Test that broken metadata is ignored during iteritems."""
+        self.shelf['ok_key'] = {'status':'this is valid metadata'}
+        self.shelf['bad_file'] = {}
+        path = self.shelf.key_file('bad_file')
+        open(path, 'w').close()
+        self.assertRaises(KeyError, self.shelf.__getitem__, 'bad_file')
+        self.assertEquals(1, len(list(self.shelf.iteritems())))
+        self.assertFalse(os.path.exists(path))
+
+        self.shelf['broken_pickle'] = {}
+        path = self.shelf.key_file('broken_pickle')
+        with open(path, 'w') as f:
+            f.write(BROKEN_PICKLE)
+        self.assertRaises(KeyError, self.shelf.__getitem__, 'broken_pickle')
+        self.assertEquals(1, len(list(self.shelf.iteritems())))
+        self.assertFalse(os.path.exists(path))
+
+    def test_broken_metadata_items(self):
+        """Test that broken metadata is ignored during iteritems."""
+        self.shelf['ok_key'] = {'status':'this is valid metadata'}
+        self.shelf['bad_file'] = {}
+        path = self.shelf.key_file('bad_file')
+        open(path, 'w').close()
+        self.assertRaises(KeyError, self.shelf.__getitem__, 'bad_file')
+        self.assertEquals(1, len(list(self.shelf.items())))
+        self.assertFalse(os.path.exists(path))
+
+        self.shelf['broken_pickle'] = {}
+        path = self.shelf.key_file('broken_pickle')
+        with open(path, 'w') as f:
+            f.write(BROKEN_PICKLE)
+        self.assertRaises(KeyError, self.shelf.__getitem__, 'broken_pickle')
+        self.assertEquals(1, len(list(self.shelf.items())))
+        self.assertFalse(os.path.exists(path))
+
 
 class CachedFileShelfTests(TestFileShelf):
     """TestFileShelf tests but using CachedFileShelf"""
@@ -273,7 +309,7 @@
         self.shelf['realkey']
         self.assertEquals(self.shelf.cache_hits, 1)
 
-    def test_broked_metadata_with_backup(self):
+    def test_broken_metadata_with_backup(self):
         """overrides parent test as we have the value in the cache."""
         self.shelf['bad_file'] = {'value':'old'}
         path = self.shelf.key_file('bad_file')

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2010-03-22 21:01:01 +0000
+++ tests/syncdaemon/test_vm.py	2010-03-29 17:42:32 +0000
@@ -1819,7 +1819,8 @@
         for new_key in new_keys:
             self.assertIn(new_key, old_keys)
         # check the old data is still there (in the backup)
-        backup_shelf = LegacyShareFileShelf(os.path.join(self.vm_data_dir, '0.bkp'))
+        bkp_dir = os.path.join(os.path.dirname(self.vm_data_dir), '5.bkp', '0.bkp')
+        backup_shelf = LegacyShareFileShelf(bkp_dir)
         backup_keys = [key for key in backup_shelf.keys()]
         for old_key in old_keys:
             self.assertIn(old_key, backup_keys)
@@ -2528,6 +2529,75 @@
             self.assertTrue(isinstance(share, Shared))
             compare_share(share, old_share)
 
+    def test_upgrade_5_critical_error(self):
+        """Test the migration from version 5 with a critical error."""
+        # build a fake version 5 state
+        self._build_layout_version_4()
+        self.set_md_version('5')
+        # create some old shares and shared metadata
+        legacy_shares = LegacyShareFileShelf(self.share_md_dir)
+        root_share = _Share(path=self.root_dir, share_id='',
+                            access_level='Modify')
+        legacy_shares[''] = root_share
+        for idx, name in enumerate(['share'] * 10):
+            sid = str(uuid.uuid4())
+            share_name = name + '_' + str(idx)
+            share = _Share(path=os.path.join(self.shares_dir, share_name),
+                           share_id=sid, name=share_name,
+                           node_id=str(uuid.uuid4()),
+                           other_username='username'+str(idx),
+                           other_visible_name='visible name ' + str(idx))
+            if idx % 2:
+                share.access_level = 'Modify'
+            else:
+                share.access_level = 'View'
+            legacy_shares[sid] = share
+        # create shared shares
+        legacy_shared = LegacyShareFileShelf(self.shared_md_dir)
+        for idx, name in enumerate(['dir'] * 5):
+            sid = str(uuid.uuid4())
+            share_name = name + '_' + str(idx)
+            share = _Share(path=os.path.join(self.root_dir, share_name),
+                           share_id=sid, node_id=str(uuid.uuid4()),
+                           name=share_name, other_username='hola',
+                           other_visible_name='hola')
+            if idx % 2:
+                share.access_level = 'Modify'
+            else:
+                share.access_level = 'View'
+            legacy_shared[sid] = share
+
+        # keep a copy of the current shares and shared metadata to check
+        # the upgrade went ok
+        legacy_shares = dict(legacy_shares.items())
+        legacy_shared = dict(legacy_shared.items())
+
+        if self.md_version_None:
+            self.set_md_version('')
+        # upgrade it!
+        old_upgrade_share_to_volume = MetadataUpgrader._upgrade_share_to_volume
+        def upgrade_share_to_volume(share, shared=False):
+            raise ValueError('FAIL!')
+        MetadataUpgrader._upgrade_share_to_volume = upgrade_share_to_volume
+        try:
+            self.assertRaises(ValueError, FakeMain, self.root_dir, self.shares_dir,
+                              self.data_dir, self.partials_dir)
+        finally:
+            MetadataUpgrader._upgrade_share_to_volume = old_upgrade_share_to_volume
+
+        shares = LegacyShareFileShelf(self.share_md_dir)
+        self.assertEquals(len(list(shares.keys())), len(legacy_shares.keys()))
+        for sid, share in shares.iteritems():
+            old_share = legacy_shares[sid]
+            self.assertTrue(isinstance(share, _Share))
+            self.assertTrue(isinstance(old_share, _Share))
+        shared = LegacyShareFileShelf(self.shared_md_dir)
+        self.assertEquals(len(list(shared.keys())), len(legacy_shared.keys()))
+        for sid, share in shared.iteritems():
+            old_share = legacy_shared[sid]
+            self.assertTrue(isinstance(share, _Share))
+            self.assertTrue(isinstance(old_share, _Share))
+
 
 class BrokenOldMDVersionUpgradeTests(MetadataOldLayoutTests):
     """MetadataOldLayoutTests with broken .version file."""

=== modified file 'ubuntuone/syncdaemon/file_shelf.py'
--- ubuntuone/syncdaemon/file_shelf.py	2010-01-15 20:04:32 +0000
+++ ubuntuone/syncdaemon/file_shelf.py	2010-03-29 17:42:32 +0000
@@ -186,6 +186,15 @@
             counter += 1
         return counter
 
+    def iteritems(self):
+        """Custom iteritems that discard 'broken' metadata."""
+        for k in self:
+            try:
+                yield (k, self[k])
+            except KeyError:
+                del self[k]
+                continue
+
 
 class CachedFileShelf(FileShelf):
     """A extension of FileShelf that uses a cache of 1500 items"""

=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
--- ubuntuone/syncdaemon/volume_manager.py	2010-03-22 21:01:01 +0000
+++ ubuntuone/syncdaemon/volume_manager.py	2010-03-29 17:42:32 +0000
@@ -1156,8 +1156,8 @@
         if not os.path.exists(self._shares_dir):
             os.makedirs(self._shares_dir)
         new_shelf = LegacyShareFileShelf(self._shares_md_dir)
-        for key in old_shelf.keys():
-            new_shelf[key] = old_shelf[key]
+        for key, share in old_shelf.iteritems():
+            new_shelf[key] = share
         # now upgrade to metadata 2
         self._upgrade_metadata_2(md_version)
 
@@ -1169,11 +1169,11 @@
         """
         self.log.debug('upgrading share shelfs from metadata 1')
         shares = LegacyShareFileShelf(self._shares_md_dir)
-        for key in shares.keys():
-            shares[key] = shares[key]
+        for key, share in shares.iteritems():
+            shares[key] = share
         shared = LegacyShareFileShelf(self._shared_md_dir)
-        for key in shared.keys():
-            shared[key] = shared[key]
+        for key, share in shared.iteritems():
+            shared[key] = share
         # now upgrade to metadata 3
         self._upgrade_metadata_2(md_version)
 
@@ -1264,8 +1264,7 @@
 
         # update the shares metadata
         shares = LegacyShareFileShelf(self._shares_md_dir)
-        for key in shares.keys():
-            share = shares[key]
+        for key, share in shares.iteritems():
             if share.path is not None:
                 if share.path == old_root_dir:
                     share.path = share.path.replace(old_root_dir,
@@ -1276,8 +1275,7 @@
                 shares[key] = share
 
         shared = LegacyShareFileShelf(self._shared_md_dir)
-        for key in shared.keys():
-            share = shared[key]
+        for key, share in shared.iteritems():
             if share.path is not None:
                 share.path = share.path.replace(old_root_dir, self._root_dir)
             shared[key] = share
@@ -1321,26 +1319,37 @@
     def _upgrade_metadata_5(self, md_version):
         """Upgrade to version 6 (plain dict storage)."""
         self.log.debug('upgrading from metadata 5')
-        # upgrade shares
-        old_shares = LegacyShareFileShelf(self._shares_md_dir)
-        shares = VMFileShelf(self._shares_md_dir)
-        for key in old_shares.keys():
-            share = old_shares[key]
-            shares[key] = self._upgrade_share_to_volume(share)
-        # upgrade shared folders
-        old_shared = LegacyShareFileShelf(self._shared_md_dir)
-        shared = VMFileShelf(self._shared_md_dir)
-        for key in shared.keys():
-            share = old_shared[key]
-            shared[key] = self._upgrade_share_to_volume(share, shared=True)
-        # upgrade the udfs
-        old_udfs = LegacyShareFileShelf(self._udfs_md_dir)
-        udfs = VMFileShelf(self._udfs_md_dir)
-        for key in old_udfs.keys():
-            udf = old_udfs[key]
-            udfs[key] = UDF(udf.id, udf.node_id, udf.suggested_path,
-                            udf.path, udf.subscribed)
-        self.update_metadata_version()
+        bkp_dir = os.path.join(os.path.dirname(self._data_dir), '5.bkp')
+        new_md_dir = os.path.join(os.path.dirname(self._data_dir), 'md_6.new')
+        new_shares_md_dir = os.path.join(new_md_dir, 'shares')
+        new_shared_md_dir = os.path.join(new_md_dir, 'shared')
+        new_udfs_md_dir = os.path.join(new_md_dir, 'udfs')
+        try:
+            # upgrade shares
+            old_shares = LegacyShareFileShelf(self._shares_md_dir)
+            shares = VMFileShelf(new_shares_md_dir)
+            for key, share in old_shares.iteritems():
+                shares[key] = self._upgrade_share_to_volume(share)
+            # upgrade shared folders
+            old_shared = LegacyShareFileShelf(self._shared_md_dir)
+            shared = VMFileShelf(new_shared_md_dir)
+            for key, share in old_shared.iteritems():
+                shared[key] = self._upgrade_share_to_volume(share, shared=True)
+            # upgrade the udfs
+            old_udfs = LegacyShareFileShelf(self._udfs_md_dir)
+            udfs = VMFileShelf(new_udfs_md_dir)
+            for key, udf in old_udfs.iteritems():
+                udfs[key] = UDF(udf.id, udf.node_id, udf.suggested_path,
+                                udf.path, udf.subscribed)
+            # move md dir to bkp
+            os.rename(self._data_dir, bkp_dir)
+            # move new to md dir
+            os.rename(new_md_dir, self._data_dir)
+            self.update_metadata_version()
+        except Exception:
+            # something bad happend, remove partially upgraded metadata
+            shutil.rmtree(new_md_dir)
+            raise
 
     def _upgrade_share_to_volume(self, share, shared=False):
         """Upgrade from _Share to new Volume hierarchy."""