"Connetion.commit" makes object inaccessible

[d-maurer]

After a call to `ZODB.Connection.Connection.commit", new objects can become inaccessible (until the following "tpc_finish" call). The behaviour is demonstrated by the following doctest:

>>> # preliminaries
>>> from persistent import Persistent
>>> from ZODB.DB import DB
>>> import transaction
>>> 
>>> class P(Persistent): pass
... 
>>> def access(o):
...   """emulate access to *o*."""
...   o._p_deactivate() # simulate cache eviction
...   o._p_activate() # simulate access
... 
>>> # test `MappingStorage`
>>> from ZODB.MappingStorage import MappingStorage
>>> 
>>> db = DB(MappingStorage())
>>> T = transaction.get()
>>> c = db.open()
>>> o = P()
>>> c.add(o)
>>> T.savepoint()
<transaction._transaction.Savepoint instance ...>
>>> # access still works
>>> access(o)
>>> c.tpc_begin(T)
>>> # access still works
>>> access(o)
>>> c.commit(T)
>>> # access fails
>>> access(o)
No handlers could be found for logger "ZODB.Connection"
Traceback (most recent call last):
  ...
POSKeyError: 0x01
>>> c.tpc_vote(T)
>>> c.tpc_finish(T)
>>> # access works again
>>> access(o)
>>> T.abort()
>>> 
>>> 
>>> # test `FileStorage`
>>> from ZODB.FileStorage import FileStorage
>>> from tempfile import mktemp
>>> 
>>> tfn = mktemp()
>>> 
>>> db = DB(FileStorage(tfn))
>>> T = transaction.get()
>>> c = db.open()
>>> o = P()
>>> c.add(o)
>>> T.savepoint()
<transaction._transaction.Savepoint instance at ...>
>>> # access still works
>>> access(o)
>>> c.tpc_begin(T)
>>> # access still works
>>> access(o)
>>> c.commit(T)
>>> # access fails
>>> access(o)
Traceback (most recent call last):
  ...
POSKeyError: 0x01
c.tpc_vote(T)
c.tpc_finish(T)
# access works again
access(o)
T.abort()

The error happens, because the savepoint has given the Connection its own storage which serves the savepoint modifications. On commit, the savepoint modifications are stored in the primary storage and the savepoint storage is dropped. However, the typical primary storages (shown by the doctest for MappingStorage and FileStorage; I have verified that this applies as well to ClientStorage) are not ready to load stored objects before the tpc_finish.

The error can affect other resource managers (such as the collective.indexing.transactions.QueueTM in ticket zopefoundation/transaction#6) which access persistent objects.

The error shows some apparently non-deterministic behavior: the error only occurs when the accessed persistent object must be really loaded from the ZODB (and is not in the connection's cache).

New objects can become inaccessible by this error. On the other hand, the error can also affect existing objects. For them, resource managers can read the old state (and not see modifications by the current transaction).

[jimfulton]

As I mentioned in the other issue, having storage-level machinery depend on persistent state is a bad idea.

[d-maurer]

It definitely is unintuitive that a commit call causes the (temporary) inaccessiblity of an object (or the delivery of an outdated state).
And the error could likely be fixed on the Connection level rather than in "the storage machinery".

[jimfulton]

By "storage machinery" I was being imprecise. My point was that what happens during transaction commit should not involve application logic.

If you think there's a clean simple "fix", I'll entertain a pull request.

[d-maurer]

I will hear what the authors of collective.indexing say to my problem report (plone/collective.indexing#8). If they think, they can avoid accessing persistent objects during (at least) QueueTM.tpc_finish, we can let things for the time being.
Otherwise, I may explore a fix in a dm.zodbpatches. package. My idea looks like: Connection uses a special multiplexing storage to implement savepoint. This loads first from savepoint modified state, then from the primary storage. Currently, the connection drops the multiplexed storage on commit reverting to the primary storage. I think keeping the multiplexing storage a bit longer (until tpc_finish) will ensure that the connection can deliver correct state throughout the transaction process.

[jimfulton]

+1 for dm.zodbpatches.

P.S. Dieter, it's great to hear from you. :)

[d-maurer]

The package containing the monkey patches for this problem is dm.zodbpatches.commit_savepoint.