[ZODB-Dev] RE: [Zope-Annce] ZODB 3.2.4 release candidate 1released

[Tim Peters]

[Dieter Maurer]
>>> Let's cast it into a slightly different picture:
>>>
>>>     try:
>>>       try:
>>>         # perform modifications on persistent state
>>>       except ...
>>>         # do something that does not abort the transaction
>>>         # and does not raise an exception
>>>       ...
>>>       commit()
>>>     except:
>>>       abort()
>>>       ...
>>>
>>> Should the "perform modifications" result in any exception, then the
>>> exception is likely to leave the state inconsitent. Catching the
>>> exception causes "commit" to be called which makes the inconsistent
>>> state persistent.

[Chris McDonough]
> I don't think there's anything ZODB can do about the above case, is
> there?  I'm so confused my head is spinning, to be honest. ;-)

There isn't, and don't forget that application invariants can be broken even
if no exceptions occur (let alone get suppressed), and even if conflict
resolution doesn't get involved.  It depends on the app and its invariants.
It makes it hard to know exactly which problem(s) we're trying to solve
here.

> But I *think* that it's only in the case that the exception which is
> raised is a ConflictError or a commit failure does ZODB have any
> responsibility whatsoever, and that's only because both are very fatal
> and there's no way to prevent either from being caught inappropriately by
> overeager exception handlers.  The machinery to make sure that nothing
> "bad" happens to persistent state at commit time due to inappropriately
> catching a ZODB-related exception is basically a hack, isn't it?

I keep saying ReadConflictError because that's the only exception ZODB
arranged to "make stick" before 3.2.4c1 (ConnectionStateError got added
then, and persists until abort()/begin()/commit()).  There's no special
treatment of, e.g., VersionLockError or POSKeyError.  Maybe there should be;
I don't know.

AFAIK, write conflicts can happen only during commit(), and *any* unhandled
exception during commit() aborts the current transaction, so write conflicts
aren't special either.

WRT ReadConflictError, yes, that's made sticky via a hack spread out over 3
places in Connection.py.  It's so hackish that 2 of those 3 places had bugs
(which we discovered and fixed last week -- but they were minor bugs, one
with no visible consequences, and the other I bet never occurred before I
added a strained test for it).

> If Python had a separate exception hierarchy for errors that a bare
> except: (or C equivalent, if it exists, and hasattr too!) couldn't catch,
> I think ZODB could even relinquish responsibility for doing anything
> special when ReadConflictError happens.

Indeed, the only reason ZODB does anything special with ReadConflictError is
because you Zope weenies kept whining about it until Jeremy gave up asking
you to learn how to write decent code <wink>.

> By that time we'll all have MVCC anyway, I guess. ;-)  But it would seem
> to also prevent ZODB from needing to do anything special when a commit
> fails in the way we're jawing on about in the other thread too.  It would
> also probably be more efficient, because the transaction wouldn't need to
> hopelessly go on doing work that is preordained to fail at commit time.

That's the discussion on python-dev, which gained a little momentum, but
seems to have fizzled out already.  At the C level this is very difficult,
because PyErr_Clear() calls are *all over the place* (not just in Python!
ZODB 3.2.4c1's C code calls it 66 times).  Many of those are more-than-less
obviously safe, but you can burn an hour trying to guess about just one of
the others.  Like:

    oid = PyObject_GetAttr(object, py__p_oid);
    if (!oid) {
	PyErr_Clear();
	goto return_none;
    }

__getattr__ can do anything, so this is basically a duplication inside
ZODB's C code of the same problems Python's __builtin__.hasattr() creates.
Armin Rigo has a speculative patch pending that tries to get "dangerous"
exceptions raised *even if* PyErr_Clear() gets done at the C level.  Haven't
had any time to look at that, though.  There's no cause to hope for quick
relief here, anyway.