Merge in current trunk state.

branches/tim-simpler_connection/NEWS.txt

@@ -2,15 +2,30 @@ What's new in ZODB3 3.3 ?
 =========================
 Release date: DD-MMM-YYYY
 
+Tools
+-----
+
+New tool fsoids.py, for heavy debugging of FileStorages; shows all
+uses of specified oids in the entire database (e.g., suppose oid 0x345620
+is missing -- did it ever exist?  if so, when?  who referenced it?  when
+was the last transaction that modified an object that referenced it?
+which objects did it reference?  what kind of object was it?).
+ZODB/test/testfsoids.py is a tutorial doctest.
+
+
+What's new in ZODB3 3.3 ?
+=========================
+Release date: DD-MMM-YYYY
+
 Connection
 ----------
 
-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 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,
+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
+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::
 
     ConnnectionStateError: Cannot close a connection with a pending subtransaction
 
@@ -19,21 +34,48 @@ is raised.
 transaction
 -----------
 
-Growing pains:  ZODB 3.1 and 3.2 had a bug wherein Transaction.begin()
-didn't abort the current transaction if the only pending changes were in a
-subtransaction.  In ZODB 3.3, it's intended that transaction managers be
-used instead of invoking methods directly on Transaction objects, and
-calling begin() on a transaction manager didn't have this old bug.  However,
-Transaction.begin() still exists in 3.3, and it had a worse bug:  it never
-aborted the transaction (not even if changes were pending outside of
-subtransactions).  Transaction.begin() has been changed to abort the
-transaction, although it's still strongly recommended to invoke begin() on
-the relevant transaction manager instead.  For example,
+- 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.
+
+- If ReadConflictError was raised by an attempt to load an object with a
+  ``_p_independent()`` method that returned false, attempting to commit the
+  transaction failed to (re)raise ReadConflictError for that object.  Note
+  that ZODB intends to prevent committing a transaction in which a
+  ReadConflictError occurred; this was an obscure case it missed.
+
+- Growing pains:  ZODB 3.2 had a bug wherein ``Transaction.begin()`` didn't
+  abort the current transaction if the only pending changes were in a
+  subtransaction.  In ZODB 3.3, it's intended that a transaction manager be
+  used to effect ``begin()`` (instead of invoking ``Transaction.begin()``),
+  and calling ``begin()`` on a transaction manager didn't have this old
+  bug.  However, ``Transaction.begin()`` still exists in 3.3, and it had a
+  worse bug:  it never aborted the transaction (not even if changes were
+  pending outside of subtransactions). ``Transaction.begin()`` has been
+  changed to abort the transaction. ``Transaction.begin()`` is also
+  deprecated.  Don't use it.  Use ``begin()`` on the relevant transaction
+  manager instead.  For example,
+
+      >>> 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:
+
+      >>> txn = transaction.get()
+      >>> ...
+      >>> txn.begin()
+      >>> ...
+      >>> txn.commit()
+
+  can't work as intended is 3.3, because ``txn`` is no longer the current
+  Transaction object the instant ``txn.begin()`` returns.
 
-    import transaction
-    transaction.begin()
 
-if using the default ThreadTransactionManager (see news for 3.3a3 below).
 
 BTrees
 ------
@@ -44,20 +86,30 @@ version of Zope's Interface package, which doesn't even ship with ZODB.
 The latter in particular created problems, at least clashing with
 PythonCAD's Interface package.
 
-Tools
------
+POSException
+------------
 
-New tool fsoids.py, for heavy debugging of FileStorages; shows all
-uses of specified oids in the entire database (e.g., suppose oid 0x345620
-is missing -- did it ever exist?  if so, when?  who referenced it?  when
-was the last transaction that modified an object that referenced it?
-which objects did it reference?  what kind of object was it?).
-ZODB/test/testfsoids.py is a tutorial doctest.
+Collector #1488 (TemporaryStorage -- going backward in time).  This
+confusion was really due to that the detail on a ConflictError exception
+didn't make sense.  It called the current revision "was", and the old
+revision "now".  The detail is much more informative now.  For example,
+if the exception said:
 
+    ConflictError: database conflict error (oid 0xcb22,
+    serial was 0x03441422948b4399, now 0x034414228c3728d5)
 
