polygene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niclas Hedhman <nic...@hedhman.org>
Subject Re: JSONEntityState internal clone logic
Date Fri, 24 Jul 2015 05:01:15 GMT
The "this testcase" is referring to the pull request? I'll take a look a
bit later.... Checking the "Release" at the moment.

Cheers
Niclas

On Fri, Jul 24, 2015 at 12:14 PM, Tibor Mlynarik <tibor.mlynarik@gmail.com>
wrote:

> Hi Niclas,
>
> >
> > The "isStateNotCloned()" fix seems legit and a testcase for that would be
> > awesome (can't figure one out in my at this early hour)
>
> this testcase should be showing that "isStateNotCloned()" is broken.
> Change to entity in uow3, which is discarded, is visible to others, as
> cloning is not working.
>
> account3.balance().set( BigDecimal.TEN);
> uow3.discard() ;
>
> on other side, clone is executed with each set to entity, what is probably
> only unnecessary performance penalty.
>
> > As for the testcase, the bug that it exposes is that it doesn't prevent
> you
> > from using the "Account account1" references outside the UoW it was
> created
> > in.
>
>
> I see your point that accessing entity fetched from other uow ( closed
> one) should be prohibited.
> But maybe accessing just identity value should be allowed.
> But this is not what I wanted to report.
>
> thanks ,
>
>         Tibor
>
> On Jul 24, 2015, at 3:50 AM, Niclas Hedhman <niclas@hedhman.org> wrote:
>
> > I have not checked the code yet, but the clone is supposed to be there to
> > provide UoW isolation, i.e. a value pulled out of the cache, and modified
> > (any of the set methods) must not be visible into the cached state until
> > the UoW completes, at which time I presume the entity state is replaced
> in
> > the the cache.
> >
> > As for the testcase, the bug that it exposes is that it doesn't prevent
> you
> > from using the "Account account1" references outside the UoW it was
> created
> > in. That is the bug, and I would be happy to look into that (perhaps for
> > 3.0 just in case you depend on this fault behavior). If identity is used,
> > it is more likely working as expected.
> >
> >
> > The "isStateNotCloned()" fix seems legit and a testcase for that would be
> > awesome (can't figure one out in my at this early hour)
> >
> >
> > Thanks, Tibor
> >
> >
> > On Fri, Jul 24, 2015 at 12:31 AM, Tibor Mlynarik <
> tibor.mlynarik@gmail.com>
> > wrote:
> >
> >> Hi Paul,
> >>
> >> I have prepared test case to demonstrate issue.
> >> Problem is if there is enabled cache in json based entity store.
> >> Entities could be in wrong state.
> >>
> >> see here :
> >> https://github.com/apache/zest-qi4j/pull/2
> >>
> >> to make test success, fix :
> >>
> >> JSONEntityState
> >>    void cloneStateIfGlobalStateLoaded()
> >>    {
> >>        if( ! isStateNotCloned() )
> >>
> >>
> >> there is one more fix needed in JSONMapEntityStoreMixin#fetchCachedState
> >> method .
> >>
> >> Note: I was not able to configure entity store with cache service in one
> >> layer ,
> >> probably because of circular dependency: store uses cache, cache need
> >> store for config .
> >>
> >> cheers,
> >>
> >>        Tibor
> >>
> >>
> >> On Jul 23, 2015, at 1:49 PM, Paul Merlin <paul@nosphere.org> wrote:
> >>
> >>> Hi Tibor,
> >>>
> >>> Tibor Mlynarik a écrit :
> >>>> Hi,
> >>>>
> >>>> I have noticed code in JSONEntityState that seems suspicious for me.
> >>>>
> >>>> In method JSONEntityState#cloneStateIfGlobalStateLoaded
> >>>> if parts are swapped.
> >>>>
> >>>> I suppose that by global state is meant copy taken from cache that is
> >> shared across UoWs.
> >>>> And before first entity change own copy for UoW should be cloned.
> >>>>
> >>>> But as it is now, cloning is not done at first change and redundantly
> >> executed with each entity change.
> >>>> Also maybe whole cloning could be avoided if cache is not used.
> >>>>
> >>>> Or am I wrong in understanding of purpose of this state clone ?
> >>> IIRC this is the sole purpose of this state clone.
> >>> As you pointed out there may be room for improvements here.
> >>>
> >>> Niclas, WDYT?
> >>>
> >>> /Paul
> >>>
> >>
> >>
> >
> >
> > --
> > Niclas Hedhman, Software Developer
> > http://zest.apache.org - New Energy for Java
>
>


-- 
Niclas Hedhman, Software Developer
http://zest.apache.org - New Energy for Java

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message