Rev 3804: (jam) First part of fixing #153786, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Oct 28 18:16:18 GMT 2008


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 3804
revision-id: pqm at pqm.ubuntu.com-20081028181614-p3qlghekhffb6cbu
parent: pqm at pqm.ubuntu.com-20081028094831-81he4yysmaobxb41
parent: john at arbash-meinel.com-20081028174135-h1ye1sag9l0jxjwh
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2008-10-28 18:16:14 +0000
message:
  (jam) First part of fixing #153786,
  	CombinedGraphIndex reloads index list and retries operation.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
  bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
  bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
  bzrlib/tests/test_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
  bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
    ------------------------------------------------------------
    revno: 3789.1.11
    revision-id: john at arbash-meinel.com-20081028174135-h1ye1sag9l0jxjwh
    parent: john at arbash-meinel.com-20081028174004-ygf3v01q2iivzrbl
    parent: pqm at pqm.ubuntu.com-20081027195553-876pyjww9zmjqj87
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786-1
    timestamp: Tue 2008-10-28 12:41:35 -0500
    message:
      Merge in bzr.dev 3801, resolve NEWS
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/_walkdirs_win32.pyx     _walkdirs_win32.pyx-20080716220454-kweh3tgxez5dvw2l-2
      bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
      bzrlib/btree_index.py          index.py-20080624222253-p0x5f92uyh5hw734-7
      bzrlib/builtins.py             builtins.py-20050830033751-fc01482b9ca23183
      bzrlib/chunk_writer.py         chunk_writer.py-20080630234519-6ggn4id17nipovny-1
      bzrlib/help_topics/en/hooks.txt hooks.txt-20070830033044-xxu2rced13f72dka-1
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/plugin.py               plugin.py-20050622060424-829b654519533d69
      bzrlib/plugins/launchpad/account.py account.py-20071011033320-50y6vfftywf4yllw-1
      bzrlib/plugins/launchpad/test_account.py test_account.py-20071011033320-50y6vfftywf4yllw-2
      bzrlib/python-compat.h         pythoncompat.h-20080924041409-9kvi0fgtuuqp743j-1
      bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
      bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
      bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/blackbox/test_missing.py test_missing.py-20051211212735-a2cf4c1840bb84c4
      bzrlib/tests/branch_implementations/test_stacking.py test_stacking.py-20080214020755-msjlkb7urobwly0f-1
      bzrlib/tests/test_btree_index.py test_index.py-20080624222253-p0x5f92uyh5hw734-13
      bzrlib/tests/test_chunk_writer.py test_chunk_writer.py-20080630234519-6ggn4id17nipovny-2
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
      bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
      bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
      bzrlib/win32utils.py           win32console.py-20051021033308-123c6c929d04973d
      doc/en/user-guide/branching_a_project.txt branching_a_project.-20071122141511-0knao2lklsdsvb1q-2
      doc/en/user-guide/core_concepts.txt core_concepts.txt-20071114035000-q36a9h57ps06uvnl-2
      doc/en/user-guide/using_checkouts.txt using_checkouts.txt-20071123055134-k5x4ekduci2lbn36-4
    ------------------------------------------------------------
    revno: 3789.1.10
    revision-id: john at arbash-meinel.com-20081028174004-ygf3v01q2iivzrbl
    parent: john at arbash-meinel.com-20081023210643-6pxsgdybl89n0tz9
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786-1
    timestamp: Tue 2008-10-28 12:40:04 -0500
    message:
      Review comments from Martin.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
    ------------------------------------------------------------
    revno: 3789.1.9
    revision-id: john at arbash-meinel.com-20081023210643-6pxsgdybl89n0tz9
    parent: john at arbash-meinel.com-20081023210428-b70cl16n1h54gwa8
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 16:06:43 -0500
    message:
      NEWS about partial fix for bug #153786
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
    ------------------------------------------------------------
    revno: 3789.1.8
    revision-id: john at arbash-meinel.com-20081023210428-b70cl16n1h54gwa8
    parent: john at arbash-meinel.com-20081023205350-ynduycv3bh5qsup3
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 16:04:28 -0500
    message:
      Change the api of reload_pack_names().
      
      I sort of preferred when it returned what changed, but most callers really don't
      have any way to do anything with the details. So instead, we just return
      whether or not something actually changed.
    modified:
      bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
      bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
    ------------------------------------------------------------
    revno: 3789.1.7
    revision-id: john at arbash-meinel.com-20081023205350-ynduycv3bh5qsup3
    parent: john at arbash-meinel.com-20081023205058-z80mz4hp6o25v63w
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 15:53:50 -0500
    message:
      CombinedGraphIndex.validate() will now reload.
    modified:
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
    ------------------------------------------------------------
    revno: 3789.1.6
    revision-id: john at arbash-meinel.com-20081023205058-z80mz4hp6o25v63w
    parent: john at arbash-meinel.com-20081023203400-ftvlkbrx2zc8lww5
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 15:50:58 -0500
    message:
      CombinedGraphIndex.iter_entries_prefix can now reload when needed.
    modified:
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
    ------------------------------------------------------------
    revno: 3789.1.5
    revision-id: john at arbash-meinel.com-20081023203400-ftvlkbrx2zc8lww5
    parent: john at arbash-meinel.com-20081023202849-2k0p95mvwrhcjze9
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 15:34:00 -0500
    message:
      CombinedGraphIndex.iter_all_entries() can now reload when needed.
    modified:
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
    ------------------------------------------------------------
    revno: 3789.1.4
    revision-id: john at arbash-meinel.com-20081023202849-2k0p95mvwrhcjze9
    parent: john at arbash-meinel.com-20081023200801-72xbksw2uyj8cdmq
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 15:28:49 -0500
    message:
      CombinedGraphIndex.iter_entries() is now able to reload on request.
    modified:
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
    ------------------------------------------------------------
    revno: 3789.1.3
    revision-id: john at arbash-meinel.com-20081023200801-72xbksw2uyj8cdmq
    parent: john at arbash-meinel.com-20081023193959-xdytlck4ym0vr6lt
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 15:08:01 -0500
    message:
      CombinedGraphIndex can now reload when calling key_count().
    modified:
      bzrlib/index.py                index.py-20070712131115-lolkarso50vjr64s-1
      bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
      bzrlib/tests/test_index.py     test_index.py-20070712131115-lolkarso50vjr64s-2
    ------------------------------------------------------------
    revno: 3789.1.2
    revision-id: john at arbash-meinel.com-20081023193959-xdytlck4ym0vr6lt
    parent: john at arbash-meinel.com-20081023191146-jiz88y5og2koaahs
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 14:39:59 -0500
    message:
      Add RepositoryPackCollection.reload_pack_names()
      
      This refactors the _save_pack_names code into helper functions, and
      exposes a public member for outside entities to call to refresh the
      list. It updates the internal names and also re-updates the various
      indexes.
    modified:
      bzrlib/repofmt/pack_repo.py    pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
      bzrlib/tests/test_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
      bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
    ------------------------------------------------------------
    revno: 3789.1.1
    revision-id: john at arbash-meinel.com-20081023191146-jiz88y5og2koaahs
    parent: pqm at pqm.ubuntu.com-20081021231845-k119hl1icewguq50
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: pack_retry_153786
    timestamp: Thu 2008-10-23 14:11:46 -0500
    message:
      add the failing acceptance test for the first portion.
      
      When going through 'get_parent_map', if the index is missing, we should have
      reloaded the index list without the calling code needing to know anything.
    modified:
      bzrlib/tests/test_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
