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 Mon, 29 Jun 2015 21:37:30 GMT
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