[Zodb-checkins] SVN: ZODB/branches/3.3/ As discussed on zodb-dev,
failing commit "sticks" now.
Tim Peters
tim.one at comcast.net
Tue Sep 14 13:38:11 EDT 2004
Log message for revision 27520:
As discussed on zodb-dev, failing commit "sticks" now.
After a commit fails (raises an exception), all subsequent attempts
to commit, join, or register with the transaction now raise the new
TransactionFailedError. The failed transaction must be explicitly
discarded now, via abort() on the transaction or begin() on its
transaction manager.
Changed:
U ZODB/branches/3.3/NEWS.txt
U ZODB/branches/3.3/src/ZODB/POSException.py
U ZODB/branches/3.3/src/ZODB/__init__.py
U ZODB/branches/3.3/src/ZODB/tests/testZODB.py
U ZODB/branches/3.3/src/ZODB/tests/testmvcc.py
U ZODB/branches/3.3/src/transaction/_transaction.py
-=-
Modified: ZODB/branches/3.3/NEWS.txt
===================================================================
--- ZODB/branches/3.3/NEWS.txt 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/NEWS.txt 2004-09-14 17:38:10 UTC (rev 27520)
@@ -7,7 +7,7 @@
ZODB intends to raise ConnnectionStateError if an attempt is made to close
a connection while modifications are pending (the connection is involved in
-a transaction that hasn't been ``abort()``'ed or ``commit()``'ed). It was
+a transaction that hasn't been ``abort()``'ed or ``commit()``'ed). It was
missing the case where the only pending modifications were made in
subtransactions. This has been fixed. If an attempt to close a connection
with pending subtransactions is made now::
@@ -19,6 +19,39 @@
transaction
-----------
+- Transactions have new, backward-incompatible behavior in one respect:
+ if a ``Transaction.commit()``, ``Transaction.commit(False)``, or
+ ``Transaction.commit(True)`` raised an exception, prior behavior was that
+ the transaction effectively aborted, and a new transaction began.
+ A primary bad consequence was that, if in a sequence of subtransaction
+ commits, one of the commits failed but the exception was suppressed,
+ all changes up to and including the failing commit were lost, but
+ later subtransaction commits in the sequence got no indication that
+ something had gone wrong, nor did the final (top level) commit. This
+ could easily lead to inconsistent data being committed, from the
+ application's point of view.
+
+ The new behavior is that a failing commit "sticks" until explicitly
+ cleared. Now if an exception is raised by a ``commit()`` call (whether
+ subtransaction or top level) on a Transaction object ``T``:
+
+ - Pending changes are aborted, exactly as they were for a failing
+ commit before.
+
+ - But ``T`` remains the current transaction object (if ``tm`` is ``T``'s
+ transaction manger, ``tm.get()`` continues to return ``T``).
+
+ - All subsequent attempts to do ``T.commit()``, ``T.join()``, or
+ ``T.register()`` raise the new ``TransactionFailedError`` exception.
+ Note that if you try to modify a persistent object, that object's
+ resource manager (usually a ``Connection`` object) will attempt to
+ ``join()`` the failed transaction, and ``TransactionFailedError``
+ will be raised right away.
+
+ So after a transaction or subtransaction commit fails, that must be
+ explicitly cleared now, either by invoking ``abort()`` on the transaction
+ object, or by invoking ``begin()`` on its transaction manager.
+
- Some explanations of new transaction features in the 3.3a3 news
were incorrect, and this news file has been retroactively edited to
repair that. See news for 3.3a3 below.
@@ -44,12 +77,13 @@
>>> import transaction
>>> txn = transaction.begin() # start a txn using the default TM
- if using the default ThreadTransactionManager (see news for 3.3a3 below).
- In 3.3, it's intended that a single Transaction object is used for exactly
- one transaction. So, unlike as in 3.2, when somtimes Transaction objects
- were reused across transactions, but sometimes weren't, when you do
- ``Transaction.begin()`` in 3.3 a brand new transaction object is
- created. That's why this use is deprecated. Code of the form:
+ if using the default ``ThreadTransactionManager`` (see news for 3.3a3
+ below). In 3.3, it's intended that a single ``Transaction`` object is
+ used for exactly one transaction. So, unlike as in 3.2, when somtimes
+ ``Transaction`` objects were reused across transactions, but sometimes
+ weren't, when you do ``Transaction.begin()`` in 3.3 a brand new
+ transaction object is created. That's why this use is deprecated. Code
+ of the form:
>>> txn = transaction.get()
>>> ...
@@ -57,11 +91,9 @@
>>> ...
>>> txn.commit()
- can't work as intended is 3.3, because ``txn`` is no longer the current
- Transaction object the instant ``txn.begin()`` returns.
+ can't work as intended ib 3.3, because ``txn`` is no longer the current
+ ``Transaction`` object the instant ``txn.begin()`` returns.
-
-
BTrees
------
Modified: ZODB/branches/3.3/src/ZODB/POSException.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/POSException.py 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/src/ZODB/POSException.py 2004-09-14 17:38:10 UTC (rev 27520)
@@ -33,6 +33,16 @@
class TransactionError(POSError):
"""An error occured due to normal transaction processing."""
+class TransactionFailedError(POSError):
+ """Cannot perform an operation on a transaction that previously failed.
+
+ An attempt was made to commit a transaction, or to join a transaction,
+ but this transaction previously raised an exception during an attempt
+ to commit it. The transaction must be explicitly aborted, either by
+ invoking abort() on the transaction, or begin() on its transaction
+ manager.
+ """
+
class ConflictError(TransactionError):
"""Two transactions tried to modify the same object at once.
Modified: ZODB/branches/3.3/src/ZODB/__init__.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/__init__.py 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/src/ZODB/__init__.py 2004-09-14 17:38:10 UTC (rev 27520)
@@ -26,6 +26,6 @@
# really be in persistent anyway.
sys.modules['ZODB.TimeStamp'] = sys.modules['persistent.TimeStamp']
-# XXX Issue deprecation warning if this variant is used?
+# TODO Issue deprecation warning if this variant is used?
__builtin__.get_transaction = get_transaction
del __builtin__
Modified: ZODB/branches/3.3/src/ZODB/tests/testZODB.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/tests/testZODB.py 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/src/ZODB/tests/testZODB.py 2004-09-14 17:38:10 UTC (rev 27520)
@@ -16,6 +16,7 @@
import ZODB
import ZODB.FileStorage
from ZODB.POSException import ReadConflictError, ConflictError
+from ZODB.POSException import TransactionFailedError
from ZODB.tests.warnhook import WarningsHook
from persistent import Persistent
@@ -254,7 +255,7 @@
self.obj = P()
self.readConflict()
- def readConflict(self, shouldFail=1):
+ def readConflict(self, shouldFail=True):
# Two transactions run concurrently. Each reads some object,
# then one commits and the other tries to read an object
# modified by the first. This read should fail with a conflict
@@ -290,6 +291,15 @@
# the transaction should re-raise it. checkNotIndependent()
# failed this part of the test for a long time.
self.assertRaises(ReadConflictError, tm2.get().commit)
+
+ # And since that commit failed, trying to commit again should
+ # fail again.
+ self.assertRaises(TransactionFailedError, tm2.get().commit)
+ # And again.
+ self.assertRaises(TransactionFailedError, tm2.get().commit)
+ # Etc.
+ self.assertRaises(TransactionFailedError, tm2.get().commit)
+
else:
# make sure that accessing the object succeeds
obj.child1
@@ -337,12 +347,13 @@
self.assert_(not index2[0]._p_changed)
self.assert_(not index2[1]._p_changed)
- self.assertRaises(ConflictError, tm.get().commit)
- transaction.abort()
+ self.assertRaises(ReadConflictError, tm.get().commit)
+ self.assertRaises(TransactionFailedError, tm.get().commit)
+ tm.get().abort()
def checkIndependent(self):
self.obj = Independent()
- self.readConflict(shouldFail=0)
+ self.readConflict(shouldFail=False)
def checkNotIndependent(self):
self.obj = DecoyIndependent()
@@ -438,6 +449,149 @@
finally:
del warnings.filters[0]
+ def checkFailingCommitSticks(self):
+ # See also checkFailingSubtransactionCommitSticks.
+ cn = self._db.open()
+ rt = cn.root()
+ rt['a'] = 1
+
+ # Arrange for commit to fail during tpc_vote.
+ poisoned = PoisonedObject(PoisonedJar(break_tpc_vote=True))
+ transaction.get().register(poisoned)
+
+ self.assertRaises(PoisonedError, transaction.get().commit)
+ # Trying to commit again fails too.
+ self.assertRaises(TransactionFailedError, transaction.get().commit)
+ self.assertRaises(TransactionFailedError, transaction.get().commit)
+ self.assertRaises(TransactionFailedError, transaction.get().commit)
+
+ # The change to rt['a'] is lost.
+ self.assertRaises(KeyError, rt.__getitem__, 'a')
+
+ # Trying to modify an object also fails, because Transaction.join()
+ # also raises TransactionFailedError.
+ self.assertRaises(TransactionFailedError, rt.__setitem__, 'b', 2)
+
+ # Clean up via abort(), and try again.
+ transaction.get().abort()
+ rt['a'] = 1
+ transaction.get().commit()
+ self.assertEqual(rt['a'], 1)
+
+ # Cleaning up via begin() should also work.
+ rt['a'] = 2
+ transaction.get().register(poisoned)
+ self.assertRaises(PoisonedError, transaction.get().commit)
+ self.assertRaises(TransactionFailedError, transaction.get().commit)
+ # The change to rt['a'] is lost.
+ self.assertEqual(rt['a'], 1)
+ # Trying to modify an object also fails.
+ self.assertRaises(TransactionFailedError, rt.__setitem__, 'b', 2)
+ # Clean up via begin(), and try again.
+ transaction.begin()
+ rt['a'] = 2
+ transaction.get().commit()
+ self.assertEqual(rt['a'], 2)
+
+ cn.close()
+
+ def checkFailingSubtransactionCommitSticks(self):
+ cn = self._db.open()
+ rt = cn.root()
+ rt['a'] = 1
+ transaction.get().commit(True)
+ self.assertEqual(rt['a'], 1)
+
+ rt['b'] = 2
+ # Subtransactions don't do tpc_vote, so we poison tpc_begin.
+ poisoned = PoisonedObject(PoisonedJar(break_tpc_begin=True))
+ transaction.get().register(poisoned)
+ self.assertRaises(PoisonedError, transaction.get().commit, True)
+ # Trying to subtxn-commit again fails too.
+ self.assertRaises(TransactionFailedError, transaction.get().commit, True)
+ self.assertRaises(TransactionFailedError, transaction.get().commit, True)
+ # Top-level commit also fails.
+ self.assertRaises(TransactionFailedError, transaction.get().commit)
+
+ # The changes to rt['a'] and rt['b'] are lost.
+ self.assertRaises(KeyError, rt.__getitem__, 'a')
+ self.assertRaises(KeyError, rt.__getitem__, 'b')
+
+ # Trying to modify an object also fails, because Transaction.join()
+ # also raises TransactionFailedError.
+ self.assertRaises(TransactionFailedError, rt.__setitem__, 'b', 2)
+
+ # Clean up via abort(), and try again.
+ transaction.get().abort()
+ rt['a'] = 1
+ transaction.get().commit()
+ self.assertEqual(rt['a'], 1)
+
+ # Cleaning up via begin() should also work.
+ rt['a'] = 2
+ transaction.get().register(poisoned)
+ self.assertRaises(PoisonedError, transaction.get().commit, True)
+ self.assertRaises(TransactionFailedError, transaction.get().commit, True)
+ # The change to rt['a'] is lost.
+ self.assertEqual(rt['a'], 1)
+ # Trying to modify an object also fails.
+ self.assertRaises(TransactionFailedError, rt.__setitem__, 'b', 2)
+ # Clean up via begin(), and try again.
+ transaction.begin()
+ rt['a'] = 2
+ transaction.get().commit(True)
+ self.assertEqual(rt['a'], 2)
+ transaction.get().commit()
+
+ cn2 = self._db.open()
+ rt = cn.root()
+ self.assertEqual(rt['a'], 2)
+
+ cn.close()
+ cn2.close()
+
+class PoisonedError(Exception):
+ pass
+
+# PoisonedJar arranges to raise exceptions from interesting places.
+# For whatever reason, subtransaction commits don't call tpc_vote.
+class PoisonedJar:
+ def __init__(self, break_tpc_begin=False, break_tpc_vote=False):
+ self.break_tpc_begin = break_tpc_begin
+ self.break_tpc_vote = break_tpc_vote
+
+ def sortKey(self):
+ return str(id(self))
+
+ # A way to poison a subtransaction commit.
+ def tpc_begin(self, *args):
+ if self.break_tpc_begin:
+ raise PoisonedError("tpc_begin fails")
+
+ # A way to poison a top-level commit.
+ def tpc_vote(self, *args):
+ if self.break_tpc_vote:
+ raise PoisonedError("tpc_vote fails")
+
+ # commit_sub is needed else this jar is ignored during subtransaction
+ # commit.
+ def commit_sub(*args):
+ pass
+
+ def abort_sub(*args):
+ pass
+
+ def commit(*args):
+ pass
+
+ def abort(*self):
+ pass
+
+
+class PoisonedObject:
+ def __init__(self, poisonedjar):
+ self._p_jar = poisonedjar
+
def test_suite():
return unittest.makeSuite(ZODBTests, 'check')
Modified: ZODB/branches/3.3/src/ZODB/tests/testmvcc.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/tests/testmvcc.py 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/src/ZODB/tests/testmvcc.py 2004-09-14 17:38:10 UTC (rev 27520)
@@ -147,10 +147,11 @@
...
ConflictError: database conflict error (oid 0x01, class ZODB.tests.MinPO.MinPO)
-The failed commit aborted the current transaction, so we can try
-again. This example will demonstrate that we can commit a transaction
-if we only modify current revisions.
+>>> tm2.get().abort()
+This example will demonstrate that we can commit a transaction if we only
+modify current revisions.
+
>>> print cn2._txn_time
None
Modified: ZODB/branches/3.3/src/transaction/_transaction.py
===================================================================
--- ZODB/branches/3.3/src/transaction/_transaction.py 2004-09-14 15:31:05 UTC (rev 27519)
+++ ZODB/branches/3.3/src/transaction/_transaction.py 2004-09-14 17:38:10 UTC (rev 27520)
@@ -137,7 +137,17 @@
import sys
import thread
import warnings
+import traceback
+from cStringIO import StringIO
+# Sigh. In the maze of __init__.py's, ZODB.__init__.py takes 'get'
+# out of transaction.__init__.py, in order to stuff the 'get_transaction'
+# alias in __builtin__. So here in _transaction.py, we can't import
+# exceptions from ZODB.POSException at top level (we're imported by
+# our __init__.py, which is imported by ZODB's __init__, so the ZODB
+# package isn't well-formed when we're first imported).
+# from ZODB.POSException import TransactionError, TransactionFailedError
+
_marker = object()
# The point of this is to avoid hiding exceptions (which the builtin
@@ -146,14 +156,16 @@
return getattr(obj, attr, _marker) is not _marker
class Status:
+ # ACTIVE is the initial state.
+ ACTIVE = "Active"
- ACTIVE = "Active"
- COMMITTING = "Committing"
- COMMITTED = "Committed"
- ABORTING = "Aborting"
- ABORTED = "Aborted"
- FAILED = "Failed"
+ COMMITTING = "Committing"
+ COMMITTED = "Committed"
+ # commit() or commit(True) raised an exception. All further attempts
+ # to commit or join this transaction will raise TransactionFailedError.
+ COMMITFAILED = "Commit failed"
+
class Transaction(object):
def __init__(self, synchronizers=None, manager=None):
@@ -186,8 +198,26 @@
# subtransactions that were involved in subtransaction commits.
self._nonsub = {}
+ # If a commit fails, the traceback is saved in _failure_traceback.
+ # If another attempt is made to commit, TransactionFailedError is
+ # raised, incorporating this traceback.
+ self._failure_traceback = None
+
+ # Raise TransactionFailedError, due to commit()/join()/register()
+ # getting called when the current transaction has already suffered
+ # a commit failure.
+ def _prior_commit_failed(self):
+ from ZODB.POSException import TransactionFailedError
+ assert self._failure_traceback is not None
+ raise TransactionFailedError("commit() previously failed, "
+ "with this traceback:\n\n%s" %
+ self._failure_traceback.getvalue())
+
def join(self, resource):
- if self.status != Status.ACTIVE:
+ if self.status is Status.COMMITFAILED:
+ self._prior_commit_failed() # doesn't return
+
+ if self.status is not Status.ACTIVE:
# TODO: Should it be possible to join a committing transaction?
# I think some users want it.
raise ValueError("expected txn status %r, but it's %r" % (
@@ -244,6 +274,9 @@
# transaction.
def commit(self, subtransaction=False):
+ if self.status is Status.COMMITFAILED:
+ self._prior_commit_failed() # doesn't return
+
if not subtransaction and self._sub and self._resources:
# This commit is for a top-level transaction that has
# previously committed subtransactions. Do one last
@@ -256,7 +289,20 @@
s.beforeCompletion(self)
self.status = Status.COMMITTING
- self._commitResources(subtransaction)
+ try:
+ self._commitResources(subtransaction)
+ except:
+ self.status = Status.COMMITFAILED
+ # Save the traceback for TransactionFailedError.
+ ft = self._failure_traceback = StringIO()
+ t, v, tb = sys.exc_info()
+ # Record how we got into commit().
+ traceback.print_stack(sys._getframe(1), None, ft)
+ # Append the stack entries from here down to the exception.
+ traceback.print_tb(tb, None, ft)
+ # Append the exception type and value.
+ ft.writelines(traceback.format_exception_only(t, v))
+ raise t, v, tb
if subtransaction:
self._resources = []
@@ -315,9 +361,6 @@
self._cleanup(L)
finally:
if not subtransaction:
- self.status = Status.FAILED
- if self._manager:
- self._manager.free(self)
for s in self._synchronizers:
s.afterCompletion(self)
raise t, v, tb
@@ -389,6 +432,7 @@
s.beforeCompletion(self)
if subtransaction and self._nonsub:
+ from ZODB.POSException import TransactionError
raise TransactionError("Resource manager does not support "
"subtransaction abort")
More information about the Zodb-checkins
mailing list