Rev 3803: Change _DirectPackAccess to only raise Retry when _reload_func is defined. in http://bzr.arbash-meinel.com/branches/bzr/1.9-dev/pack_retry_153786
John Arbash Meinel
john at arbash-meinel.com
Fri Oct 24 22:53:04 BST 2008
At http://bzr.arbash-meinel.com/branches/bzr/1.9-dev/pack_retry_153786
------------------------------------------------------------
revno: 3803
revision-id: john at arbash-meinel.com-20081024215259-xh1qcfeu9bdy6350
parent: john at arbash-meinel.com-20081024205359-dqeht8yxzc6b6r89
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: pack_retry_153786
timestamp: Fri 2008-10-24 16:52:59 -0500
message:
Change _DirectPackAccess to only raise Retry when _reload_func is defined.
-------------- next part --------------
=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py 2008-10-24 20:53:59 +0000
+++ b/bzrlib/knit.py 2008-10-24 21:52:59 +0000
@@ -2401,15 +2401,19 @@
class _DirectPackAccess(object):
"""Access to data in one or more packs with less translation."""
- def __init__(self, index_to_packs):
+ def __init__(self, index_to_packs, reload_func=None):
"""Create a _DirectPackAccess object.
:param index_to_packs: A dict mapping index objects to the transport
and file names for obtaining data.
+ :param reload_func: A function to call if we determine that the pack
+ files have moved and we need to reload our caches. See
+ bzrlib.repo_fmt.pack_repo.AggregateIndex for more details.
"""
self._container_writer = None
self._write_index = None
self._indices = index_to_packs
+ self._reload_func = reload_func
def add_raw_records(self, key_sizes, raw_data):
"""Add raw knit bytes to a storage area.
@@ -2467,6 +2471,10 @@
# A KeyError here indicates that someone has triggered an index
# reload, and this index has gone missing, we need to start
# over.
+ if self._reload_func is None:
+ # If we don't have a _reload_func there is nothing that can
+ # be done
+ raise
raise errors.RetryWithNewPacks(reload_occurred=True,
exc_info=sys.exc_info())
try:
@@ -2476,6 +2484,8 @@
except errors.NoSuchFile:
# A NoSuchFile error indicates that a pack file has gone
# missing on disk, we need to trigger a reload, and start over.
+ if self._reload_func is None:
+ raise
raise errors.RetryWithNewPacks(reload_occurred=False,
exc_info=sys.exc_info())
@@ -2486,6 +2496,22 @@
self._container_writer = writer
self._write_index = index
+ def reload_or_raise(self, retry_exc):
+ """Try calling the reload function, or re-raise the original exception.
+
+ This should be called after _DirectPackAccess raises a
+ RetryWithNewPacks exception. This function will handle the common logic
+ of determining when the error is fatal versus being temporary.
+ It will also make sure that the original exception is raised, rather
+ than the RetryWithNewPacks exception.
+
+ If this function returns, then the calling function should retry
+ whatever operation was being performed. Otherwise an exception will
+ be raised.
+
+ :param retry_exc: A RetryWithNewPacks exception.
+ """
+
# Deprecated, use PatienceSequenceMatcher instead
KnitSequenceMatcher = patiencediff.PatienceSequenceMatcher
=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py 2008-10-24 20:53:59 +0000
+++ b/bzrlib/tests/test_knit.py 2008-10-24 21:52:59 +0000
@@ -340,6 +340,22 @@
access.set_writer(writer, index, (transport, packname))
return access, writer
+ def make_pack_file(self):
+ """Create a pack file with 2 records."""
+ access, writer = self._get_access(packname='packname', index='foo')
+ memos = []
+ memos.extend(access.add_raw_records([('key1', 10)], '1234567890'))
+ memos.extend(access.add_raw_records([('key2', 5)], '12345'))
+ writer.end()
+ return memos
+
+ def make_reload_func(self):
+ reload_called = [0]
+ def reload():
+ reload_called[0] += 1
+ return True
+ return reload_called, reload
+
def test_read_from_several_packs(self):
access, writer = self._get_access()
memos = []
@@ -382,13 +398,12 @@
self.assertEqual(['1234567890'], list(access.get_raw_records(memos)))
def test_missing_index_raises_retry(self):
- access, writer = self._get_access(packname='packname', index='foo')
- memos = []
- memos.extend(access.add_raw_records([('key', 10)], '1234567890'))
- writer.end()
+ memos = self.make_pack_file()
transport = self.get_transport()
- # Note that the 'index' key has changed
- access = _DirectPackAccess({'bar':(transport, 'packname')})
+ reload_called, reload_func = self.make_reload_func()
+ # Note that the index key has changed from 'foo' to 'bar'
+ access = _DirectPackAccess({'bar':(transport, 'packname')},
+ reload_func=reload_func)
e = self.assertListRaises(errors.RetryWithNewPacks,
access.get_raw_records, memos)
# Because a key was passed in which does not match our index list, we
@@ -398,32 +413,66 @@
self.assertIs(e.exc_info[0], KeyError)
self.assertIsInstance(e.exc_info[1], KeyError)
+ def test_missing_index_raises_key_error_with_no_reload(self):
+ memos = self.make_pack_file()
+ transport = self.get_transport()
+ # Note that the index key has changed from 'foo' to 'bar'
+ access = _DirectPackAccess({'bar':(transport, 'packname')})
+ e = self.assertListRaises(KeyError, access.get_raw_records, memos)
+
def test_missing_file_raises_retry(self):
- access, writer = self._get_access(packname='packname', index='foo')
- memos = []
- memos.extend(access.add_raw_records([('key', 10)], '1234567890'))
- writer.end()
- transport = self.get_transport()
- # Note that the 'filename' has been changed
+ memos = self.make_pack_file()
+ transport = self.get_transport()
+ reload_called, reload_func = self.make_reload_func()
+ # Note that the 'filename' has been changed to 'different-packname'
+ access = _DirectPackAccess({'foo':(transport, 'different-packname')},
+ reload_func=reload_func)
+ e = self.assertListRaises(errors.RetryWithNewPacks,
+ access.get_raw_records, memos)
+ # The file has gone missing, so we assume we need to reload
+ self.assertFalse(e.reload_occurred)
+ self.assertIsInstance(e.exc_info, tuple)
+ self.assertIs(e.exc_info[0], errors.NoSuchFile)
+ self.assertIsInstance(e.exc_info[1], errors.NoSuchFile)
+ self.assertEqual('different-packname', e.exc_info[1].path)
+
+ def test_missing_file_raises_no_such_file_with_no_reload(self):
+ memos = self.make_pack_file()
+ transport = self.get_transport()
+ # Note that the 'filename' has been changed to 'different-packname'
access = _DirectPackAccess({'foo':(transport, 'different-packname')})
- e = self.assertListRaises(errors.RetryWithNewPacks,
+ e = self.assertListRaises(errors.NoSuchFile,
access.get_raw_records, memos)
- # The file has gone missing, so we assume we need to reload
- self.assertFalse(e.reload_occurred)
- self.assertIsInstance(e.exc_info, tuple)
- self.assertIs(e.exc_info[0], errors.NoSuchFile)
- self.assertIsInstance(e.exc_info[1], errors.NoSuchFile)
- self.assertEqual('different-packname', e.exc_info[1].path)
def test_failing_readv_raises_retry(self):
- access, writer = self._get_access(packname='packname', index='foo')
- memos = []
- memos.extend(access.add_raw_records([('key', 10)], '1234567890'))
- memos.extend(access.add_raw_records([('key', 5)], '12345'))
- writer.end()
- transport = self.get_transport()
- failing_transport = MockReadvFailingTransport(
- [transport.get_bytes('packname')])
+ memos = self.make_pack_file()
+ transport = self.get_transport()
+ failing_transport = MockReadvFailingTransport(
+ [transport.get_bytes('packname')])
+ reload_called, reload_func = self.make_reload_func()
+ access = _DirectPackAccess({'foo':(failing_transport, 'packname')},
+ reload_func=reload_func)
+ # Asking for a single record will not trigger the Mock failure
+ self.assertEqual(['1234567890'],
+ list(access.get_raw_records(memos[:1])))
+ self.assertEqual(['12345'],
+ list(access.get_raw_records(memos[1:2])))
+ # A multiple offset readv() will fail mid-way through
+ e = self.assertListRaises(errors.RetryWithNewPacks,
+ access.get_raw_records, memos)
+ # The file has gone missing, so we assume we need to reload
+ self.assertFalse(e.reload_occurred)
+ self.assertIsInstance(e.exc_info, tuple)
+ self.assertIs(e.exc_info[0], errors.NoSuchFile)
+ self.assertIsInstance(e.exc_info[1], errors.NoSuchFile)
+ self.assertEqual('packname', e.exc_info[1].path)
+
+ def test_failing_readv_raises_no_such_file_with_no_reload(self):
+ memos = self.make_pack_file()
+ transport = self.get_transport()
+ failing_transport = MockReadvFailingTransport(
+ [transport.get_bytes('packname')])
+ reload_called, reload_func = self.make_reload_func()
access = _DirectPackAccess({'foo':(failing_transport, 'packname')})
# Asking for a single record will not trigger the Mock failure
self.assertEqual(['1234567890'],
@@ -431,14 +480,8 @@
self.assertEqual(['12345'],
list(access.get_raw_records(memos[1:2])))
# A multiple offset readv() will fail mid-way through
- e = self.assertListRaises(errors.RetryWithNewPacks,
+ e = self.assertListRaises(errors.NoSuchFile,
access.get_raw_records, memos)
- # The file has gone missing, so we assume we need to reload
- self.assertFalse(e.reload_occurred)
- self.assertIsInstance(e.exc_info, tuple)
- self.assertIs(e.exc_info[0], errors.NoSuchFile)
- self.assertIsInstance(e.exc_info[1], errors.NoSuchFile)
- self.assertEqual('packname', e.exc_info[1].path)
class LowLevelKnitDataTests(TestCase):
More information about the bazaar-commits
mailing list