[ZODB-Dev] TmpStore missing loadBefore

[Tim Peters]

[Tim Peters]
>> This was fixed a few weeks [ago], in rev 30825:
>>
>>     http://svn.zope.org/ZODB/branches/3.4/?rev=30825&view=rev

[Sidnei da Silva]
> Ah, there we go. So that was fixed after 3.4.0 was tagged, and Zope 2.8
> branch still points to the 3.4.0 tag.

All correct!

> Guess we need a new tag.

I'll make an internal 3.4.1a1 release and stitch that into Zope 2.8 branch
and Zope trunk tomorrow.  "Internal" means I skip most of the steps in a
full-blown release (will not create a ZODB tarball or Windows installers,
will not register it with PyPI, will not send a release announcement, ...).
I won't make any serious move toward releasing 3.4.1 before I detect serious
work elsewhere toward releasing Zope 2.8.1 or Zope 2.9 (ZODB releases are
driven by Zope releases).

>> If you don't care too much about the new test, it's just a matter of
>> adding the string "loadBefore" to this list in TmpStore.__init__:
>>
>>         for method in (
>>             'getName', 'new_oid', 'modifiedInVersion', 'getSize',
>>             'undoLog', 'versionEmpty', 'sortKey', 'loadBefore',
>>             ):
>>             setattr(self, method, getattr(storage, method))

> Good, I had came to the same result with test and all *wink*. Wish I had
> looked at checkins before.

Indeed, and you're even one of the few people subscribed to zodb-checkins
<wink>.  BTW, Jim & I conspired last Wednesday to stop the flow of spam
coming from that list, so if anyone unsubscribed in a fit of Spam Hatred,
feel free to sign up again.

>>> Wouldn't it be simpler to make TmpStore subclass from FileStorage

>> No, because TmpStore is used by Connection (to support savepoints), and
>> Connection can't make assumptions about the _kind_ of storage in use.

> Humm... I don't see where the assumption would be except for a import
> dependency.

For example, you do a savepoint (or subtransaction) when connected to ZEO,
and so are using ClientStorage (rather than FileStorage directly).  TmpStore
only knows about a handful of things, and, e.g., knows nothing about how to
do a loadBefore().  Passing it off to FileStorage's implementation of
loadBefore() would be plain wrong in this case, because FileStorage has no
idea how to talk to ZEO either.  It has to get passed off to
ClientStorage.loadBefore().  Likewise for any storage of type S where S !=
FileStorage -- TmpStore is just a layer to manage Connection savepoints,
which are done in a way independent of the underlying storage type.

> My fear is that TmpStore does basically what FileStorage does

Ah, maybe that's where we're getting off track.  Not really; TmpStore and
FileStorage both write pickles to files, and both keep a kind of index
structure, but those are shallow similarities.  TmpStore is about 60 lines
of code total, while FileStorage is almost 2100 in FileStorage.py alone (not
counting support modules only FileStorage uses); because TmpStore _must_
defer to the underlying storage for most operations, it couldn't even
usefully take more from FileStorage.

> but get way less attention and ends up bitrotting. This is not the first
> time I've hit a (shallow) bug in TmpStore that would just be triggered by
> a fairly big or long subtransaction/savepoint.

More tests would be appreciated!  Since nobody looks at most of this code
most of the time, tests are the only reliable defense we have.  Jim
"confessed" last week that he deliberately didn't write TmpStore tests
originally because TmpStore is "an internal implementation detail".  You'll
note that the new test I added didn't mention TmpStore either, except in
comments.  For that matter, the new test didn't use FileStorage either (it
used MinimalMemoryStorage, which is a bare-bones storage used only in the
test suite).  Tests specifically against TmpStore would be welcome.

> Anyway, you're probably in better position to judge this than I.

Possibly, but you're in a better position to judge that <wink>.