-What's new in ZODB3 3.3 ?
-=========================
-Release date: DD-MMM-YYYY
+before, it now says:
+
+    ConflictError: database conflict error (oid 0xcb22,
+    serial this txn started with 0x034414228c3728d5 2002-04-14 20:50:32.863000,
+    serial currently committed 0x03441422948b4399 2002-04-14 20:50:34.815000)
+
+ConflictError
+-------------
+
+The undocumented ``get_old_serial()`` and ``get_new_serial()`` methods
+were swapped (the first returned the new serial, and the second returned
+the old serial).
 
 Tools
 -----
@@ -239,9 +291,9 @@ There is a new transaction package, which provides new interfaces for
 application code and for the interaction between transactions and
 resource managers.
 
-The top-level transaction package has functions commit(), abort(),
-get(), and begin().  They should be used instead of the magic
-get_transaction() builtin, which will be deprecated.  For example:
+The top-level transaction package has functions ``commit()``, ``abort()``,
+``get()``, and ``begin()``.  They should be used instead of the magic
+``get_transaction()`` builtin, which will be deprecated.  For example:
 
     >>> get_transaction().commit()
 
@@ -250,38 +302,38 @@ should now be written as
     >>> import transaction
     >>> transaction.commit()
 
-The new API provides explicit transaction manager objects.  The
-transaction manager (TM) is responsible for associating resource
-managers with a "current" transaction.  It is available as
-`transaction.manager`.  The default TM, implemented by
-ThreadedTransactionManager, assigns each thread its own current
-transaction.  The TransactionManager class assigns all threads to the
-same transaction.
+The new API provides explicit transaction manager objects.  A transaction
+manager (TM) is responsible for associating resource managers with a
+"current" transaction.  The default TM, implemented by class
+``ThreadedTransactionManager``, assigns each thread its own current
+transaction.  This default TM is available as ``transaction.manager``.  The
+``TransactionManager`` class assigns all threads to the same transaction,
+and is an explicit replacement for the ``Connection.setLocalTransaction()``
+method:
 
-A transaction manager instance can be passed as the txn_mgr argument
-to DB.open().  If you do, the connection will use the specified
-transaction manager instead of the default transaction manager.  You
-will need to call commit() and abort() on the transaction manager
-explicitly.  For example:
+A transaction manager instance can be passed as the txn_mgr argument to
+``DB.open()``.  If you do, the connection will use the specified
+transaction manager instead of the default TM.  The current transaction is
+obtained by calling ``get()`` on a TM. For example:
 
     >>> tm = transaction.TransactionManager()
     >>> cn = db.open(txn_mgr=tm)
     [...]
-    >>> tm.commit()
+    >>> tm.get().commit()
 
-The setLocalTransaction() and getTransaction() methods of Connection
-are deprecated.  Use an explicit TM passed via txn_mgr instead.  The
-setLocalTransaction() manager functions still works, but it returns a
-TM instead of a Transaction.
+The ``setLocalTransaction()`` and ``getTransaction()`` methods of
+Connection are deprecated.  Use an explicit TM passed via ``txn_mgr=`` to
+``DB.open()`` instead.  The ``setLocalTransaction()`` method still works,
+but it returns a TM instead of a Transaction.
 
-The TM creates Transaction objects, which are used for exactly one
-transaction.  They have a status() method that returns their current
-state.
+A TM creates Transaction objects, which are used for exactly one
+transaction.  Transaction objects still have ``commit()``, ``abort()``,
+``note()``, ``setUser()``, and ``setExtendedInfo()`` methods.
 
-Resource managers, e.g. Connection or RDB adapter, should use join()
-instead of register().  An object that calls join() manages its own
-resources.  An object that calls register() expects the TM to manage
-the objects.
+Resource managers, e.g. Connection or RDB adapter, should use a
+Transaction's ``join()`` method instead of its ``register()`` method.  An
+object that calls ``join()`` manages its own resources.  An object that
+calls ``register()`` expects the TM to manage the objects.
 
 Data managers written against the ZODB 4 transaction API are now
 supported in ZODB 3.

branches/tim-simpler_connection/src/ZODB/Connection.py

@@ -229,6 +229,14 @@ class Connection(ExportImport, object):
         self._inv_lock = threading.Lock()
         self._invalidated = d = {}
         self._invalid = d.has_key
+
+        # We intend to prevent committing a transaction in which
+        # ReadConflictError occurs.  _conflicts is the set of oids that
+        # experienced ReadConflictError.  Any time we raise ReadConflictError,
+        # the oid should be added to this set, and we should be sure that the
+        # object is registered.  Because it's registered, Connection.commit()
+        # will raise ReadConflictError again (because the oid is in
+        # _conflicts).
         self._conflicts = {}
 
         # If MVCC is enabled, then _mvcc is True and _txn_time stores
