Rev 3810: iter_lines_added_or_present now retries. in http://bzr.arbash-meinel.com/branches/bzr/1.9-dev/pack_retry_153786

John Arbash Meinel john at arbash-meinel.com
Sat Oct 25 03:22:35 BST 2008


At http://bzr.arbash-meinel.com/branches/bzr/1.9-dev/pack_retry_153786

------------------------------------------------------------
revno: 3810
revision-id: john at arbash-meinel.com-20081025022225-wbds6xl8t5ptod6p
parent: john at arbash-meinel.com-20081025015845-7424d9znf6v36kuv
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: pack_retry_153786
timestamp: Fri 2008-10-24 21:22:25 -0500
message:
  iter_lines_added_or_present now retries.
  
  Change the test to use 3 packs/revisions, as it makes it more likely that we
  will have already processed some of the results, before we finally run into
  the missing pack file.
-------------- next part --------------
=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py	2008-10-25 01:58:45 +0000
+++ b/bzrlib/knit.py	2008-10-25 02:22:25 +0000
@@ -1458,30 +1458,39 @@
             pb = progress.DummyProgress()
         keys = set(keys)
         total = len(keys)
-        # we don't care about inclusions, the caller cares.
-        # but we need to setup a list of records to visit.
-        # we need key, position, length
-        key_records = []
-        build_details = self._index.get_build_details(keys)
-        for key, details in build_details.iteritems():
-            if key in keys:
-                key_records.append((key, details[0]))
-                keys.remove(key)
-        records_iter = enumerate(self._read_records_iter(key_records))
-        for (key_idx, (key, data, sha_value)) in records_iter:
-            pb.update('Walking content.', key_idx, total)
-            compression_parent = build_details[key][1]
-            if compression_parent is None:
-                # fulltext
-                line_iterator = self._factory.get_fulltext_content(data)
-            else:
-                # Delta 
-                line_iterator = self._factory.get_linedelta_content(data)
-            # XXX: It might be more efficient to yield (key,
-            # line_iterator) in the future. However for now, this is a simpler
-            # change to integrate into the rest of the codebase. RBC 20071110
-            for line in line_iterator:
-                yield line, key
+        done = False
+        while not done:
+            try:
+                # we don't care about inclusions, the caller cares.
+                # but we need to setup a list of records to visit.
+                # we need key, position, length
+                key_records = []
+                build_details = self._index.get_build_details(keys)
+                for key, details in build_details.iteritems():
+                    if key in keys:
+                        key_records.append((key, details[0]))
+                records_iter = enumerate(self._read_records_iter(key_records))
+                for (key_idx, (key, data, sha_value)) in records_iter:
+                    pb.update('Walking content.', key_idx, total)
+                    compression_parent = build_details[key][1]
+                    if compression_parent is None:
+                        # fulltext
+                        line_iterator = self._factory.get_fulltext_content(data)
+                    else:
+                        # Delta 
+                        line_iterator = self._factory.get_linedelta_content(data)
+                    # Now that we are yielding the data for this key, remove it
+                    # from the list
+                    keys.remove(key)
+                    # XXX: It might be more efficient to yield (key,
+                    # line_iterator) in the future. However for now, this is a
+                    # simpler change to integrate into the rest of the
+                    # codebase. RBC 20071110
+                    for line in line_iterator:
+                        yield line, key
+                done = True
+            except errors.RetryWithNewPacks, e:
+                self._access.reload_or_raise(e)
         for source in self._fallback_vfs:
             if not keys:
                 break
@@ -1890,7 +1899,6 @@
                 extra information about the content which needs to be passed to
                 Factory.parse_record
         """
-        prefixes = self._partition_keys(keys)
         parent_map = self.get_parent_map(keys)
         result = {}
         for key in keys:

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2008-10-25 01:58:45 +0000
+++ b/bzrlib/tests/test_knit.py	2008-10-25 02:22:25 +0000
@@ -370,6 +370,7 @@
             tree.add([''], ['root-id'])
             tree.commit('one', rev_id='rev-1')
             tree.commit('two', rev_id='rev-2')
+            tree.commit('three', rev_id='rev-3')
             # Pack these two revisions into another pack file, but don't remove
             # the originials
             repo = tree.branch.repository
@@ -401,7 +402,7 @@
             vf._access._indices[new_index] = access_tuple
             return True
         # Delete one of the pack files so the data will need to be reloaded. We
-        # will delete the file with 'rev-1' in it
+        # will delete the file with 'rev-2' in it
         trans, name = orig_packs[1].access_tuple()
         trans.delete(name)
         # We don't have the index trigger reloading because we want to test
@@ -585,7 +586,7 @@
 
     def test__get_record_map_retries(self):
         vf, reload_counter = self.make_vf_for_retrying()
-        keys = [('rev-1',), ('rev-2',)]
+        keys = [('rev-1',), ('rev-2',), ('rev-3',)]
         records = vf._get_record_map(keys)
         self.assertEqual(keys, sorted(records.keys()))
         self.assertEqual([1, 1, 0], reload_counter)
@@ -598,7 +599,7 @@
 
     def test_get_record_stream_retries(self):
         vf, reload_counter = self.make_vf_for_retrying()
-        keys = [('rev-1',), ('rev-2',)]
+        keys = [('rev-1',), ('rev-2',), ('rev-3',)]
         record_stream = vf.get_record_stream(keys, 'topological', False)
         record = record_stream.next()
         self.assertEqual(('rev-1',), record.key)
@@ -606,12 +607,40 @@
         record = record_stream.next()
         self.assertEqual(('rev-2',), record.key)
         self.assertEqual([1, 1, 0], reload_counter)
+        record = record_stream.next()
+        self.assertEqual(('rev-3',), record.key)
+        self.assertEqual([1, 1, 0], reload_counter)
         # Now delete all pack files, and see that we raise the right error
         for trans, name in vf._access._indices.itervalues():
             trans.delete(name)
         self.assertListRaises(errors.NoSuchFile,
             vf.get_record_stream, keys, 'topological', False)
 
+    def test_iter_lines_added_or_present_in_keys_retries(self):
+        vf, reload_counter = self.make_vf_for_retrying()
+        keys = [('rev-1',), ('rev-2',), ('rev-3',)]
+        # Unfortunately, iter_lines_added_or_present_in_keys iterates the
+        # result in random order (determined by the iteration order from a
+        # set()), so we don't have any solid way to trigger whether data is
+        # read before or after. However we tried to delete the middle node to
+        # exercise the code well.
+        # What we care about is that all lines are always yielded, but not
+        # duplicated
+        count = 0
+        reload_lines = sorted(vf.iter_lines_added_or_present_in_keys(keys))
+        self.assertEqual([1, 1, 0], reload_counter)
+        # Now do it again, to make sure the result is equivalent
+        plain_lines = sorted(vf.iter_lines_added_or_present_in_keys(keys))
+        self.assertEqual([1, 1, 0], reload_counter) # No extra reloading
+        self.assertEqual(plain_lines, reload_lines)
+        self.assertEqual(21, len(plain_lines))
+        # Now delete all pack files, and see that we raise the right error
+        for trans, name in vf._access._indices.itervalues():
+            trans.delete(name)
+        self.assertListRaises(errors.NoSuchFile,
+            vf.iter_lines_added_or_present_in_keys, keys)
+        self.assertEqual([2, 1, 1], reload_counter)
+
 
 class LowLevelKnitDataTests(TestCase):
 



More information about the bazaar-commits mailing list