Commit 75a39dc2 authored by Jim Fulton's avatar Jim Fulton

Bugs Fixed

----------

- 3.10 introduced an optimization to try to address BTree conflict
  errors arrising for basing BTree keys on object ids. The
  optimization caused object ids allocated in aborted transactions to
  be reused. Unfortunately, this optimzation led to some rather
  severe failures in some applications.  The symptom is a conflict
  error in which one of the serials mentioned is zero.  This
  optimization has been removed.
parent 34c6c04d
...@@ -2,6 +2,21 @@ ...@@ -2,6 +2,21 @@
Change History Change History
================ ================
3.10.2 (2011-02-??)
===================
Bugs Fixed
----------
- 3.10 introduced an optimization to try to address BTree conflict
errors arrising for basing BTree keys on object ids. The
optimization caused object ids allocated in aborted transactions to
be reused. Unfortunately, this optimzation led to some rather
severe failures in some applications. The symptom is a conflict
error in which one of the serials mentioned is zero. This
optimization has been removed.
3.10.1 (2010-10-27) 3.10.1 (2010-10-27)
=================== ===================
......
...@@ -452,7 +452,6 @@ class Connection(ExportImport, object): ...@@ -452,7 +452,6 @@ class Connection(ExportImport, object):
del obj._p_oid del obj._p_oid
if obj._p_changed: if obj._p_changed:
obj._p_changed = False obj._p_changed = False
self._db.save_oid(oid)
else: else:
# Note: If we invalidate a non-ghostifiable object # Note: If we invalidate a non-ghostifiable object
...@@ -760,7 +759,6 @@ class Connection(ExportImport, object): ...@@ -760,7 +759,6 @@ class Connection(ExportImport, object):
self._creating = {} self._creating = {}
for oid in creating: for oid in creating:
self._db.save_oid(oid)
o = self._cache.get(oid) o = self._cache.get(oid)
if o is not None: if o is not None:
del self._cache[oid] del self._cache[oid]
......
...@@ -377,7 +377,6 @@ class DB(object): ...@@ -377,7 +377,6 @@ class DB(object):
database_name='unnamed', database_name='unnamed',
databases=None, databases=None,
xrefs=True, xrefs=True,
max_saved_oids=999,
large_record_size=1<<24, large_record_size=1<<24,
**storage_args): **storage_args):
"""Create an object database. """Create an object database.
...@@ -475,8 +474,6 @@ class DB(object): ...@@ -475,8 +474,6 @@ class DB(object):
databases[database_name] = self databases[database_name] = self
self.xrefs = xrefs self.xrefs = xrefs
self._saved_oids = set()
self._max_saved_oids = max_saved_oids
self.large_record_size = large_record_size self.large_record_size = large_record_size
@property @property
...@@ -957,19 +954,7 @@ class DB(object): ...@@ -957,19 +954,7 @@ class DB(object):
def transaction(self): def transaction(self):
return ContextManager(self) return ContextManager(self)
def save_oid(self, oid):
if oid in self._saved_oids:
raise ValueError("Duplicate saved object ids.")
if len(self._saved_oids) < self._max_saved_oids:
self._saved_oids.add(oid)
def new_oid(self): def new_oid(self):
if self._saved_oids:
try:
return self._saved_oids.pop()
except IndexError:
pass # Hm, threads
return self.storage.new_oid() return self.storage.new_oid()
......
New OIDs get reused if a transaction aborts
===========================================
Historical note:
An OID is a terrible thing to waste.
Seriously: sequential allocation of OIDs could cause problems when
OIDs are used as (or as the basis of) BTree keys. This happened
with Zope 3's intid utility where the object->id mapping uses an
object key based on the OID. We got frequent conflict errors
because, in a site with many users, many objects are added at the
same time and conficts happened when conflicting changes caused
bucket splits.
Reusing an earlier allocated, but discarded OID will allow retries
of transactions to work because they'll use earlier OIDs which won't
tend to conflict with newly allocated ones.
If a transaction is aborted, new OIDs assigned in the transaction are
saved and made available for later transactions.
>>> import ZODB.tests.util, transaction
>>> db = ZODB.tests.util.DB()
>>> tm1 = transaction.TransactionManager()
>>> conn1 = db.open(tm1)
>>> conn1.root.x = ZODB.tests.util.P()
>>> tm1.commit()
>>> conn1.root.x.x = ZODB.tests.util.P()
>>> conn1.root.y = 1
>>> tm2 = transaction.TransactionManager()
>>> conn2 = db.open(tm2)
>>> conn2.root.y = ZODB.tests.util.P()
>>> tm2.commit()
We get a conflict when we try to commit the change to the first connection:
>>> tm1.commit() # doctest: +ELLIPSIS
Traceback (most recent call last):
...
ConflictError: ...
>>> tm1.abort()
When we try, we get the same oid we would have gotten on the first transaction:
>>> conn1.root.x.x = ZODB.tests.util.P()
>>> tm1.commit()
>>> conn1.root.x.x._p_oid
'\x00\x00\x00\x00\x00\x00\x00\x03'
We see this more clearly when we use add to assign the oid before the commit:
>>> conn1.root.z = ZODB.tests.util.P()
>>> conn1.add(conn1.root.z)
>>> conn1.root.z._p_oid
'\x00\x00\x00\x00\x00\x00\x00\x04'
>>> tm1.abort()
>>> conn2.root.a = ZODB.tests.util.P()
>>> conn2.add(conn2.root.a)
>>> conn2.root.a._p_oid
'\x00\x00\x00\x00\x00\x00\x00\x04'
...@@ -1238,8 +1238,6 @@ class StubDatabase: ...@@ -1238,8 +1238,6 @@ class StubDatabase:
def invalidate(self, transaction, dict_with_oid_keys, connection): def invalidate(self, transaction, dict_with_oid_keys, connection):
pass pass
save_oid = lambda self, oid: None
large_record_size = 1<<30 large_record_size = 1<<30
def test_suite(): def test_suite():
......
...@@ -180,90 +180,6 @@ the wrong state. ...@@ -180,90 +180,6 @@ the wrong state.
""" """
def testNewOidsGetReusedOnRollback():
"""\
New oids assigned during a transaction should be reused on
rollback, not just on abort.
>>> connection = ZODB.connection(None)
>>> root = connection.root()
Let's make a savepoint (with a real change so that the connection
is involved, otherwise rolling back to the savepoint will just be
an abort).
>>> root._p_changed = True # Important!
>>> sp = transaction.savepoint()
Add an object, and make a new savepoint so that it gets an oid.
>>> root['p'] = p = ZODB.tests.util.P()
>>> sp2 = transaction.savepoint()
>>> p._p_oid
'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x01'
Rolling back to the first savepoint unassigns the oid, but not
before invalidating the object. If a custom _p_invalidate method
tries to reactivate the object *during* rollback, a POSKeyError is
raised.
>>> sp.rollback()
>>> p._p_oid is None
True
>>> p._p_jar is None
True
>>> print p._p_changed
False
The rest of the state of p is undefined after the rollback. Using
p in any way from this point is a bad idea.
>>> del p
If we create a new object and add it, the oid is reused.
>>> root['p'] = p = ZODB.tests.util.P()
>>> connection.add(p)
>>> p._p_oid
'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x01'
>>> transaction.abort()
"""
def testNewOidNotReusedAfterRollbackOfPostAddSavepoint():
"""\
>>> connection = ZODB.connection(None)
>>> root = connection.root()
Add an item, and make a savepoint.
>>> root['p'] = p = ZODB.tests.util.P()
>>> sp = transaction.savepoint()
Now rollback the savepoint.
>>> sp.rollback()
At this point, the item still exists, since it was created before
the savepoint we rolled back.
>>> root['p']._p_oid
'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x01'
Add another item, and make another savepoint.
>>> root['p2'] = p2 = ZODB.tests.util.P()
>>> sp2 = transaction.savepoint()
The second item should not reuse the oid of the first.
>>> p2._p_oid
'\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x02'
transaction.abort()
"""
def tearDown(test): def tearDown(test):
transaction.abort() transaction.abort()
......
...@@ -345,30 +345,6 @@ def minimally_test_connection_timeout(): ...@@ -345,30 +345,6 @@ def minimally_test_connection_timeout():
""" """
def saving_oid_multiple_times_doesnt_cause_dups():
r"""Duplicate saves of an oid shouldn't happen unless there's a bug elsewhere
But saving dups makes matters worse, because it dooms the process,
not just the transaction.
>>> db = ZODB.DB(None)
>>> oid = db.new_oid()
>>> db.save_oid(oid)
>>> db.new_oid() is oid
True
>>> db.save_oid(oid)
>>> db.save_oid(oid)
Traceback (most recent call last):
...
ValueError: Duplicate saved object ids.
>>> db.new_oid() is oid
True
>>> db.new_oid()
'\x00\x00\x00\x00\x00\x00\x00\x02'
"""
def test_suite(): def test_suite():
s = unittest.makeSuite(DBTests) s = unittest.makeSuite(DBTests)
s.addTest(doctest.DocTestSuite( s.addTest(doctest.DocTestSuite(
......
...@@ -18,7 +18,6 @@ from ZODB.POSException import ReadConflictError, ConflictError ...@@ -18,7 +18,6 @@ from ZODB.POSException import ReadConflictError, ConflictError
from ZODB.POSException import TransactionFailedError from ZODB.POSException import TransactionFailedError
from ZODB.tests.warnhook import WarningsHook from ZODB.tests.warnhook import WarningsHook
import doctest
import transaction import transaction
import unittest import unittest
import warnings import warnings
...@@ -615,7 +614,6 @@ class PoisonedObject: ...@@ -615,7 +614,6 @@ class PoisonedObject:
def test_suite(): def test_suite():
suite = unittest.makeSuite(ZODBTests, 'check') suite = unittest.makeSuite(ZODBTests, 'check')
suite.addTest(doctest.DocFileSuite('new_oids_get_reused_on_abort.test'))
return suite return suite
if __name__ == "__main__": if __name__ == "__main__":
......
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