@@ -907,6 +915,7 @@ class Connection(ExportImport, object):
             finally:
                 self._inv_lock.release()
         else:
+            self._conflicts[obj._p_oid] = 1
             self._register(obj)
             raise ReadConflictError(object=obj)
 

branches/tim-simpler_connection/src/ZODB/POSException.py

@@ -15,7 +15,7 @@
 
 $Id$"""
 
-from ZODB.utils import oid_repr, serial_repr
+from ZODB.utils import oid_repr, readable_tid_repr
 
 def _fmt_undo(oid, reason):
     s = reason and (": %s" % reason) or ""
@@ -48,8 +48,8 @@ class ConflictError(TransactionError):
       serials : (string, string)
         a pair of 8-byte packed strings; these are the serial numbers
         related to conflict.  The first is the revision of object that
-        is in conflict, the second is the revision of that the current
-        transaction read when it started.
+        is in conflict, the currently committed serial.  The second is
+        the revision the current transaction read when it started.
       data : string
         The database record that failed to commit, used to put the
         class name in the error message.
@@ -95,8 +95,11 @@ class ConflictError(TransactionError):
         if self.class_name:
             extras.append("class %s" % self.class_name)
         if self.serials:
-            extras.append("serial was %s, now %s" %
-                          tuple(map(serial_repr, self.serials)))
+            current, old = self.serials
+            extras.append("serial this txn started with %s" %
+                          readable_tid_repr(old))
+            extras.append("serial currently committed %s" %
+                          readable_tid_repr(current))
         if extras:
             return "%s (%s)" % (self.message, ", ".join(extras))
         else:
@@ -109,10 +112,10 @@ class ConflictError(TransactionError):
         return self.class_name
 
     def get_old_serial(self):
-        return self.serials[0]
+        return self.serials[1]
 
     def get_new_serial(self):
-        return self.serials[1]
+        return self.serials[0]
 
     def get_serials(self):
         return self.serials

branches/tim-simpler_connection/src/ZODB/tests/testZODB.py

@@ -286,6 +286,10 @@ class ZODBTests(unittest.TestCase):
         # transaction.
         if shouldFail:
             self.assertRaises(ReadConflictError, lambda: obj.child1)
+            # And since ReadConflictError was raised, attempting to commit
+            # the transaction should re-raise it.  checkNotIndependent()
+            # failed this part of the test for a long time.
+            self.assertRaises(ReadConflictError, tm2.get().commit)
         else:
             # make sure that accessing the object succeeds
             obj.child1
@@ -382,38 +386,57 @@ class ZODBTests(unittest.TestCase):
         # transaction, and, in fact, when this test was written,
         # Transaction.begin() didn't do anything (everything from here
         # down failed).
-        cn = self._db.open()
-        rt = cn.root()
-        rt['a'] = 1
-
-        transaction.get().begin()  # should abort adding 'a' to the root
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
-
-        # A longstanding bug:  this didn't work if changes were only in
-        # subtransactions.
-        transaction.get().begin()
-        rt = cn.root()
-        rt['a'] = 2
-        transaction.get().commit(1)
 
-        transaction.get().begin()
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
+        # Oh, bleech.  Since Transaction.begin is also deprecated, we have
+        # to goof around suppressing the deprecation warning.
+        import warnings
 
-        # One more time, mixing "top level" and subtransaction changes.
-        transaction.get().begin()
-        rt = cn.root()
-        rt['a'] = 3
-        transaction.get().commit(1)
-        rt['b'] = 4
+        # First verify that Transaction.begin *is* deprecated, by turning
+        # the warning into an error.
+        warnings.filterwarnings("error", category=DeprecationWarning)
+        self.assertRaises(DeprecationWarning, transaction.get().begin)
+        del warnings.filters[0]
 
-        transaction.get().begin()
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
-        self.assertRaises(KeyError, rt.__getitem__, 'b')
+        # Now ignore DeprecationWarnings for the duration.  Use a
+        # try/finally block to ensure we reenable DeprecationWarnings
+        # no matter what.
+        warnings.filterwarnings("ignore", category=DeprecationWarning)
+        try:
+            cn = self._db.open()
+            rt = cn.root()
+            rt['a'] = 1
+
+            transaction.get().begin()  # should abort adding 'a' to the root
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+
+            # A longstanding bug:  this didn't work if changes were only in
+            # subtransactions.
+            transaction.get().begin()
+            rt = cn.root()
+            rt['a'] = 2
+            transaction.get().commit(1)
+
+            transaction.get().begin()
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+
+            # One more time, mixing "top level" and subtransaction changes.
+            transaction.get().begin()
+            rt = cn.root()
+            rt['a'] = 3
+            transaction.get().commit(1)
+            rt['b'] = 4
+
+            transaction.get().begin()
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+            self.assertRaises(KeyError, rt.__getitem__, 'b')
+
+            cn.close()
 
-        cn.close()
+        finally:
+            del warnings.filters[0]
 
 def test_suite():
     return unittest.makeSuite(ZODBTests, 'check')

branches/tim-simpler_connection/src/ZODB/tests/testfsoids.py

@@ -44,11 +44,11 @@ There's not a lot interesting in an empty DB!
 >>> t.run()
 >>> t.report()
 oid 0x00 <unknown> 0 revisions
-    this oid was neither defined nor referenced
+    this oid was not defined (no data record for it found)
 oid 0x01 <unknown> 0 revisions
-    this oid was neither defined nor referenced
+    this oid was not defined (no data record for it found)
 oid 0x123456 <unknown> 0 revisions
-    this oid was neither defined nor referenced
+    this oid was not defined (no data record for it found)
 
 That didn't tell us much, but does show that the specified oids are sorted
 into increasing order.
@@ -65,7 +65,7 @@ oid 0x00 persistent.mapping.PersistentMapping 1 revision
         tid description='initial database creation'
         new revision persistent.mapping.PersistentMapping at 52
 oid 0x01 <unknown> 0 revisions
-    this oid was neither defined nor referenced
+    this oid was not defined (no data record for it found)
 
 So we see oid 0 has been used in our one transaction, and that it was created
 there, and is a PersistentMapping.  4 is the file offset to the start of the
@@ -151,7 +151,7 @@ oid 0x01 BTrees._OOBTree.OOBTree 2 revisions
         new revision BTrees._OOBTree.OOBTree at 491
         references 0x00 <unknown> at 491
 oid 0x02 <unknown> 0 revisions
-    this oid was neither defined nor referenced
+    this oid was not defined (no data record for it found)
 
 Note that we didn't create any new object there (oid 2 is still unused), we
 just made oid 1 refer to oid 0.  Therefore there's a new "new revision" line

branches/tim-simpler_connection/src/ZODB/utils.py

@@ -15,7 +15,6 @@
 import sys
 import time
 from struct import pack, unpack
-from types import StringType
 from binascii import hexlify
 import cPickle
 import cStringIO
@@ -34,6 +33,7 @@ __all__ = ['z64',
            'tid_repr',
            'positive_id',
            'get_refs',
+           'readable_tid_repr',
           ]
 
 z64 = '\0'*8
@@ -89,7 +89,7 @@ def newTimeStamp(old=None,
 
 
 def oid_repr(oid):
-    if isinstance(oid, StringType) and len(oid) == 8:
+    if isinstance(oid, str) and len(oid) == 8:
         # Convert to hex and strip leading zeroes.
         as_hex = hexlify(oid).lstrip('0')
         # Ensure two characters per input byte.
@@ -104,6 +104,15 @@ def oid_repr(oid):
 serial_repr = oid_repr
 tid_repr = serial_repr
 
+# For example, produce
+#     '0x03441422948b4399 2002-04-14 20:50:34.815000'
+# for 8-byte string tid '\x03D\x14"\x94\x8bC\x99'.
+def readable_tid_repr(tid):
+    result = tid_repr(tid)
+    if isinstance(tid, str) and len(tid) == 8:
+        result = "%s %s" % (result, TimeStamp(tid))
+    return result
+
 # Addresses can "look negative" on some boxes, some of the time.  If you
 # feed a "negative address" to an %x format, Python 2.3 displays it as
 # unsigned, but produces a FutureWarning, because Python 2.4 will display

branches/tim-simpler_connection/src/transaction/_transaction.py

@@ -136,6 +136,7 @@ XXX This code isn't tested.
 import logging
 import sys
 import thread
+import warnings
 
 _marker = object()
 
@@ -230,6 +231,9 @@ class Transaction(object):
                 self._resources.append(adapter)
 
     def begin(self):
+        warnings.warn("Transaction.begin() should no longer be used; use "
+                      "the begin() method of a transaction manager.",
+                      DeprecationWarning)
         if (self._resources or
               self._sub or
               self._nonsub or