Commit ca575add authored by Tim Peters's avatar Tim Peters

Merge rev 29668 from 3.3 branch.

Collector #1734.  Critical bug in BTree conflict resolution.

Stop silent data loss in some BTree cases where a transaction adds
a new key to a bucket while a concurrent transaction deletes all
keys from the same bucket.

Still needs porting to 3.2 line.
parent 98978059
......@@ -110,6 +110,21 @@ What's new in ZODB3 3.3.1a2?
============================
Release date: DD-MMM-2005
BTrees
------
Collector #1734: BTrees conflict resolution leads to index inconsistencies.
Silent data loss could occur due to BTree conflict resolution when one
transaction T1 added a new key to a BTree containing at least three buckets,
and a concurrent transaction T2 deleted all keys in the bucket to which the
new key was added. Conflict resolution then created a bucket containing the
newly added key, but the bucket remained isolated, disconnected from the
BTree. In other words, the committed BTree didn't contain the new key added by
T1. Conflict resolution doesn't have enough information to repair this,
so ``ConflictError`` is now raised in such cases.
ZEO
---
......
......@@ -70,7 +70,9 @@ merge_error(int p1, int p2, int p3, int reason)
* However, it's not OK for s2 and s3 to, between them, end up deleting all
* the keys. This is a higher-level constraint, due to that the caller of
* bucket_merge() doesn't have enough info to unlink the resulting empty
* bucket from its BTree correctly.
* bucket from its BTree correctly. It's also not OK if s2 or s3 are empty,
* because the transaction that emptied the bucket unlinked the bucket from
* the tree, and nothing we do here can get it linked back in again.
*
* Key insertion: s2 or s3 can add a new key, provided the other transaction
* doesn't insert the same key. It's not OK even if they insert the same
......@@ -91,6 +93,13 @@ bucket_merge(Bucket *s1, Bucket *s2, Bucket *s3)
SetIteration i1 = {0,0,0}, i2 = {0,0,0}, i3 = {0,0,0};
int cmp12, cmp13, cmp23, mapping, set;
/* If either "after" bucket is empty, punt. */
if (s2->len == 0 || s3->len == 0)
{
merge_error(-1, -1, -1, 12);
goto err;
}
if (initSetIteration(&i1, OBJECT(s1), 1) < 0)
goto err;
if (initSetIteration(&i2, OBJECT(s2), 1) < 0)
......
......@@ -178,7 +178,7 @@ class MappingBase(Base):
test_merge(base, b1, b2, bm, 'merge insert from empty')
def testMergeEmptyAndFill(self):
def testFailMergeEmptyAndFill(self):
base, b1, b2, bm, e1, e2, items = self._setupConflict()
b1.clear()
......@@ -186,7 +186,7 @@ class MappingBase(Base):
b2.update(e2)
bm.update(e2)
test_merge(base, b1, b2, bm, 'merge insert from empty')
test_merge(base, b1, b2, bm, 'merge insert from empty', should_fail=1)
def testMergeEmpty(self):
base, b1, b2, bm, e1, e2, items = self._setupConflict()
......@@ -275,7 +275,7 @@ class SetTests(Base):
test_merge(base, b1, b2, bm, 'merge insert from empty')
def testMergeEmptyAndFill(self):
def testFailMergeEmptyAndFill(self):
base, b1, b2, bm, e1, e2, items = self._setupConflict()
b1.clear()
......@@ -283,7 +283,7 @@ class SetTests(Base):
b2.update(e2)
bm.update(e2)
test_merge(base, b1, b2, bm, 'merge insert from empty')
test_merge(base, b1, b2, bm, 'merge insert from empty', should_fail=1)
def testMergeEmpty(self):
base, b1, b2, bm, e1, e2, items = self._setupConflict()
......@@ -759,6 +759,90 @@ class NastyConfict(Base, TestCase):
else:
self.fail("expected ConflictError")
def testConflictWithOneEmptyBucket(self):
# If one transaction empties a bucket, while another adds an item
# to the bucket, all the changes "look resolvable": bucket conflict
# resolution returns a bucket containing (only) the item added by
# the latter transaction, but changes from the former transaction
# removing the bucket are uncontested: the bucket is removed from
# the BTree despite that resolution thinks it's non-empty! This
# was first reported by Dieter Maurer, to zodb-dev on 22 Mar 2005.
b = self.t
for i in range(0, 200, 4):
b[i] = i
# bucket 0 has 15 values: 0, 4 .. 56
# bucket 1 has 15 values: 60, 64 .. 116
# bucket 2 has 20 values: 120, 124 .. 196
state = b.__getstate__()
# Looks like: ((bucket0, 60, bucket1, 120, bucket2), firstbucket)
# If these fail, the *preconditions* for running the test aren't
# satisfied -- the test itself hasn't been run yet.
self.assertEqual(len(state), 2)
self.assertEqual(len(state[0]), 5)
self.assertEqual(state[0][1], 60)
self.assertEqual(state[0][3], 120)
# Set up database connections to provoke conflict.
self.openDB()
r1 = self.db.open().root()
r1["t"] = self.t
transaction.commit()
r2 = self.db.open(synch=False).root()
copy = r2["t"]
# Make sure all of copy is loaded.
list(copy.values())
self.assertEqual(self.t._p_serial, copy._p_serial)
# Now one transaction empties the first bucket, and another adds a
# key to the first bucket.
for k in range(0, 60, 4):
del self.t[k]
transaction.commit()
copy[1] = 1
try:
transaction.commit()
except ConflictError, detail:
self.assert_(str(detail).startswith('database conflict error'))
transaction.abort()
else:
self.fail("expected ConflictError")
# Same thing, except commit the transactions in the opposite order.
b = OOBTree()
for i in range(0, 200, 4):
b[i] = i
r1 = self.db.open().root()
r1["t"] = b
transaction.commit()
r2 = self.db.open(synch=False).root()
copy = r2["t"]
# Make sure all of copy is loaded.
list(copy.values())
self.assertEqual(b._p_serial, copy._p_serial)
# Now one transaction empties the first bucket, and another adds a
# key to the first bucket.
b[1] = 1
transaction.commit()
for k in range(0, 60, 4):
del copy[k]
try:
transaction.commit()
except ConflictError, detail:
self.assert_(str(detail).startswith('database conflict error'))
transaction.abort()
else:
self.fail("expected ConflictError")
def test_suite():
suite = TestSuite()
......
......@@ -187,6 +187,9 @@ class BTreesConflictError(ConflictError):
# 11; conflicting changes in an internal BTree node
'Conflicting changes in an internal BTree node',
# 12; i2 or i3 was empty
'Empty bucket in a transaction',
]
def __init__(self, p1, p2, p3, reason):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment