SVN: ZODB/branches/3.9/src/ Bugs Fixed:

Jim Fulton <[email protected]>
Newsgroups gmane.comp.python.zope.zodb.cvs
Message-ID <20091022143054.945549418D__21999.8069726445$1256222327$gmane$org@cvs.zope.org>
Log message for revision 105216:
  Bugs Fixed:
    2 BTree bugs, introduced by a bug fix in 3.9.0c2, sometimes caused
    deletion of keys to be improperly handled, resulting in data being
    available via iteraation but not item access.
  

Changed:
  U   ZODB/branches/3.9/src/BTrees/BTreeTemplate.c
  U   ZODB/branches/3.9/src/BTrees/MergeTemplate.c
  U   ZODB/branches/3.9/src/BTrees/tests/testBTrees.py
  U   ZODB/branches/3.9/src/BTrees/tests/testConflict.py
  U   ZODB/branches/3.9/src/CHANGES.txt
  U   ZODB/branches/3.9/src/ZODB/POSException.py

-=-
Modified: ZODB/branches/3.9/src/BTrees/BTreeTemplate.c
===================================================================
--- ZODB/branches/3.9/src/BTrees/BTreeTemplate.c	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/BTrees/BTreeTemplate.c	2009-10-22 14:30:54 UTC (rev 105216)
@@ -720,6 +720,7 @@
           COPY_KEY(d->key, bucket->keys[0]);
           INCREF_KEY(d->key);
           PER_UNUSE(bucket);
+          if (PER_CHANGED(self) < 0) goto Error;
         }
     }
 

Modified: ZODB/branches/3.9/src/BTrees/MergeTemplate.c
===================================================================
--- ZODB/branches/3.9/src/BTrees/MergeTemplate.c	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/BTrees/MergeTemplate.c	2009-10-22 14:30:54 UTC (rev 105216)
@@ -160,6 +160,15 @@
             }
           else if (set || (TEST_VALUE(i1.value, i2.value) == 0))
             {                   /* deleted in i3 */
+              if (i3.position == 1)
+                {
+                  /* Deleted the first item.  This will modify the
+                     parent node, so we don't know if merging will be
+                     safe
+                  */
+                  merge_error(i1.position, i2.position, i3.position, 13);
+                  goto err;
+                }
               if (i1.next(&i1) < 0) goto err;
               if (i2.next(&i2) < 0) goto err;
             }
@@ -178,6 +187,15 @@
             }
           else if (set || (TEST_VALUE(i1.value, i3.value) == 0))
             {                   /* deleted in i2 */
+              if (i2.position == 1)
+                {
+                  /* Deleted the first item.  This will modify the
+                     parent node, so we don't know if merging will be
+                     safe
+                  */
+                  merge_error(i1.position, i2.position, i3.position, 13);
+                  goto err;
+                }
               if (i1.next(&i1) < 0) goto err;
               if (i3.next(&i3) < 0) goto err;
             }

Modified: ZODB/branches/3.9/src/BTrees/tests/testBTrees.py
===================================================================
--- ZODB/branches/3.9/src/BTrees/tests/testBTrees.py	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/BTrees/tests/testBTrees.py	2009-10-22 14:30:54 UTC (rev 105216)
@@ -1975,7 +1975,11 @@
         with bucket children.
         """
 
-        tree = self.t_class()
+        from ZODB.MappingStorage import DB
+        db = DB()
+        conn = db.open()
+
+        tree = conn.root.tree = self.t_class()
         i = 0
 
         # Grow the btree until we have multiple buckets
@@ -1986,12 +1990,17 @@
             if len(data) >= 3:
                 break
 
+        transaction.commit()
+
         # Now, delete the internal key and make sure it's really gone
         key = data[1]
         del tree[key]
         data = tree.__getstate__()[0]
         self.assert_(data[1] != key)
 
+        # The tree should have changed:
+        self.assert_(tree._p_changed)
+
         # Grow the btree until we have multiple levels
         while 1:
             i += 1
@@ -2007,6 +2016,9 @@
         data = tree.__getstate__()[0]
         self.assert_(data[1] != key)
 
+        transaction.abort()
+        db.close()
+
 class InternalKeysSetTest:
     """There must not be any internal keys not in the TreeSet
     """

Modified: ZODB/branches/3.9/src/BTrees/tests/testConflict.py
===================================================================
--- ZODB/branches/3.9/src/BTrees/tests/testConflict.py	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/BTrees/tests/testConflict.py	2009-10-22 14:30:54 UTC (rev 105216)
@@ -49,6 +49,7 @@
         n = 'fs_tmp__%s' % os.getpid()
         self.storage = FileStorage(n)
         self.db = DB(self.storage)
+        return self.db
 
 class MappingBase(Base):
     """ Tests common to mappings (buckets, btrees) """
@@ -102,11 +103,11 @@
 
     def testMergeDelete(self):
         base, b1, b2, bm, e1, e2, items = self._setupConflict()
-        del b1[items[0][0]]
+        del b1[items[1][0]]
         del b2[items[5][0]]
         del b1[items[-1][0]]
         del b2[items[-2][0]]
-        del bm[items[0][0]]
+        del bm[items[1][0]]
         del bm[items[5][0]]
         del bm[items[-1][0]]
         del bm[items[-2][0]]
@@ -114,11 +115,11 @@
 
     def testMergeDeleteAndUpdate(self):
         base, b1, b2, bm, e1, e2, items = self._setupConflict()
-        del b1[items[0][0]]
+        del b1[items[1][0]]
         b2[items[5][0]]=1
         del b1[items[-1][0]]
         b2[items[-2][0]]=2
-        del bm[items[0][0]]
+        del bm[items[1][0]]
         bm[items[5][0]]=1
         del bm[items[-1][0]]
         bm[items[-2][0]]=2
@@ -237,11 +238,11 @@
 
     def testMergeDelete(self):
         base, b1, b2, bm, e1, e2, items = self._setupConflict()
-        b1.remove(items[0])
+        b1.remove(items[1])
         b2.remove(items[5])
         b1.remove(items[-1])
         b2.remove(items[-2])
-        bm.remove(items[0])
+        bm.remove(items[1])
         bm.remove(items[5])
         bm.remove(items[-1])
         bm.remove(items[-2])
@@ -331,9 +332,9 @@
         assert merged == expected, message
 
 class NastyConfict(Base, TestCase):
-    def setUp(self):
-        self.t = OOBTree()
 
+    t_type = OOBTree
+
     # This tests a problem that cropped up while trying to write
     # testBucketSplitConflict (below):  conflict resolution wasn't
     # working at all in non-trivial cases.  Symptoms varied from
@@ -765,7 +766,69 @@
         else:
             self.fail("expected ConflictError")
 
+    def testConflictOfInsertAndDeleteOfFirstBucketItem(self):
+        """
+        Recently, BTrees became careful about removing internal keys
+        (keys in internal aka BTree nodes) when they were deleted from
+        buckets. This poses a problem for conflict resolution.
 
