... >> The most obvious failure is that the hook gets called on both commit() >> and abort(). I view that as a mistake in the design of >> beforeCompletion; there are currently no uses of beforeCompletion() >> anywhere in ZODB, so I doubt it would cause any pain to change that. >> Perhaps the hook method could be passed a string arg, 'commit' or >> 'abort'.
[Florent Guillaume] > Actually it can check txn.status to decide. Are you sure? I don't see it. .status isn't changed until after beforeCompletion() is called: 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 # subtransaction commit to clear out the current objects, # then commit all the subjars. self.commit(True) if not subtransaction: for s in self._synchronizers: s.beforeCompletion(self) self.status = Status.COMMITTING in 3.3, or in 3.4 the tail end is: if not subtransaction: self._synchronizers.map(lambda s: s.beforeCompletion(self)) self.status = Status.COMMITTING .status isn't changed before calling beforeCompletion() in the abort() case either. >> A subtler failure is that "are called for subsequent transactions until >> they are unregistered" isn't the full truth: the TM holds only a weak >> reference to a registered synchronizer. If such a synchronizer becomes >> otherwise unreferenced, it will vanish from the TM's weak set of >> registered synchronizers, and then its methods won't get invoked at all. >> If the caller arranges to hold a strong reference to a OneShot instance >> until the current transaction ends, then that part isn't an issue. > Why the weak refs? What does it help doing in the ZODB case? I'm not really sure -- gotta ask Jim. He said it was important to cater to people who open Connection objects but never close them. And, indeed, the Zope3 test suite sucked up enormous amounts of RAM precisely because some tests did exactly that, before all this synch stuff in transaction managers got rewritten to use "weak sets" of synchronizers instead of a list of synchronizers (and in the latter case, registering with a TM made a synchronizer effectively immortal). The change above between 3.3 and 3.4 is highly relevant here too. It turned out that materializing a list of the still-living elements of a weak set (in order to iterate over it) was unacceptable, because doing so has a very high chance of tricking Python's garbage collector into moving those elements into older generations, with the end result that synchronizers were staying alive for a very long time after all strong references went away. Weak sets grew their own .map() method then, materializing only one weakly-referenced thing at a time. There's more explanation in ZODB/utils.py WeakSet.map in the ZODB 3.4 branch. _______________________________________________ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev