Re: [dev] RFC IMVCCStorage everywhere (Move ZODB's MVCC to storage layer)

On Thu, Jun 16, 2016 at 6:34 AM, Julien Muchembled <[email protected]> wrote:
> Hello Jim,
>
> I see you'd like not to delay the release of ZODB5.

I have no desire to rush its release. I won't need it until I'm ready
to release ZEO 5, and that's at least a week or 2 off.

> I have started to read the adapter code more carefully. I wish I had more time to do that earlier.

Note that it is mainly a refactoring. It doesn't change any external
semantics.  It doesn't change how ZODB interacts with storages (except
for RelStorage where unchanges some things :) ).

> Since ZODB5 is a major release because it is more strict on what loadBefore/sync/lastTransaction must do, it could be a good idea to do other backward incompatible changes.

I'm not sure that follows.

> 1. MVCCAdapter
>
> I'm not enthusiast with so many lines of glue code and extra function calls.

I don't think there are more lines of code or more function calls. The
MVCCAdapter work just moved some code around.

> I would understand this way of implementing things if you'd like not to break existing code but we're already discussing of a major release.

The main impact of ZODB5, having nothing to do with IMVCCAdapter, is
on ZEO and NEO.  ZEO required meaningful, but minor changes. Given
that the changes reflect the way NEO already worked, I'm curious what
impact these changes have on NEO.  I would have thought none, or few.

> I'd rather merge IMVCCStorage into IStorage, and for ZODB implementations that don't implement IMVCCStorage, let them inherit a mixin (a mix of MVCCAdapter and MVCCAdapterInstance), the same way as it is done for ConflictResolvingStorage.

I don't consider ConflictResolvingStorage a success story. Generally,
I don't like providing public inheritance APIs. Conflict resolution
would have been better provided via composition.

Of course, there's nothing preventing any storage implementation from
implementing IMVCCStorage other than the fact that IMVCCStorage needs
more thought. In any case, I'm not sure what you'd end up with would
look much different than the adapters I've written.

On that last point, I think IMVCCStorage needs to be split into 2
separate interface.  IMO, there should be an interface for dealing
with non-transactional aspects and for creating the things now called
"instances". There should be a separate interface for the things now
called instances that deals with per-connection transactional
semantics.  The current IMVCC interface is based on an implementation
hack. (I don't mean to denigrate RelStorage with that remark, but want
to do something cleaner going forward.) I personally don't want to
take the time now to work this out now, but I wouldn't object if
someone else wanted to work on this.

> I think registerDB() would also disappear.

If ZEO and NEO implemented IMVCCStorage, then registerDB would largely
become an internal implementation detail.  It would still be needed by
ZEO servers to implement fan out, although ZEO fan out is largely an
academic feature.

registerDB should probably be made optional. It exists primarily to
support storages that generate invalidations asynchronously, although
there's also the transformation suppport.


> I think the result would be simpler, and ZODB implementations would have more freedom to tweak parts.

Well, this is largely possible now.

>
> 2. IStorage.lastOID() ?

You need to be more specific. This doesn't exist, so I guess you're
proposing this.

> We'd use it to migrate databases to NEO. Currently, we hack by calling new_oid(), without actually modifying the DB, which prevent us from opening a ZODB in read-only.
>
> I know you suggested a different way to implement transparent migration, but the memory cost is in O(number of oids) and it is quite a lot of non-trivial changes in NEO whereas the current implementation has already been well-tested.
>
> In ZODB, this would very little change, which I'm ready to do. Same for RelStorage (3 simple methods in adapters + 1 relstorage.py). For ZEO, a new simple rpc is needed.

I don't know what you're proposing exactly or what problem it's
solving. You need to be more specific.


> 3. deprecated stuff
>
> - getting rid off versions was already mentionned in PR 66, I'm ok with your answer
> - what about dropping the tpc_vote monkey-patch in DB.__init__ ?
> similarly in Connection.tpc_vote, there's a "try ... except" on _storage.tpc_vote
> - in Connection._store_objects, there's a TODO:deprecate then remove
> - DB._storage
> - IStorageDB/IStorageWrapper ?

I'd be OK with all of this, but the transition requires more planning
and care. This isn't something *I* am willing to spend time on now.  I
will take a little time this morning to look for low-hanging fruit
though.  Expect a PR later today.

I think the idea of making all of the breaking changes at once has
been proven to be a bad one by Python 3 and Zope 3.

Jim

-- 
Jim Fulton
http://jimfulton.info

-- 
You received this message because you are subscribed to the Google Groups "zodb" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.