[Zodb-checkins] SVN: ZODB/branches/3.4/src/ Partial savepoint
review.
Tim Peters
tim.one at comcast.net
Tue May 3 17:29:41 EDT 2005
Log message for revision 30235:
Partial savepoint review.
Added more comments. Changed some comments to English.
Renamed some attributes and methods for clarity.
Switched to using a Python WeakKeyDictionary instead of
rolling our own out of a Python dict and raw weakrefs.
Changed:
U ZODB/branches/3.4/src/ZODB/Connection.py
U ZODB/branches/3.4/src/transaction/_transaction.py
U ZODB/branches/3.4/src/transaction/savepoint.txt
U ZODB/branches/3.4/src/transaction/tests/savepointsample.py
-=-
Modified: ZODB/branches/3.4/src/ZODB/Connection.py
===================================================================
--- ZODB/branches/3.4/src/ZODB/Connection.py 2005-05-03 12:59:42 UTC (rev 30234)
+++ ZODB/branches/3.4/src/ZODB/Connection.py 2005-05-03 21:29:40 UTC (rev 30235)
@@ -331,7 +331,6 @@
# they've been unadded. This will make the code in _abort
# confused.
-
self._abort()
if self._savepoint_storage is not None:
@@ -353,9 +352,9 @@
# Note: If we invalidate a non-ghostifiable object
# (i.e. a persistent class), the object will
- # immediately reread it's state. That means that the
+ # immediately reread its state. That means that the
# following call could result in a call to
- # self.setstate, which, of course, must suceed.
+ # self.setstate, which, of course, must succeed.
# In general, it would be better if the read could be
# delayed until the start of the next transaction. If
# we read at the end of a transaction and if the
@@ -365,7 +364,7 @@
# can only delay the read if this abort corresponds to
# a top-level-transaction abort. We can't tell if
# this is a top-level-transaction abort, so we have to
- # go ahead and invalidate now. Fortunately, it's
+ # go ahead and invalidate now. Fortunately, it's
# pretty unlikely that the object we are invalidating
# was invalidated by another thread, so the risk of a
# reread is pretty low.
@@ -383,21 +382,20 @@
def _flush_invalidations(self):
self._inv_lock.acquire()
try:
-
# Non-ghostifiable objects may need to read when they are
- # invalidated, so, we'll quickly just replace the
+ # invalidated, so we'll quickly just replace the
# invalidating dict with a new one. We'll then process
# the invalidations after freeing the lock *and* after
- # reseting the time. This means that invalidations will
+ # resetting the time. This means that invalidations will
# happen after the start of the transactions. They are
- # subject to conflict errors and to reading old data,
+ # subject to conflict errors and to reading old data.
# TODO: There is a potential problem lurking for persistent
- # classes. Suppose we have an invlidation of a persistent
+ # classes. Suppose we have an invalidation of a persistent
# class and of an instance. If the instance is
# invalidated first and if the invalidation logic uses
# data read from the class, then the invalidation could
- # be performed with state data. Or, suppose that there
+ # be performed with stale data. Or, suppose that there
# are instances of the class that are freed as a result of
# invalidating some object. Perhaps code in their __del__
# uses class data. Really, the only way to properly fix
@@ -407,10 +405,10 @@
# much worse than that though, because we'd also need to
# deal with slots. When a class is ghostified, we'd need
# to replace all of the slot operations with versions that
- # reloaded the object when caled. It's hard to say which
- # is better for worse. For now, it seems the risk of
+ # reloaded the object when called. It's hard to say which
+ # is better or worse. For now, it seems the risk of
# using a class while objects are being invalidated seems
- # small enough t be acceptable.
+ # small enough to be acceptable.
invalidated = self._invalidated
self._invalidated = {}
Modified: ZODB/branches/3.4/src/transaction/_transaction.py
===================================================================
--- ZODB/branches/3.4/src/transaction/_transaction.py 2005-05-03 12:59:42 UTC (rev 30234)
+++ ZODB/branches/3.4/src/transaction/_transaction.py 2005-05-03 21:29:40 UTC (rev 30235)
@@ -30,7 +30,7 @@
Subtransactions
---------------
-Note: Suntransactions are deprecated!
+Note: Suntransactions are deprecated! Use savepoint/rollback instead.
A subtransaction applies the transaction notion recursively. It
allows a set of modifications within a transaction to be committed or
@@ -189,17 +189,20 @@
interfaces.ITransactionDeprecated)
- # Assign an index to each savepoint so we can invalidate
- # later savepoints on rollback
+ # Assign an index to each savepoint so we can invalidate later savepoints
+ # on rollback. The first index assigned is 1, and it goes up by 1 each
+ # time.
_savepoint_index = 0
- # If savepoints are used, keep a weak key dict of them
- _savepoints = None
+ # If savepoints are used, keep a weak key dict of them. This maps a
+ # savepoint to its index (see above).
+ _savepoint2index = None
- # Remamber the savepoint for the last subtransaction
+ # Remember the savepoint for the last subtransaction.
_subtransaction_savepoint = None
- # Meta data
+ # Meta data. ._extension is also metadata, but is initialized to an
+ # emtpy dict in __init__.
user = ""
description = ""
@@ -267,26 +270,21 @@
resource = DataManagerAdapter(resource)
self._resources.append(resource)
-
- if self._savepoints:
-
+ if self._savepoint2index:
# A data manager has joined a transaction *after* a savepoint
# was created. A couple of things are different in this case:
-
- # 1. We need to add it's savepoint to all previous savepoints.
- # so that if they are rolled back, we roll this was back too.
-
- # 2. We don't actualy need to ask it for a savepoint.
- # Because is just joining. We can just abort it to roll
- # back to the current state, so we simply use and
+ #
+ # 1. We need to add its savepoint to all previous savepoints.
+ # so that if they are rolled back, we roll this one back too.
+ #
+ # 2. We don't actually need to ask the data manager for a
+ # savepoint: because it's just joining, we can just abort it to
+ # roll back to the current state, so we simply use an
# AbortSavepoint.
-
datamanager_savepoint = AbortSavepoint(resource, self)
- for ref in self._savepoints:
- transaction_savepoint = ref()
- if transaction_savepoint is not None:
- transaction_savepoint._savepoints.append(
- datamanager_savepoint)
+ for transaction_savepoint in self._savepoint2index.keys():
+ transaction_savepoint._savepoints.append(
+ datamanager_savepoint)
def savepoint(self, optimistic=False):
if self.status is Status.COMMITFAILED:
@@ -298,43 +296,30 @@
self._cleanup(self._resources)
self._saveCommitishError() # reraises!
-
+ if self._savepoint2index is None:
+ self._savepoint2index = weakref.WeakKeyDictionary()
self._savepoint_index += 1
- ref = weakref.ref(savepoint, self._remove_savepoint_ref)
- if self._savepoints is None:
- self._savepoints = {}
- self._savepoints[ref] = self._savepoint_index
+ self._savepoint2index[savepoint] = self._savepoint_index
return savepoint
- def _remove_savepoint_ref(self, ref):
- try:
- del self._savepoints[ref]
- except KeyError:
- pass
-
- def _invalidate_next(self, savepoint):
- savepoints = self._savepoints
- ref = weakref.ref(savepoint)
- index = savepoints[ref]
- del savepoints[ref]
-
+ # Remove `savepoint` from _savepoint2index, and also remove and invalidate
+ # all savepoints we know about with an index larger than `savepoint`'s.
+ # This is what's needed when a rollback _to_ `savepoint` is done.
+ def _remove_and_invalidate_after(self, savepoint):
+ savepoint2index = self._savepoint2index
+ index = savepoint2index.pop(savepoint)
# use items to make copy to avoid mutating while iterating
- for ref, i in savepoints.items():
+ for savepoint, i in savepoint2index.items():
if i > index:
- savepoint = ref()
- if savepoint is not None:
- savepoint.transaction = None # invalidate
- del savepoints[ref]
-
- def _invalidate_savepoints(self):
- savepoints = self._savepoints
- for ref in savepoints:
- savepoint = ref()
- if savepoint is not None:
savepoint.transaction = None # invalidate
+ del savepoint2index[savepoint]
- savepoints.clear()
+ # Invalidate and forget about all savepoints.
+ def _invalidate_all_savepoints(self):
+ for savepoint in self._savepoint2index.keys():
+ savepoint.transaction = None # invalidate
+ self._savepoint2index.clear()
def register(self, obj):
@@ -375,8 +360,8 @@
def commit(self, subtransaction=False):
- if self._savepoints:
- self._invalidate_savepoints()
+ if self._savepoint2index:
+ self._invalidate_all_savepoints()
if subtransaction:
# TODO deprecate subtransactions
@@ -492,8 +477,8 @@
self._subtransaction_savepoint.rollback()
return
- if self._savepoints:
- self._invalidate_savepoints()
+ if self._savepoint2index:
+ self._invalidate_all_savepoints()
self._synchronizers.map(lambda s: s.beforeCompletion(self))
@@ -647,11 +632,10 @@
return self._datamanager.sortKey()
class Savepoint:
- """Transaction savepoint
+ """Transaction savepoint.
Transaction savepoints coordinate savepoints for data managers
participating in a transaction.
-
"""
interface.implements(interfaces.ISavepoint)
@@ -678,7 +662,7 @@
if transaction is None:
raise interfaces.InvalidSavepointRollbackError
self.transaction = None
- transaction._invalidate_next(self)
+ transaction._remove_and_invalidate_after(self)
try:
for savepoint in self._savepoints:
Modified: ZODB/branches/3.4/src/transaction/savepoint.txt
===================================================================
--- ZODB/branches/3.4/src/transaction/savepoint.txt 2005-05-03 12:59:42 UTC (rev 30234)
+++ ZODB/branches/3.4/src/transaction/savepoint.txt 2005-05-03 21:29:40 UTC (rev 30235)
@@ -24,7 +24,7 @@
simple. It provides flat storage of named immutable values, like strings
and numbers.
-
+
>>> import transaction.tests.savepointsample
>>> dm = transaction.tests.savepointsample.SampleSavepointDataManager()
>>> dm['name'] = 'bob'
@@ -49,7 +49,7 @@
It accepts a sequence of entries and generates a sequence of status
messages. For each entry, it applies the change and then validates
the user's account. If the user's account is invalid, we role back
-the change for that entry. The success or failure of an entry is
+the change for that entry. The success or failure of an entry is
indicated in the output status. First we'll initialize some accounts:
>>> dm['bob-balance'] = 0.0
@@ -65,7 +65,7 @@
... raise ValueError('Overdrawn', name)
And a function to apply entries. If the function fails in some
-unexpected way, it rolls back all of it's changes and
+unexpected way, it rolls back all of it's changes and
prints the error:
>>> def apply_entries(entries):
@@ -107,7 +107,7 @@
>>> dm['sally-balance']
-80.0
-
+
If we give provide entries that cause an unexpected error:
>>> apply_entries([
@@ -120,7 +120,7 @@
Updated sally
Unexpected exception unsupported operand type(s) for +=: 'float' and 'str'
-Because the apply_entries used a savepoint for the entire function,
+Because the apply_entries used a savepoint for the entire function,
it was able to rollback the partial changes without rolling back
changes made in the previous call to apply_entries:
@@ -199,7 +199,7 @@
TypeError: ('Savepoints unsupported', {'name': 'bob'})
>>> transaction.abort()
-
+
However, a flag can be passed to the transaction savepoint method to
indicate that databases without savepoint support should be tolerated
until a savepoint is roled back. This allows transactions to proceed
@@ -259,11 +259,11 @@
...
TypeError: ('Savepoints unsupported', {'name': 'sue'})
<BLANKLINE>
-
+
>>> transaction.abort()
-
+
After clearing the transaction with an abort, we can get on with new
-transactions:
+transactions:
>>> dm_no_sp['name'] = 'sally'
>>> dm['name'] = 'sally'
@@ -272,4 +272,4 @@
'sally'
>>> dm['name']
'sally'
-
+
Modified: ZODB/branches/3.4/src/transaction/tests/savepointsample.py
===================================================================
--- ZODB/branches/3.4/src/transaction/tests/savepointsample.py 2005-05-03 12:59:42 UTC (rev 30234)
+++ ZODB/branches/3.4/src/transaction/tests/savepointsample.py 2005-05-03 21:29:40 UTC (rev 30235)
@@ -29,10 +29,10 @@
This data manager stores named simple values, like strings and numbers.
"""
-
+
interface.implements(transaction.interfaces.IDataManager)
- def __init__(self, transaction_manager = None):
+ def __init__(self, transaction_manager=None):
if transaction_manager is None:
# Use the thread-local transaction manager if none is provided:
transaction_manager = transaction.manager
@@ -43,7 +43,7 @@
self.uncommitted = self.committed.copy()
# Our transaction state:
-
+ #
# If our uncommitted data is modified, we'll join a transaction
# and keep track of the transaction we joined. Any commit
# related messages we get should be for this same transaction
@@ -56,14 +56,14 @@
#######################################################################
# Provide a mapping interface to uncommitted data. We provide
# a basic subset of the interface. DictMixin does the rest.
-
+
def __getitem__(self, name):
return self.uncommitted[name]
-
+
def __setitem__(self, name, value):
self._join() # join the current transaction, if we haven't already
self.uncommitted[name] = value
-
+
def __delitem__(self, name):
self._join() # join the current transaction, if we haven't already
del self.uncommitted[name]
@@ -126,13 +126,13 @@
assert self.tpc_phase == 2, "Must be called in second phase of tpc"
self.committed = self.uncommitted.copy()
self._resetTransaction()
-
+
def tpc_abort(self, transaction):
assert transaction is self.transaction, "Must not change transactions"
assert self.tpc_phase is not None, "Must be called inside of tpc"
self.uncommitted = self.committed.copy()
self._resetTransaction()
-
+
#
#######################################################################
More information about the Zodb-checkins
mailing list