summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLars Wirzenius <liw@liw.fi>2013-11-06 20:35:12 +0000
committerLars Wirzenius <liw@liw.fi>2013-11-06 20:35:12 +0000
commit95bdc966cf8abe4e79ce32a8accd2e323b487cc1 (patch)
tree68718f614ffe84d1ac0b697b5badaab2199c07f6
parent267b049865cc239e98847ea013dd9c88abe3f8e6 (diff)
parent1d31b862b1f0fc6732d0c2811b7775025039db89 (diff)
downloadlarch-95bdc966cf8abe4e79ce32a8accd2e323b487cc1.tar.gz
Merge branch 'liw/refcount-keyerror'
-rw-r--r--NEWS9
-rw-r--r--larch/refcountstore.py27
-rw-r--r--larch/refcountstore_tests.py15
-rwxr-xr-xrefcount-speed14
4 files changed, 32 insertions, 33 deletions
diff --git a/NEWS b/NEWS
index a1ffb28..f2e8276 100644
--- a/NEWS
+++ b/NEWS
@@ -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')