=== modified file 'NEWS'
--- a/NEWS	2008-10-27 08:02:47 +0000
+++ b/NEWS	2008-10-28 17:41:35 +0000
@@ -63,6 +63,12 @@
     * Some compatibility fixes for building the extensions with MSVC and
       for python2.4. (John Arbash Meinel, #277484)
 
+    * The index logic is now able to reload the list of pack files if and
+      index ends up disappearing. We still don't reload if the pack data
+      itself goes missing after checking the index. This bug appears as a
+      transient failure (file not found) when another process is writing
+      to the repository.  (John Arbash Meinel, #153786)
+
   DOCUMENTATION:
 
   API CHANGES:

=== modified file 'bzrlib/index.py'
--- a/bzrlib/index.py	2008-10-15 21:40:03 +0000
+++ b/bzrlib/index.py	2008-10-28 17:41:35 +0000
@@ -27,6 +27,7 @@
 from bisect import bisect_right
 from cStringIO import StringIO
 import re
+import sys
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
@@ -1118,12 +1119,16 @@
     in the index list.
     """
 
-    def __init__(self, indices):
+    def __init__(self, indices, reload_func=None):
         """Create a CombinedGraphIndex backed by indices.
 
         :param indices: An ordered list of indices to query for data.
+        :param reload_func: A function to call if we find we are missing an
+            index. Should have the form reload_func() => True/False to indicate
+            if reloading actually changed anything.
         """
         self._indices = indices
+        self._reload_func = reload_func
 
     def __repr__(self):
         return "%s(%s)" % (
@@ -1181,11 +1186,16 @@
             the most efficient order for the index.
         """
         seen_keys = set()
-        for index in self._indices:
-            for node in index.iter_all_entries():
-                if node[1] not in seen_keys:
-                    yield node
-                    seen_keys.add(node[1])
+        while True:
+            try:
+                for index in self._indices:
+                    for node in index.iter_all_entries():
+                        if node[1] not in seen_keys:
+                            yield node
+                            seen_keys.add(node[1])
+                return
+            except errors.NoSuchFile:
+                self._reload_or_raise()
 
     def iter_entries(self, keys):
         """Iterate over keys within the index.
@@ -1199,12 +1209,17 @@
             efficient order for the index.
         """
         keys = set(keys)
-        for index in self._indices:
-            if not keys:
+        while True:
+            try:
+                for index in self._indices:
+                    if not keys:
+                        return
+                    for node in index.iter_entries(keys):
+                        keys.remove(node[1])
+                        yield node
                 return
-            for node in index.iter_entries(keys):
-                keys.remove(node[1])
-                yield node
+            except errors.NoSuchFile:
+                self._reload_or_raise()
 
     def iter_entries_prefix(self, keys):
         """Iterate over keys within the index using prefix matching.
@@ -1230,27 +1245,58 @@
         if not keys:
             return
         seen_keys = set()
-        for index in self._indices:
-            for node in index.iter_entries_prefix(keys):
-                if node[1] in seen_keys:
-                    continue
-                seen_keys.add(node[1])
-                yield node
+        while True:
+            try:
+                for index in self._indices:
+                    for node in index.iter_entries_prefix(keys):
+                        if node[1] in seen_keys:
+                            continue
+                        seen_keys.add(node[1])
+                        yield node
+                return
+            except errors.NoSuchFile:
+                self._reload_or_raise()
 
     def key_count(self):
         """Return an estimate of the number of keys in this index.
-        
+
         For CombinedGraphIndex this is approximated by the sum of the keys of
         the child indices. As child indices may have duplicate keys this can
         have a maximum error of the number of child indices * largest number of
         keys in any index.
         """
-        return sum((index.key_count() for index in self._indices), 0)
+        while True:
+            try:
+                return sum((index.key_count() for index in self._indices), 0)
+            except errors.NoSuchFile:
+                self._reload_or_raise()
+
+    def _reload_or_raise(self):
+        """We just got a NoSuchFile exception.
+
+        Try to reload the indices, if it fails, just raise the current
+        exception.
+        """
+        if self._reload_func is None:
+            raise
+        exc_type, exc_value, exc_traceback = sys.exc_info()
+        trace.mutter('Trying to reload after getting exception: %s',
+                     exc_value)
+        if not self._reload_func():
+            # We tried to reload, but nothing changed, so we fail anyway
+            trace.mutter('_reload_func indicated nothing has changed.'
+                         ' Raising original exception.')
+            raise exc_type, exc_value, exc_traceback
 
     def validate(self):
         """Validate that everything in the index can be accessed."""
-        for index in self._indices:
-            index.validate()
+        while True:
+            try:
+                for index in self._indices:
+                    index.validate()
+                return
+            except errors.NoSuchFile:
+                self._reload_or_raise()
 
 
 class InMemoryGraphIndex(GraphIndexBuilder):

=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py	2008-10-26 15:31:06 +0000
+++ b/bzrlib/repofmt/pack_repo.py	2008-10-28 17:41:35 +0000
@@ -452,10 +452,16 @@
     # XXX: Probably 'can be written to' could/should be separated from 'acts
     # like a knit index' -- mbp 20071024
 
-    def __init__(self):
-        """Create an AggregateIndex."""
+    def __init__(self, reload_func=None):
+        """Create an AggregateIndex.
+
+        :param reload_func: A function to call if we find we are missing an
+            index. Should have the form reload_func() => True if the list of
+            active pack files has changed.
+        """
+        self._reload_func = reload_func
         self.index_to_pack = {}
-        self.combined_index = CombinedGraphIndex([])
+        self.combined_index = CombinedGraphIndex([], reload_func=reload_func)
         self.data_access = _DirectPackAccess(self.index_to_pack)
         self.add_callback = None
 
@@ -1159,10 +1165,10 @@
         # when a pack is being created by this object, the state of that pack.
         self._new_pack = None
         # aggregated revision index data
-        self.revision_index = AggregateIndex()
-        self.inventory_index = AggregateIndex()
-        self.text_index = AggregateIndex()
-        self.signature_index = AggregateIndex()
+        self.revision_index = AggregateIndex(self.reload_pack_names)
+        self.inventory_index = AggregateIndex(self.reload_pack_names)
+        self.text_index = AggregateIndex(self.reload_pack_names)
+        self.signature_index = AggregateIndex(self.reload_pack_names)
 
     def add_pack_to_memory(self, pack):
         """Make a Pack object available to the repository to satisfy queries.
@@ -1564,51 +1570,55 @@
         """Release the mutex around the pack-names index."""
         self.repo.control_files.unlock()
 
-    def _save_pack_names(self, clear_obsolete_packs=False):
-        """Save the list of packs.
-
-        This will take out the mutex around the pack names list for the
-        duration of the method call. If concurrent updates have been made, a
-        three-way merge between the current list and the current in memory list
-        is performed.
-
-        :param clear_obsolete_packs: If True, clear out the contents of the
-            obsolete_packs directory.
-        """
-        self.lock_names()
-        try:
-            builder = self._index_builder_class()
-            # load the disk nodes across
-            disk_nodes = set()
-            for index, key, value in self._iter_disk_pack_index():
-                disk_nodes.add((key, value))
-            # do a two-way diff against our original content
-            current_nodes = set()
-            for name, sizes in self._names.iteritems():
-                current_nodes.add(
-                    ((name, ), ' '.join(str(size) for size in sizes)))
-            deleted_nodes = self._packs_at_load - current_nodes
-            new_nodes = current_nodes - self._packs_at_load
-            disk_nodes.difference_update(deleted_nodes)
-            disk_nodes.update(new_nodes)
-            # TODO: handle same-name, index-size-changes here - 
-            # e.g. use the value from disk, not ours, *unless* we're the one
-            # changing it.
-            for key, value in disk_nodes:
-                builder.add_node(key, value)
-            self.transport.put_file('pack-names', builder.finish(),
-                mode=self.repo.bzrdir._get_file_mode())
-            # move the baseline forward
-            self._packs_at_load = disk_nodes
-            if clear_obsolete_packs:
-                self._clear_obsolete_packs()
-        finally:
-            self._unlock_names()
-        # synchronise the memory packs list with what we just wrote:
+    def _diff_pack_names(self):
+        """Read the pack names from disk, and compare it to the one in memory.
+
+        :return: (disk_nodes, deleted_nodes, new_nodes)
+            disk_nodes    The final set of nodes that should be referenced
+            deleted_nodes Nodes which have been removed from when we started
+            new_nodes     Nodes that are newly introduced
+        """
+        # load the disk nodes across
+        disk_nodes = set()
+        for index, key, value in self._iter_disk_pack_index():
+            disk_nodes.add((key, value))
+
+        # do a two-way diff against our original content
+        current_nodes = set()
+        for name, sizes in self._names.iteritems():
+            current_nodes.add(
+                ((name, ), ' '.join(str(size) for size in sizes)))
+
+        # Packs no longer present in the repository, which were present when we
+        # locked the repository
+        deleted_nodes = self._packs_at_load - current_nodes
+        # Packs which this process is adding
+        new_nodes = current_nodes - self._packs_at_load
+
+        # Update the disk_nodes set to include the ones we are adding, and
+        # remove the ones which were removed by someone else
+        disk_nodes.difference_update(deleted_nodes)
+        disk_nodes.update(new_nodes)
+
+        return disk_nodes, deleted_nodes, new_nodes
+
+    def _syncronize_pack_names_from_disk_nodes(self, disk_nodes):
+        """Given the correct set of pack files, update our saved info.
+
+        :return: (removed, added, modified)
+            removed     pack names removed from self._names
+            added       pack names added to self._names
+            modified    pack names that had changed value
+        """
+        removed = []
+        added = []
+        modified = []
+        ## self._packs_at_load = disk_nodes
         new_names = dict(disk_nodes)
         # drop no longer present nodes
         for pack in self.all_packs():
             if (pack.name,) not in new_names:
+                removed.append(pack.name)
                 self._remove_pack_from_memory(pack)
         # add new nodes/refresh existing ones
         for key, value in disk_nodes:
@@ -1628,10 +1638,61 @@
                     self._remove_pack_from_memory(self.get_pack_by_name(name))
                     self._names[name] = sizes
                     self.get_pack_by_name(name)
+                    modified.append(name)
             else:
                 # new
                 self._names[name] = sizes
                 self.get_pack_by_name(name)
+                added.append(name)
+        return removed, added, modified
+
+    def _save_pack_names(self, clear_obsolete_packs=False):
+        """Save the list of packs.
+
+        This will take out the mutex around the pack names list for the
+        duration of the method call. If concurrent updates have been made, a
+        three-way merge between the current list and the current in memory list
+        is performed.
+
+        :param clear_obsolete_packs: If True, clear out the contents of the
+            obsolete_packs directory.
+        """
+        self.lock_names()
+        try:
+            builder = self._index_builder_class()
+            disk_nodes, deleted_nodes, new_nodes = self._diff_pack_names()
+            # TODO: handle same-name, index-size-changes here - 
+            # e.g. use the value from disk, not ours, *unless* we're the one
+            # changing it.
+            for key, value in disk_nodes:
+                builder.add_node(key, value)
+            self.transport.put_file('pack-names', builder.finish(),
+                mode=self.repo.bzrdir._get_file_mode())
+            # move the baseline forward
+            self._packs_at_load = disk_nodes
+            if clear_obsolete_packs:
+                self._clear_obsolete_packs()
+        finally:
+            self._unlock_names()
+        # synchronise the memory packs list with what we just wrote:
+        self._syncronize_pack_names_from_disk_nodes(disk_nodes)
+
+    def reload_pack_names(self):
+        """Sync our pack listing with what is present in the repository.
+
+        This should be called when we find out that something we thought was
+        present is now missing. This happens when another process re-packs the
+        repository, etc.
+        """
+        # This is functionally similar to _save_pack_names, but we don't write
+        # out the new value.
+        disk_nodes, _, _ = self._diff_pack_names()
+        self._packs_at_load = disk_nodes
+        (removed, added,
+         modified) = self._syncronize_pack_names_from_disk_nodes(disk_nodes)
+        if removed or added or modified:
+            return True
+        return False
 
     def _clear_obsolete_packs(self):
         """Delete everything from the obsolete-packs directory.

=== modified file 'bzrlib/tests/test_index.py'
--- a/bzrlib/tests/test_index.py	2008-10-15 21:40:03 +0000
+++ b/bzrlib/tests/test_index.py	2008-10-28 17:41:35 +0000
@@ -934,6 +934,39 @@
         size = trans.put_file(name, stream)
         return GraphIndex(trans, name, size)
 
+    def make_combined_index_with_missing(self, missing=['1', '2']):
+        """Create a CombinedGraphIndex which will have missing indexes.
+
+        This creates a CGI which thinks it has 2 indexes, however they have
+        been deleted. If CGI._reload_func() is called, then it will repopulate
+        with a new index.
+
+        :param missing: The underlying indexes to delete
+        :return: (CombinedGraphIndex, reload_counter)
+        """
+        index1 = self.make_index('1', nodes=[(('1',), '', ())])
+        index2 = self.make_index('2', nodes=[(('2',), '', ())])
+        index3 = self.make_index('3', nodes=[
+            (('1',), '', ()),
+            (('2',), '', ())])
+
+        # total_reloads, num_changed, num_unchanged
+        reload_counter = [0, 0, 0]
+        def reload():
+            reload_counter[0] += 1
+            new_indices = [index3]
+            if index._indices == new_indices:
+                reload_counter[2] += 1
+                return False
+            reload_counter[1] += 1
+            index._indices[:] = new_indices
+            return True
+        index = CombinedGraphIndex([index1, index2], reload_func=reload)
+        trans = self.get_transport()
+        for fname in missing:
+            trans.delete(fname)
+        return index, reload_counter
+
     def test_open_missing_index_no_error(self):
         trans = self.get_transport()
         index1 = GraphIndex(trans, 'missing', 100)
@@ -1077,6 +1110,136 @@
         index = CombinedGraphIndex([])
         index.validate()
 
+    def test_key_count_reloads(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        self.assertEqual(2, index.key_count())
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_key_count_no_reload(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index._reload_func = None
+        # Without a _reload_func we just raise the exception
+        self.assertRaises(errors.NoSuchFile, index.key_count)
+
+    def test_key_count_reloads_and_fails(self):
+        # We have deleted all underlying indexes, so we will try to reload, but
+        # still fail. This is mostly to test we don't get stuck in an infinite
+        # loop trying to reload
+        index, reload_counter = self.make_combined_index_with_missing(
+                                    ['1', '2', '3'])
+        self.assertRaises(errors.NoSuchFile, index.key_count)
+        self.assertEqual([2, 1, 1], reload_counter)
+
+    def test_iter_entries_reloads(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        result = list(index.iter_entries([('1',), ('2',), ('3',)]))
+        index3 = index._indices[0]
+        self.assertEqual([(index3, ('1',), ''), (index3, ('2',), '')],
+                         result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_entries_reloads_midway(self):
+        # The first index still looks present, so we get interrupted mid-way
+        # through
+        index, reload_counter = self.make_combined_index_with_missing(['2'])
+        index1, index2 = index._indices
+        result = list(index.iter_entries([('1',), ('2',), ('3',)]))
+        index3 = index._indices[0]
+        # We had already yielded '1', so we just go on to the next, we should
+        # not yield '1' twice.
+        self.assertEqual([(index1, ('1',), ''), (index3, ('2',), '')],
+                         result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_entries_no_reload(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index._reload_func = None
+        # Without a _reload_func we just raise the exception
+        self.assertListRaises(errors.NoSuchFile, index.iter_entries, [('3',)])
+
+    def test_iter_entries_reloads_and_fails(self):
+        index, reload_counter = self.make_combined_index_with_missing(
+                                    ['1', '2', '3'])
+        self.assertListRaises(errors.NoSuchFile, index.iter_entries, [('3',)])
+        self.assertEqual([2, 1, 1], reload_counter)
+
+    def test_iter_all_entries_reloads(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        result = list(index.iter_all_entries())
+        index3 = index._indices[0]
+        self.assertEqual([(index3, ('1',), ''), (index3, ('2',), '')],
+                         result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_all_entries_reloads_midway(self):
+        index, reload_counter = self.make_combined_index_with_missing(['2'])
+        index1, index2 = index._indices
+        result = list(index.iter_all_entries())
+        index3 = index._indices[0]
+        # We had already yielded '1', so we just go on to the next, we should
+        # not yield '1' twice.
+        self.assertEqual([(index1, ('1',), ''), (index3, ('2',), '')],
+                         result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_all_entries_no_reload(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index._reload_func = None
+        self.assertListRaises(errors.NoSuchFile, index.iter_all_entries)
+
+    def test_iter_all_entries_reloads_and_fails(self):
+        index, reload_counter = self.make_combined_index_with_missing(
+                                    ['1', '2', '3'])
+        self.assertListRaises(errors.NoSuchFile, index.iter_all_entries)
+
+    def test_iter_entries_prefix_reloads(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        result = list(index.iter_entries_prefix([('1',)]))
+        index3 = index._indices[0]
+        self.assertEqual([(index3, ('1',), '')], result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_entries_prefix_reloads_midway(self):
+        index, reload_counter = self.make_combined_index_with_missing(['2'])
+        index1, index2 = index._indices
+        result = list(index.iter_entries_prefix([('1',)]))
+        index3 = index._indices[0]
+        # We had already yielded '1', so we just go on to the next, we should
+        # not yield '1' twice.
+        self.assertEqual([(index1, ('1',), '')], result)
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_iter_entries_prefix_no_reload(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index._reload_func = None
+        self.assertListRaises(errors.NoSuchFile, index.iter_entries_prefix,
+                                                 [('1',)])
+
+    def test_iter_entries_prefix_reloads_and_fails(self):
+        index, reload_counter = self.make_combined_index_with_missing(
+                                    ['1', '2', '3'])
+        self.assertListRaises(errors.NoSuchFile, index.iter_entries_prefix,
+                                                 [('1',)])
+
+    def test_validate_reloads(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index.validate()
+        self.assertEqual([1, 1, 0], reload_counter)
+
+    def test_validate_reloads_midway(self):
+        index, reload_counter = self.make_combined_index_with_missing(['2'])
+        index.validate()
+
+    def test_validate_no_reload(self):
+        index, reload_counter = self.make_combined_index_with_missing()
+        index._reload_func = None
+        self.assertRaises(errors.NoSuchFile, index.validate)
+
+    def test_validate_reloads_and_fails(self):
+        index, reload_counter = self.make_combined_index_with_missing(
+                                    ['1', '2', '3'])
+        self.assertRaises(errors.NoSuchFile, index.validate)
+
 
 class TestInMemoryGraphIndex(TestCaseWithMemoryTransport):
 

=== modified file 'bzrlib/tests/test_pack_repository.py'
--- a/bzrlib/tests/test_pack_repository.py	2008-09-25 22:25:09 +0000
+++ b/bzrlib/tests/test_pack_repository.py	2008-10-23 19:39:59 +0000
@@ -395,6 +395,25 @@
         finally:
             r1.unlock()
 
+    def test_concurrent_pack_triggers_reload(self):
+        # create 2 packs, which we will then collapse
+        tree = self.make_branch_and_tree('tree')
+        tree.lock_write()
+        try:
+            rev1 = tree.commit('one')
+            rev2 = tree.commit('two')
+            r2 = repository.Repository.open('tree')
+            r2.lock_read()
+            try:
+                # Now r2 has read the pack-names file, but will need to reload
+                # it after r1 has repacked
+                tree.branch.repository.pack()
+                self.assertEqual({rev2:(rev1,)}, r2.get_parent_map([rev2]))
+            finally:
+                r2.unlock()
+        finally:
+            tree.unlock()
+
     def test_lock_write_does_not_physically_lock(self):
         repo = self.make_repository('.', format=self.get_format())
         repo.lock_write()

=== modified file 'bzrlib/tests/test_repository.py'
--- a/bzrlib/tests/test_repository.py	2008-10-27 07:02:58 +0000
+++ b/bzrlib/tests/test_repository.py	2008-10-28 17:41:35 +0000
@@ -923,6 +923,54 @@
         # and the same instance should be returned on successive calls.
         self.assertTrue(pack_1 is packs.get_pack_by_name(name))
 
+    def test_reload_pack_names_new_entry(self):
+        tree = self.make_branch_and_tree('.')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        rev1 = tree.commit('one')
+        rev2 = tree.commit('two')
+        r = repository.Repository.open('.')
+        r.lock_read()
+        self.addCleanup(r.unlock)
+        packs = r._pack_collection
+        packs.ensure_loaded()
+        names = packs.names()
+        # Add a new pack file into the repository
+        rev3 = tree.commit('three')
+        new_names = tree.branch.repository._pack_collection.names()
+        new_name = set(new_names).difference(names)
+        self.assertEqual(1, len(new_name))
+        new_name = new_name.pop()
+        # The old collection hasn't noticed yet
+        self.assertEqual(names, packs.names())
+        self.assertTrue(packs.reload_pack_names())
+        self.assertEqual(new_names, packs.names())
+        # And the repository can access the new revision
+        self.assertEqual({rev3:(rev2,)}, r.get_parent_map([rev3]))
+        self.assertFalse(packs.reload_pack_names())
+
+    def test_reload_pack_names_added_and_removed(self):
+        tree = self.make_branch_and_tree('.')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        rev1 = tree.commit('one')
+        rev2 = tree.commit('two')
+        r = repository.Repository.open('.')
+        r.lock_read()
+        self.addCleanup(r.unlock)
+        packs = r._pack_collection
+        packs.ensure_loaded()
+        names = packs.names()
+        # Now repack the whole thing
+        tree.branch.repository.pack()
+        new_names = tree.branch.repository._pack_collection.names()
+        # The other collection hasn't noticed yet
+        self.assertEqual(names, packs.names())
+        self.assertTrue(packs.reload_pack_names())
+        self.assertEqual(new_names, packs.names())
+        self.assertEqual({rev2:(rev1,)}, r.get_parent_map([rev2]))
+        self.assertFalse(packs.reload_pack_names())
+
 
 class TestPack(TestCaseWithTransport):
     """Tests for the Pack object."""




More information about the bazaar-commits mailing list