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

[Tim Peters]

[Chris McDonough]
> [Chris McDonough]
>> I just wish it didn't subclass KeyError.  Maybe instead of going on a
>> sticky jihad I should make a ZODB branch that had a POSKeyError that
>> didn't subclass KeyError.  Would it be reasonable to make this change
>> for 3.3 or should I just forget it?

It's too late for 3.3(.0), and since this would be a major incompatible
change (not "a bugfix") anyway, it would have to target 3.4 (not 3.3.1).
Note that the storage API documents that storages raise KeyError.  Most
storage implementations would have to change, and so would all code mucking
with storages (like Connection.py).  There are lots of storages that do
raise KeyError (not POSKeyError) now; inside ZODB, FileStorage is the only
storage that raises POSKeyError instead of plain KeyError.  I agree it would
be better if everything did switch to POSKeyError, though.

> ... and paranoia overtakes me. ;-)

Paranoia is healthy.

> I'm trying to figure out a reported error case for Transience that
> involves a problem that manifests itself in a symptom like this:
>
> iobtree = IOBTree()
> iobtree[1] = 1
> iobtree[2] = 2
> keys = list(iobtree.keys(None, 100))
> for key in keys:
>     del iobtree[key]
> KeyError: 1
>
> This symptom has in the past been attributable to mutating the items in a
> BTree while iterating over a BTreeItems doodad.  But I don't do that
> anymore (note the list())  and I'm wondering if this sounds like a
> plausible series of events under Zope-2.7.2 + its version of ZODB that
> might explain it:

Not really, but I'll give you a concrete, demonstrably broken scenario
later.  A key missing detail in the above is whether iobtree ever gets
committed, or whether it's solely in memory over its lifetime.

> - a reference to a soon-to-be-unreachable object is created
>   via the "pending modifications in subtransactions reused
>   from cache when txn not committed or aborted" bug.  This
>   object is referenced in a leaf node by a bucket of an
>   IOBTree and its key is also kept by the IOBTree itself in
>   its interior node structure.

Not entirely clear what you have in mind there, but the plausible readings I
came up with flop because this is an IOBTree, and the test case only looks
at its *keys*.  Integer keys aren't persistent objects in their own right,
they're embedded directly in IO bucket and btree nodes.

> - the unreachable object is eventually ghosted in some thread

So long as we're talking about integer keys, they never get ghosted.  A
bucket containing them can be ghosted, though.  When the bucket is loaded,
all the keys are materialized at the same time (because they're integers,
not because "they're keys").

> - The IOBTree's keys() method is called.  It returns what it thinks
>   is the set of keys it has.

It actually returns a funky fixed-sized descriptor, containing pointers to
the first and last buckets in the range.  The list() call makes this
irrelevant, though -- that returns a (non-persistent) fully materialized
Python list containing all the integer keys.

> - The set of keys is iterated over and __delitem__ is called on
>   the BTree with each key.  When it reaches the ghosted object and
>   tries to unghost it, a POSKeyError is generated.  (does it *need*
>   to unghost it?)

The only relevant things that *can* be ghosted here are IOBTree and IOBucket
nodes.  Both certainly have to be unghosted to peer into their contents.

> - Whatever code implements __delitem__ on a BTree catches the
>   POSKeyError and reraises it as a KeyError

That one is unlikely in the C code, despite that Python dicts are unable to
report errors correctly.  The BTree lookup code has nothing in common with
dict lookup code; there's no try/except in C, so suppressing errors by
accident is less likely than when writing in Python; and, most importantly,
while you don't have to pay much attention to what is and isn't persistent
in Python code, in C the distinction *always* has to made, on every line of
C code.  Unghostifying doesn't happen by magic at the C level, you have to
call something explicitly to make that happen.  So POSKeyError from an
attempted load at the C level isn't "hiding", it punches you in the face.
Every line of C code that needs to look inside a persistent object has
something like this first:

    PER_USE_OR_RETURN(self, NULL);

It's remarkably unhelpful to know what that expands to:

	{if((self)->state==cPersistent_GHOST_STATE &&
       cPersistenceCAPI->setstate((PyObject*)(self)) < 0)
           return (NULL);
       else if ((self)->state==cPersistent_UPTODATE_STATE)
          (self)->state=cPersistent_STICKY_STATE;
      };

That point is that failure to load is a very distinct cause of failure in
C-level code.

> <waves hands furiously>

Here's code that breaks, provided you use a pre-repaired ZODB 3.2.  It's a
minor variant of code I posted before.

import ZODB
from BTrees.IOBTree import IOBTree
from ZODB.FileStorage import FileStorage

st = FileStorage("temp.fs")
db = ZODB.DB(st )
cn = db.open()
rt = cn.root()

tree = rt['tree'] = IOBTree([(1, 2), (2, 4)])
get_transaction().commit()

# It's vital here that *some* state of `tree` got
# committed.  Then when phantom cache state disappears
# later, instead of getting a POSKeyError, we magically
# get back the old state.

# Do the fatally confused "subtxn commit, close, open" dance.

tree[3] = 6
get_transaction().commit(1)
cn.close()

cn = db.open()
rt = cn.root()

tree = rt['tree']

# Now `tree` has 3 elements in cache, but only 2 elements in
# the database.

keys = list(tree.keys())
print "current keys", keys  # prints [1, 2, 3]

# Oops.  Simulate the cache getting cleaned wrt `tree`.
tree._p_deactivate()

# This one is entirely different:  the BTree gets restored from
# the database, which only had two keys.
new_keys = list(tree.keys())

for k in keys:
    try:
        del tree[k]
    except KeyError:
        # This triggers!  Key 3 vanished.
        print "oops!  key", k, "is missing now"
        print "the keys actually were", new_keys

If you run that, it prints:

    current keys [1, 2, 3]
    oops!  key 3 is missing now
    the keys actually were [1, 2]