diff options
author | Lars Wirzenius <liw@liw.fi> | 2013-11-06 20:35:12 +0000 |
---|---|---|
committer | Lars Wirzenius <liw@liw.fi> | 2013-11-06 20:35:12 +0000 |
commit | 95bdc966cf8abe4e79ce32a8accd2e323b487cc1 (patch) | |
tree | 68718f614ffe84d1ac0b697b5badaab2199c07f6 | |
parent | 267b049865cc239e98847ea013dd9c88abe3f8e6 (diff) | |
parent | 1d31b862b1f0fc6732d0c2811b7775025039db89 (diff) | |
download | larch-95bdc966cf8abe4e79ce32a8accd2e323b487cc1.tar.gz |
Merge branch 'liw/refcount-keyerror'
-rw-r--r-- | NEWS | 9 | ||||
-rw-r--r-- | larch/refcountstore.py | 27 | ||||
-rw-r--r-- | larch/refcountstore_tests.py | 15 | ||||
-rwxr-xr-x | refcount-speed | 14 |
4 files changed, 32 insertions, 33 deletions
@@ -7,6 +7,15 @@ copy-on-write B-tree, designed by Ohad Rodeh. Version 1.UNRELEASED ------------------ +* Serious bug fixed: the "KeyError" crash for reference counts. This + was false memory use optimisation, which triggered a rare bug in + related code. Repeatable test case by Rob Kendrick, and helpful + analysis by Itamar Turing-Trauring. + +* Serious bug fixed: another "node missing" bug. This crash was + caused by a bug that overwrote on-disk reference count groups + with zeroes. Repeatable test case by Rob Kendrick. + * Fixes to fsck from Antoine Brenner. Version 1.20130808 diff --git a/larch/refcountstore.py b/larch/refcountstore.py index 0aea53e..6a9411f 100644 --- a/larch/refcountstore.py +++ b/larch/refcountstore.py @@ -24,12 +24,10 @@ import tracing import larch -def encode_refcounts(refcounts, start_id, how_many): +def encode_refcounts(refcounts, start_id, how_many, keys): fmt = '!QH' + 'H' * how_many args = [start_id, how_many] + ([0] * how_many) - keys = set(refcounts.keys()) - wanted = set(range(start_id, start_id + how_many)) - for key in wanted.intersection(keys): + for key in keys: args[2 + key - start_id] = refcounts[key] return struct.pack(fmt, *args) @@ -78,11 +76,7 @@ class RefcountStore(object): def set_refcount(self, node_id, refcount): '''Set the reference count for a given node.''' tracing.trace('setting refcoutn for %s to %s' % (node_id, refcount)) - if refcount == 0: - if node_id in self.refcounts: - del self.refcounts[node_id] - else: - self.refcounts[node_id] = refcount + self.refcounts[node_id] = refcount self.dirty.add(node_id) def save_refcounts(self): @@ -95,13 +89,19 @@ class RefcountStore(object): if not self.node_store.journal.exists(dirname): self.node_store.journal.makedirs(dirname) ids = sorted(self.dirty) + all_ids_in_memory = set(self.refcounts.keys()) for start_id in range(self._start_id(ids[0]), self._start_id(ids[-1]) + 1, self.per_group): - encoded = encode_refcounts(self.refcounts, start_id, - self.per_group) - filename = self._group_filename(start_id) - self.node_store.journal.overwrite_file(filename, encoded) + + all_ids_in_group = set( + range(start_id, start_id + self.per_group)) + keys = all_ids_in_group.intersection(all_ids_in_memory) + if keys: + encoded = encode_refcounts( + self.refcounts, start_id, self.per_group, keys) + filename = self._group_filename(start_id) + self.node_store.journal.overwrite_file(filename, encoded) # We re-initialize these so that they don't grow indefinitely. self.refcounts = dict() @@ -119,4 +119,3 @@ class RefcountStore(object): def _start_id(self, node_id): return (node_id / self.per_group) * self.per_group - diff --git a/larch/refcountstore_tests.py b/larch/refcountstore_tests.py index f4f4ed1..c38f120 100644 --- a/larch/refcountstore_tests.py +++ b/larch/refcountstore_tests.py @@ -68,17 +68,6 @@ class RefcountStoreTests(unittest.TestCase): self.rs.set_refcount(123, 1) self.assertEqual(self.rs.get_refcount(123), 1) - def test_does_not_set_refcount_if_zero(self): - self.rs.set_refcount(123, 0) - self.assertFalse(123 in self.rs.refcounts) - self.assertEqual(self.rs.get_refcount(123), 0) - - def test_removes_refcount_that_drops_to_zero(self): - self.rs.set_refcount(123, 1) - self.rs.set_refcount(123, 0) - self.assertFalse(123 in self.rs.refcounts) - self.assertEqual(self.rs.get_refcount(123), 0) - def test_updates_refcount(self): self.rs.set_refcount(123, 1) self.rs.set_refcount(123, 2) @@ -102,8 +91,8 @@ class RefcountStoreTests(unittest.TestCase): refs = range(2048) for ref in refs: self.rs.set_refcount(ref, ref) - encoded = larch.refcountstore.encode_refcounts(self.rs.refcounts, - 0, 1024) + encoded = larch.refcountstore.encode_refcounts( + self.rs.refcounts, 0, 1024, range(1024)) decoded = larch.refcountstore.decode_refcounts(encoded) self.assertEqual(decoded, [(x, x) for x in refs[:1024]]) diff --git a/refcount-speed b/refcount-speed index eb489ed..5cbe797 100755 --- a/refcount-speed +++ b/refcount-speed @@ -72,12 +72,14 @@ class RefcountSpeedTest(cliapp.Application): # Calibrate. looptime = self.measure(nop, 'calibrate') - encode = self.measure(lambda: - larch.refcountstore.encode_refcounts(refcounts, - 0, len(refcounts)), - 'encode') - encoded = larch.refcountstore.encode_refcounts(refcounts, 0, - len(refcounts)) + num_refcounts = len(refcounts) + keys = refcounts.keys() + encode = self.measure( + lambda: larch.refcountstore.encode_refcounts( + refcounts, 0, num_refcounts, keys), + 'encode') + encoded = larch.refcountstore.encode_refcounts( + refcounts, 0, num_refcounts, keys) decode = self.measure(lambda: larch.refcountstore.decode_refcounts(encoded), 'decode') |