Rev 3811: KnitVersionedFile.annotate() now retries when appropriate. in http://bzr.arbash-meinel.com/branches/bzr/1.9-dev/pack_retry_153786

[John Arbash Meinel]

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

------------------------------------------------------------
revno: 3811
revision-id: john at arbash-meinel.com-20081025023735-bsht1lfu6jye6z7o
parent: john at arbash-meinel.com-20081025022225-wbds6xl8t5ptod6p
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: pack_retry_153786
timestamp: Fri 2008-10-24 21:37:35 -0500
message:
  KnitVersionedFile.annotate() now retries when appropriate.
  
  As near as I can tell, this is the last code that works in terms of records,
  which is the last place that requires the indexes and packs to stay in perfect
  sync.
-------------- next part --------------
=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py	2008-10-25 02:22:25 +0000
+++ b/bzrlib/knit.py	2008-10-25 02:37:35 +0000
@@ -1451,9 +1451,6 @@
 
         :return: An iterator over (line, key).
         """
-        # TODO: We want to build in retrying, because we only hold the
-        #       'records' for the duration of this function, outside of this
-        #       function we deal in 'keys'.
         if pb is None:
             pb = progress.DummyProgress()
         keys = set(keys)
@@ -2818,14 +2815,17 @@
         if len(self._knit._fallback_vfs) > 0:
             # stacked knits can't use the fast path at present.
             return self._simple_annotate(key)
-        # TODO: We want to create retry logic at this level, between
-        #       _get_build_graph and _annotate_records, since that is the level
-        #       that we talk in terms of 'records' and not in terms of keys
-        records = self._get_build_graph(key)
-        if key in self._ghosts:
-            raise errors.RevisionNotPresent(key, self._knit)
-        self._annotate_records(records)
-        return self._annotated_lines[key]
+        while True:
+            try:
+                records = self._get_build_graph(key)
+                if key in self._ghosts:
+                    raise errors.RevisionNotPresent(key, self._knit)
+                self._annotate_records(records)
+                return self._annotated_lines[key]
+            except errors.RetryWithNewPacks, e:
+                self._knit._access.reload_or_raise(e)
+                # The cached build_details are no longer valid
+                self._all_build_details.clear()
 
     def _simple_annotate(self, key):
         """Return annotated fulltext, rediffing from the full texts.

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2008-10-25 02:22:25 +0000
+++ b/bzrlib/tests/test_knit.py	2008-10-25 02:37:35 +0000
@@ -584,6 +584,24 @@
         access.reload_or_raise(retry_exc)
         self.assertEqual([2], reload_called)
 
+    def test_annotate_retries(self):
+        vf, reload_counter = self.make_vf_for_retrying()
+        # It is a little bit bogus to annotate the Revision VF, but it works,
+        # as we have ancestry stored there
+        key = ('rev-3',)
+        reload_lines = vf.annotate(key)
+        self.assertEqual([1, 1, 0], reload_counter)
+        plain_lines = vf.annotate(key)
+        self.assertEqual([1, 1, 0], reload_counter) # No extra reloading
+        if reload_lines != plain_lines:
+            self.fail('Annotation was not identical with reloading.')
+        # Now delete the packs-in-use, which should trigger another reload, but
+        # this time we just raise an exception because we can't recover
+        for trans, name in vf._access._indices.itervalues():
+            trans.delete(name)
+        self.assertRaises(errors.NoSuchFile, vf.annotate, key)
+        self.assertEqual([2, 1, 1], reload_counter)
+
     def test__get_record_map_retries(self):
         vf, reload_counter = self.make_vf_for_retrying()
         keys = [('rev-1',), ('rev-2',), ('rev-3',)]