Rev 5119: (spiv) Add reload-and-retry logic to RepositoryPackCollection.pack when a in file:///home/pqm/archives/thelove/bzr/2.2/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/2.2/

------------------------------------------------------------
revno: 5119 [merge]
revision-id: pqm at pqm.ubuntu.com-20110120005245-9i7zq3f8ujoy355g
parent: pqm at pqm.ubuntu.com-20110114230435-ww0f1p7y9rgdr7nn
parent: andrew.bennetts at canonical.com-20110119223447-2w8drxa2kq5mmymo
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.2
timestamp: Thu 2011-01-20 00:52:45 +0000
message:
  (spiv) Add reload-and-retry logic to RepositoryPackCollection.pack when a
   concurrent pack happens. (#701940) (Andrew Bennetts)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
  bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
=== modified file 'NEWS'
--- a/NEWS	2011-01-14 21:36:35 +0000
+++ b/NEWS	2011-01-19 22:34:47 +0000
@@ -28,6 +28,10 @@
 * Avoid UnicodeDecodeError in ``bzr add`` with multiple files under a non-ascii
   path on windows from symlink support addition. (Martin [gz], #686611)
 
+* Fix a crash during ``RepositoryPackCollection.pack`` caused by a
+  concurrent repository pack operation.  This was particularly affecting
+  ``bzr-svn`` users.  (Andrew Bennetts, #701940)
+
 * Correctly resolve content (and path) conflicts for files in subdirs.
   (Vincent Ladeuil, #660935)
 

=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py	2010-05-11 08:36:16 +0000
+++ b/bzrlib/repofmt/pack_repo.py	2011-01-19 22:26:56 +0000
@@ -1574,6 +1574,20 @@
         mutter('Packing repository %s, which has %d pack files, '
             'containing %d revisions with hint %r.', self, total_packs,
             total_revisions, hint)
+        while True:
+            try:
+                self._try_pack_operations(hint)
+            except RetryPackOperations:
+                continue
+            break
+
+        if clean_obsolete_packs:
+            self._clear_obsolete_packs()
+
+    def _try_pack_operations(self, hint):
+        """Calculate the pack operations based on the hint (if any), and
+        execute them.
+        """
         # determine which packs need changing
         pack_operations = [[0, []]]
         for pack in self.all_packs():
@@ -1582,10 +1596,8 @@
                 # or this pack was included in the hint.
                 pack_operations[-1][0] += pack.get_revision_count()
                 pack_operations[-1][1].append(pack)
-        self._execute_pack_operations(pack_operations, OptimisingPacker)
-
-        if clean_obsolete_packs:
-            self._clear_obsolete_packs()
+        self._execute_pack_operations(pack_operations, OptimisingPacker,
+            reload_func=self._restart_pack_operations)
 
     def plan_autopack_combinations(self, existing_packs, pack_distribution):
         """Plan a pack operation.
@@ -2042,6 +2054,14 @@
             raise
         raise errors.RetryAutopack(self.repo, False, sys.exc_info())
 
+    def _restart_pack_operations(self):
+        """Reload the pack names list, and restart the autopack code."""
+        if not self.reload_pack_names():
+            # Re-raise the original exception, because something went missing
+            # and a restart didn't find it
+            raise
+        raise RetryPackOperations(self.repo, False, sys.exc_info())
+
     def _clear_obsolete_packs(self, preserve=None):
         """Delete everything from the obsolete-packs directory.
 
@@ -2933,3 +2953,17 @@
         return ("Development repository format, currently the same as "
             "1.6.1-subtree with B+Tree indices.\n")
 
+
+class RetryPackOperations(errors.RetryWithNewPacks):
+    """Raised when we are packing and we find a missing file.
+
+    Meant as a signaling exception, to tell the RepositoryPackCollection.pack
+    code it should try again.
+    """
+
+    internal_error = True
+
+    _fmt = ("Pack files have changed, reload and try pack again."
+            " context: %(context)s %(orig_error)s")
+
+

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2010-06-20 11:18:38 +0000
+++ b/bzrlib/tests/test_knit.py	2011-01-19 22:06:31 +0000
@@ -338,6 +338,33 @@
         writer.end()
         return memos
 
+    def test_pack_collection_pack_retries(self):
+        """An explicit pack of a pack collection succeeds even when a
+        concurrent pack happens.
+        """
+        builder = self.make_branch_builder('.')
+        builder.start_series()
+        builder.build_snapshot('rev-1', None, [
+            ('add', ('', 'root-id', 'directory', None)),
+            ('add', ('file', 'file-id', 'file', 'content\nrev 1\n')),
+            ])
+        builder.build_snapshot('rev-2', ['rev-1'], [
+            ('modify', ('file-id', 'content\nrev 2\n')),
+            ])
+        builder.build_snapshot('rev-3', ['rev-2'], [
+            ('modify', ('file-id', 'content\nrev 3\n')),
+            ])
+        self.addCleanup(builder.finish_series)
+        b = builder.get_branch()
+        self.addCleanup(b.lock_write().unlock)
+        repo = b.repository
+        collection = repo._pack_collection
+        # Concurrently repack the repo.
+        reopened_repo = repo.bzrdir.open_repository()
+        reopened_repo.pack()
+        # Pack the new pack.
+        collection.pack()
+
     def make_vf_for_retrying(self):
         """Create 3 packs and a reload function.