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

On Fri, Jun 17, 2016 at 7:31 AM, Julien Muchembled <[email protected]> wrote:
> Le 06/16/16 22:29, Jim Fulton a écrit :
>>> You can find the monkey-patch at
>>>   https://lab.nexedi.com/nexedi/neoppod/blob/v1.6.3/neo/client/__init__.py#L35
>>>
>>> https://lab.nexedi.com/nexedi/neoppod/blob/v1.6.3/ZODB.patch
>>> contains other changes:
>>> - update of IStorage.tpc_finish doc string
>>> - update of existing unit tests
>>
>> This is the 21st Century. You should start submitting pull requests. :)
>
> Sure, when I'm actually decided to work on this. And I also realized that our patch was not ready.
>
>> I'm with you.  I'm planning to make ZEO5 like NEO in this regard and
>> there's even a new comment in ZODB that alludes to this.
>
> :)
>
>> Let's change ZODB5 to make this easier/saner for NEO and future ZEO.
>>
>> Let's think of how we **want** this to work and work out a transition
>> plan.  I'd like to do this in a way that doesn't break existing
>> storages if practical.
>>
>> Here's a straw man:
>>
>> - store, undo, and tpc_vote are no longer required to return serials,
>> except when they want to signal oids who's conflicts have been
>> resolved.
>>
>> - tpc_finish may optionally return a tid. If it does, then that tid
>> should be applied to modified objects who's without resolved
>> conflicts.
>>
>> - If tpc_finish doesn't return a tid, then store, undo, and tpc_vote
>> must return serials for all stored objects.
>
> I don't understand what you mean, because that's already what our patch does:
> - it does not touch store, undo, or tpc_vote
> - tpc_finish is not required to return a tid

I wasn't commenting on your patch. I was just describing how I think
it should work. I'm glad it matches the patch.

>
>> - There are some special cases where undo or tpc_vote should return
>> serials for without conflicts, because objects were modified via unto.
>> The serials in this case may be None, as long as tpc_finish returns a
>> tid.
>
> About the expected failure in NEO when checking undo (previous mail), it was actually a misuse of ZODB API in the test. The undo was done directly at the storage level and obviously oids were automatically invalidated.
>
> Anyway, since undo is not done via a connection, there's nothing to change here, and tpc_finish is not required to return a tid.
>
>> Eventually, it will be expected for tpc_finish to return a tid.
>>
>> Thoughts?
>
> Except for the TODO I listed in a previous mail, the patch looks acceptable.
> We can go further by adding deprecation warnings if we expected tpc_finish to return a tid.
> The return type of tpc_vote and store can also be simplified to a simple list and a boolean respectively.

I look forward to a PR!

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.