[ZODB-Dev] Clearing sticky RCEs after a transaction abort

[Tim Peters]

[Chris McDonough]
> Errr.. hi again Tim.

Hey, you didn't type at me for several hours!  I was getting worried <wink>.

> Michael Dunstan and I worked out that a transaction terminated via
> "Transaction.abort()" when a previous "Transaction.commit()" has not been
> called on the same transaction does not appear to properly clear its
> associated jars' "sticky read conflict" states.  The main symptom of this
> bug are bogus read conflict errors during a *subsequent*
> transaction.commit().
>
> It appears that the only place that the _conflicts set associated with a
> connection is cleared wholesale are during a Connection's tpc_abort or
> tpc_finish methods, but no tpc_ methods are called when a transaction is
> aborted, so the _conflicts dict is never cleared for aborted
> transactions.  Since the connection is reused, this is meaningful.

Looks right to me.  Thanks!  It's meaningful even if you don't close the
connection, of course.

> I'm not sure of the best place to put a fix.  The obvious place is to put
> a self._conflicts.clear() in Connection's "abort" method.  This indeed
> does seem to fix the problem, but I'm not sure if it's optimal.

It sucks, but I don't know of anything better.  Connection.abort() appears
to be the only jar method we're sure Transaction.abort() will invoke (and in
the ReadConflictError case, we know it will call Connection.abort(obj) at
least for the obj that experienced the RCE, since Connection goes to great
pains to ensure that's so).

> Here's a unit test method that can be stitched into the
> ZODB.tests.testZODB.ZODBTests class for 3.2 that I *think* demonstrates
> the problem.

Thanks again.  I'll put something like this in.

[Dieter Maurer]
> That's why I have put (in our private copy of "Connection.py")
> "flush_invalidation" calls in "_setDB"

ZC's Connection._setDB() also calls _flush_invalidations() (assuming that's
what "flush_invalidation" meant).

> and the "abort" code when the connection itself has registered.
>
>     def abort(self, object, transaction):
> 	...
>         if object is self:
>             # Note: when a transaction with subtransactions is aborted
>             #  "self._tmp is None" will be false.
>             #  However, this should not be a serious problem
>             self._flush_invalidations(self._tmp is None)

Maybe you can explain this:  what good is the "if object is self" branch?
Is it documented anywhere what the heck registering a jar with a transaction
is supposed to mean?  The UML model for Connection is the only doc I know
about, and that doesn't even hint that this might be a legitimate thing to
do.  To the contrary, it says the `object` argument must be an instance of
Persistent, and a Connection isn't.  The UML model for
Transaction.register(obj) says obj must have a _p_jar attribute (although it
doesn't say obj must be an instance of Persistent, so maybe that's another
distinction that escapes me), but again that's not true of a Connection.

In any case, I don't see how this could help Chris, since the "object is
self" branch is apparently almost never taken.  I ripped it once to see what
would happen, and I think one test failed.  checkCrossDBInvalidations() does
Transaction.register(Connection) once, but there's no comment explaining
what it thinks that's supposed to accomplish.

The obvious answer is "registering a jar with a transaction causes
jar._flush_invalidations() to get called if Transaction.abort() is called",
but that leaves me without a fulfilling sense of understanding <wink>.

> Our "_flush_invalidations" has an additional argument which controls
> whether sticky object can be garbage collected (ZODB extension posted to
> "zodb-dev", but almost surely lost ;-) ).

I don't know whether zodb-dev ever functioned as a tracker, but it sure
hasn't since I've been here.  Post patches on the Collector!  They may or
may not get more attention then, but at least they won't be lost forever.