polygene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kent Sølvsten <kent.soelvs...@gmail.com>
Subject Re: Bug found in UnitOfWork
Date Tue, 30 Jun 2015 07:09:37 GMT
I think you are right, since we have just repaired the unfortunate
coupling with module.


Den 30-06-2015 kl. 08:48 skrev Niclas Hedhman:
> EntityStoreUnitOfWork intends to capture all actions required for a given
> ES, within an UoW. I can't see how this could possibly be scrapped, but
> willing to entertain the idea...
>
> // Niclas
>
> On Mon, Jun 29, 2015 at 11:37 PM, Kent Sølvsten <kent.soelvsten@gmail.com>
> wrote:
>
>> Great job!
>>
>> Actually I think this change is soooo obviously correct from a design
>> perspective. Since the "module" used is not the "structural" module
>> attached to the entitystore, but the "calling module", it simply seems
>> wrong to couple those together in the EntityStoreUnitOfWork concept.
>>
>> I even start to wonder if the concept of EntityStoreUnitOfWork should
>> simply be scrapped ?
>>
>> Another note: In the codebase it is sometimes not very obvious when
>> seeing a module reference, whether we are considering a
>> "structurally-attached module" (the one holding the entity store used
>> for saving the entities) or just looking at a "calling module" (often
>> used to lookup xxxDescriptors). DCI used the concept of roles - could it
>> be useful in Qi4J ? Sometimes we are using a module as a
>> "DescriptorSource" - other times we are using it as an
>> "EntityStoreHolder" .... or something like that.
>>
>> Just toying with ideas here :-)
>>
>> /Kent
>>
>>
>>
>> Den 25-06-2015 kl. 14:29 skrev Niclas Hedhman:
>>> And YES, the proposed approach and a handful of SPI interface changes
>> made
>>> my usecase work as expected.
>>>
>>> So, even if I had the wrong approach to my particular issue, the new
>>> behavior is probably as expected, so I think it should go into the
>>> codebase, and I think it might as well go in now, rather than later.
>>>
>>> EntityStore implementations will break at compile time, but the fix is
>>> relatively straight forward.
>>>
>>> Any other opinions?
>>>
>>> // Niclas
>>>
>>> On Thu, Jun 25, 2015 at 7:23 PM, Niclas Hedhman <niclas@hedhman.org>
>> wrote:
>>>> Kent,
>>>> I tried to implement the ModuleEntityStoreUnitOfWork idea. It is not as
>>>> smooth as it first seems, since the inner EntityStoreUnitOfWork needs to
>>>> call entityStateOf() which needs the module. BUT, by modifying a couple
>> of
>>>> SPI interfaces, it is possible to pass the Module along in method calls.
>>>> Along with removing EntityStoreUnitOfWork in some cases where only
>>>> something inside the EsUoW is actually wanted, which in some cases
>> resulted
>>>> in creating a EsUoW for no practical reason, I have it working.... i.e.
>>>> doesn't break any existing testcases. I haven't checked whether my case
>> was
>>>> solved as well. Will do that shortly.
>>>>
>>>>
>>>>
>>>> On Thu, Jun 25, 2015 at 5:44 AM, Kent Sølvsten <
>> kent.soelvsten@gmail.com>
>>>> wrote:
>>>>
>>>>> Hmmm interesting problem . Could it be reproduced with something like
>>>>> this:
>>>>>
>>>>> a. Creation of a UnitOfWork
>>>>> b. Accessing entity M1 in module M
>>>>> c. Entity E calls service MS in module M.
>>>>> d. Service uses entity N1 in module N (N1 has some sort of shared
>>>>> visibility)
>>>>> .... (later in the same UOW)
>>>>> e. Someone uses service NS in module N (some short of shared
>> visibility).
>>>>> f. Service NS uses entity N2 in module N (which has only module
>>>>> visibility)
>>>>>
>>>>> I am not sure that using a composite module/entitystore cache key is
>>>>> such a good idea. Wouldn't that lead to 2 instances of
>>>>> EntityStoreUnitOfWork, basically splitting uow.complete() into 2 native
>>>>> transactions in that case?
>>>>> Could we instead reuse the "ModuleUnitOfWork-idea" ?
>>>>>
>>>>> Something like:
>>>>>
>>>>>     public EntityStoreUnitOfWork getEntityStoreUnitOfWork( EntityStore
>>>>> store, Module module )
>>>>>
>>>>>     {
>>>>>
>>>>>         EntityStoreUnitOfWork uow = storeUnitOfWork.get( store );
>>>>>
>>>>>         if( uow == null )
>>>>>
>>>>>         {
>>>>>
>>>>>             uow = store.newUnitOfWork( usecase, currentTime );    //
no
>>>>> module here!
>>>>>
>>>>>             storeUnitOfWork.put( store, uow );
>>>>>
>>>>>         }
>>>>>
>>>>>         return new ModuleEntityStoreUnitOfWork(uow, module);
>>>>>
>>>>>     }
>>>>>
>>>>> As a side-note we could one day consider some sort of
>>>>> entitystore-sharing between modules. If we had 2 modules, separated for
>>>>> design reasons but sharing the same persistence, it would be nice to
be
>>>>> able to complete a UOW spanning those 2 modules in a single native
>>>>> transaction.
>>>>>
>>>>> Regarding "bug-compatible" the safe option would be to postpone to 3.0
>> -
>>>>> but i guess it is better to just fix the issue?
>>>>>
>>>>> /Kent
>>>>>
>>>>>
>>>>>
>>>>> Den 24-06-2015 kl. 17:40 skrev Niclas Hedhman:
>>>>>> Follow-up; The case when it gets wrong is more complex than my list,
>>>>> and I
>>>>>> am not able to create a testcase for this...
>>>>>>
>>>>>> On Wed, Jun 24, 2015 at 10:51 PM, Niclas Hedhman <niclas@hedhman.org>
>>>>> wrote:
>>>>>>> Gang,
>>>>>>>
>>>>>>> I have found a fairly sophisticated bug, which is rather severe
and
>>>>>>> possibly hard to fix without breaking applications, that might
depend
>>>>> on
>>>>>>> wrong behavior...
>>>>>>>
>>>>>>> It manifests itself as not having the right Visibility of entities,
>> and
>>>>>>> the reason is because the EntityStoreUnitOfWork may be set up
>>>>> incorrectly
>>>>>>> sometimes.
>>>>>>>
>>>>>>> I think it happens if the following sequence occurs;
>>>>>>>
>>>>>>>  a. Creation of a UnitOfWork
>>>>>>>  b. Accessing entity E in module M
>>>>>>>  c. Entity E calls service S in module N.
>>>>>>>  d. Service S tries to access entity F in module N.
>>>>>>>
>>>>>>> internally, the EntityStoreUnitOfWork will have been initialized
with
>>>>> the
>>>>>>> Module M, and uses that to find entity F, which is not seen
>>>>>>> (Visibility.module).
>>>>>>>
>>>>>>> This is very similar to a design flaw a long time ago, where
the
>>>>>>> visibility was effectively in the wrong Layer, and that is when
the
>>>>> "split"
>>>>>>> UnitOfWork concept was introduced (2008?).
>>>>>>>
>>>>>>> The issue manifests itself in UnitOfWorkInstance in the following
>>>>> method;
>>>>>>> public EntityStoreUnitOfWork getEntityStoreUnitOfWork( EntityStore
>>>>> store, Module module )
>>>>>>> {
>>>>>>>     EntityStoreUnitOfWork uow = storeUnitOfWork.get( store );
>>>>>>>     if( uow == null )
>>>>>>>     {
>>>>>>>         uow = store.newUnitOfWork( usecase, module, currentTime
);
>>>>>>>         storeUnitOfWork.put( store, uow );
>>>>>>>     }
>>>>>>>     return uow;
>>>>>>> }
>>>>>>>
>>>>>>> as the EntityStoreUnitOfWork will have been created with the
wrong
>>>>> module.
>>>>>>> I think the solution is that the Module must also be part of
the
>> cache
>>>>>>> key, and that this would solve it. However, I am very anxious
that
>>>>> this may
>>>>>>> break something else very badly, as it is fairly central to the
>> entire
>>>>>>> UnitOfWork flow.
>>>>>>>
>>>>>>> The reason I say that it might break applications is that, the
called
>>>>>>> Service has access to the entities in the modules calling it,
and
>>>>> someone
>>>>>>> might depend on that. But I think, we should be allowed to break
bug
>>>>>>> compatibility... Or??
>>>>>>>
>>>>>>> BTW, this touches on the "multiple entitystore" feature that
came up
>>>>>>> during the discussion of the Messaging abstraction.
>>>>>>>
>>>>>>> Cheers
>>>>>>> --
>>>>>>> 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
View raw message