[ZODB-Dev] rasing a ReadConflictError

[Tim Peters]

[Michael Dunstan]
> I'm working on a fix for Transience (with the help of Chris). There is a
> condition in the code in which it makes sense to raise a
> ReadConflictError from Transience itself. However it would seem that
> there is some light magic required to ensure that this will survive
> swallowing of the exception. How about putting the magic into the
> __init__ for ReadConflictError?

IME, ZODB is delicate, so tread carefully.

To start with, why?  That is, ReadConflictError is defined as:

    """Conflict detected when object was loaded.

    An attempt was made to read an object that has changed in another
    transaction (eg. another thread or process).
    """

If that's not what's going on in Transience, then any patch to the code will
leave the docs even more out of synch with the code.  We can't afford more
of that, so I want to know more about why Transience thinks it *should* be
in the business of raising ReadConflictError.  It's dubious on the face of
it, since this exception was clearly (<heh>) intended to be raised by
Connection.

A clear story is needed also because this code is quite different in ZODB
3.3, and unless the need for new tricks is carefully established and
documented, we're going to fight the same problems all over again in 3.3
(the hard way, by stumbling into them again).

> Here is a patch to try and illustrate what I'm going on about (oh yeah -
> this is Zope-2_7-branch):
>
> Index: Connection.py
> =================================================================== RCS
> file: /cvs-repository/Packages/ZODB/Attic/Connection.py,v
> retrieving revision 1.98.4.7
> diff -u -r1.98.4.7 Connection.py
> --- Connection.py       27 Aug 2004 19:03:42 -0000      1.98.4.7
> +++ Connection.py       31 Aug 2004 18:29:54 -0000
> @@ -614,8 +614,6 @@
>                       # Defer _p_independent() call until state is loaded.
>                       return 1
>                   else:
> -                     self.getTransaction().register(obj)  
> -                     self._conflicts[obj._p_oid] = 1
>                       raise ReadConflictError(object=obj)
>               else:
>                   return 0

Connection raises ReadConflictError in at least 3 places, so it's unclear
why you're changing only one of them.  In one of those cases
(_handle_independent), it does not add the oid to _conflicts; it's unclear
whether that's a bug or a feature, but regardless _handle_independent()'s
behavior may change if raising ReadConflictError started doing so.  In the
other case (commit()):

        if self._conflicts.has_key(oid):
            self.getTransaction().register(object)
            raise ReadConflictError(object=object)

oid is already in _conflicts, so no need to add it.  But it's hard (for me,
anyway) to imagine why it's registering object here -- that is, how on Earth
could we have gotten into commit() if the object wasn't already registered
with the transaction?

Etc.  Every line of code you touch here blossoms into an exponentially
growing number of mysteries <0.8 wink>.

> Index: POSException.py
> =================================================================== RCS
> file: /cvs-repository/Packages/ZODB/Attic/POSException.py,v
> retrieving revision 1.20.4.3
> diff -u -r1.20.4.3 POSException.py
> --- POSException.py     27 Aug 2004 19:03:42 -0000      1.20.4.3
> +++ POSException.py     31 Aug 2004 18:29:54 -0000
> @@ -116,6 +116,8 @@
>           ConflictError.__init__(self, message="database read conflict
error",
>                                  object=object)
>           self.jar = object._p_jar
> +         self.jar.getTransaction().register(object)
> +         self.jar._conflicts[object._p_oid] = 1
>
>       def ignore(self):
>           self.jar._ignore_conflict(self.oid)


POSException.py certainly shouldn't be reaching into the guts of jar
internals (mutating self.jar._conflicts), so that has to go.  I don't know
whether anything other than Connection is used as a jar, but if something
else is, then *trying* to access self.jar._conflicts would die with
AttributeError.

So if this proceeds, jars will have to grow a new method (say,
_record_read_conflict(), possibly containing those two lines, or possibly
containing something cleverer, depending on the resolutions to the mysteries
identified above.