[ZODB-Dev] RE: [Zope-Annce] ZODB 3.2.4 release candidate 1released
Chris McDonough
chrism at plope.com
Thu Sep 16 01:59:17 EDT 2004
Thanks again for taking the time on this...
On Wed, 2004-09-15 at 15:52, Tim Peters wrote:
> 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.
OK, so I will do nothing then for now.
< example of btrees exception case >
> > 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.
Sorry, iobtree does get committed at various points over its lifetime
but it's actually unknown whether it gets committed between the time a
set of keys are added to it and when the key error happens for a key
that is part of that set. I haven't been able to reproduce it
(unhelpful, I realize, but true).
> > - 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.
Yup, I didn't know if the value related to the missing key needed to be
materialized at all given the example. "No" is the answer I presume.
> > - 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.
OK, it's helpful to know what must be unghosted to do this operation in
any case.
> > - 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.
Right. Thanks for the explanation, also very helpful.
Curious if this code (present in BTree_getm of BTreeTemplate.c) could do
arguably the wrong thing in the face of a POSKeyError:
UNLESS (PyArg_ParseTuple(args, "O|O", &key, &d)) return NULL;
if ((r=_BTree_get(self, key, 0))) return r;
UNLESS (PyErr_ExceptionMatches(PyExc_KeyError)) return NULL;
>From a quick reading of the docs a PyErr_ExceptionMatches also compares
true if a base class matches. I mean, I guess it doesn't matter much
(eventually you get None back). But it'd be nice to know if a
POSKeyError resulted from this call instead of hiding it unintentionally
(if that's indeed what it does). But probably not critical at all.
> > <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]
Whew. That must have took some work to think up. ;-)
At very least, I now have a decoy explanation for now until we find the
next case.! ;-)
- C
More information about the ZODB-Dev
mailing list