+        We want to guard against a case in which the first key in a
+        bucket is removed in one transaction while a key is added
+        after that key but before the next key in another transaction
+        with the result that the added key is unreachble
+
+        original:
+
+          Bucket(...), k1, Bucket((k1, v1), (k3, v3), ...)
+
+        tran1
+
+          Bucket(...), k3, Bucket(k3, v3), ...)
+
+        tran2
+
+          Bucket(...), k1, Bucket((k1, v1), (k2, v2), (k3, v3), ...)
+
+          where k1 < k2 < k3
+
+        We don't want:
+
+          Bucket(...), k3, Bucket((k2, v2), (k3, v3), ...)
+
+          as k2 would be unfindable, so we want a conflict.
+
+        """
+        mytype = self.t_type
+        db = self.openDB()
+        tm1 = transaction.TransactionManager()
+        conn1 = db.open(tm1)
+        conn1.root.t = t = mytype()
+        for i in range(0, 200, 2):
+            t[i] = i
+        tm1.commit()
+        k = t.__getstate__()[0][1]
+        assert t.__getstate__()[0][2].keys()[0] == k
+
+        tm2 = transaction.TransactionManager()
+        conn2 = db.open(tm2)
+
+        t[k+1] = k+1
+        del conn2.root.t[k]
+        for i in range(200,300):
+            conn2.root.t[i] = i
+
+        tm1.commit()
+        self.assertRaises(ConflictError, tm2.commit)
+        tm2.abort()
+
+        k = t.__getstate__()[0][1]
+        t[k+1] = k+1
+        del conn2.root.t[k]
+
+        tm2.commit()
+        self.assertRaises(ConflictError, tm1.commit)
+        tm1.abort()
+
 def test_suite():
     suite = TestSuite()
 

Modified: ZODB/branches/3.9/src/CHANGES.txt
===================================================================
--- ZODB/branches/3.9/src/CHANGES.txt	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/CHANGES.txt	2009-10-22 14:30:54 UTC (rev 105216)
@@ -2,6 +2,16 @@
  Change History
 ================
 
+3.9.3 (2009-10-??)
+==================
+
+Bugs Fixed
+----------
+
+- 2 BTree bugs, introduced by a bug fix in 3.9.0c2, sometimes caused
+  deletion of keys to be improperly handled, resulting in data being
+  available via iteraation but not item access.
+
 3.9.2 (2009-10-13)
 ==================
 

Modified: ZODB/branches/3.9/src/ZODB/POSException.py
===================================================================
--- ZODB/branches/3.9/src/ZODB/POSException.py	2009-10-22 14:13:18 UTC (rev 105215)
+++ ZODB/branches/3.9/src/ZODB/POSException.py	2009-10-22 14:30:54 UTC (rev 105216)
@@ -214,6 +214,9 @@
 
             # 12; i2 or i3 was empty
             'Empty bucket in a transaction',
+
+            # 13; delete of first key, which causes change to parent node
+            'Delete of first key',
             ]
 
     def __init__(self, p1, p2, p3, reason):
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.