Thanks for your work and feedback Adrian! I already did cursorily reviewed your work, if I get some chances to review more in details, which revisions will you recommend?
Jacques From: "Adrian Crum" <adrian.c...@sandglass-software.com> > No, I didn't get to that yet. I found problems in the entity models, so > I'm currently detoured to fix those, then I will get back to the > GenericEntity and caching issues. > > -Adrian > > On 5/4/2013 8:25 AM, Jacques Le Roux wrote: >> Hi Adrian, >> >> Did you also fix the DCC since? >> >> Jacques >> >> From: "Adrian Crum" <adrian.c...@sandglass-software.com> >>> I fixed the entity cache issues in revision 1476296. The fix does not >>> include the distributed cache system, but that should be easy to fix by >>> duplicating the fixes to the local cache. >>> >>> I found some flaws in the entity engine that I will discuss in another >>> thread. >>> >>> -Adrian >>> >>> On 4/22/2013 9:45 AM, Adrian Crum wrote: >>>> Thanks Jacopo. I haven't looked into the entity cache implementation >>>> thoroughly, but I was under the impression that it can be configured >>>> to be distributed. >>>> >>>> I have the fix working on my local copy. As you can see, I have made >>>> some related changes already and will be making some more, but I won't >>>> commit the fix until next weekend - to give everyone a chance to respond. >>>> >>>> -Adrian >>>> >>>> On 4/22/2013 9:33 AM, Jacopo Cappellato wrote: >>>>> It seems actually to be an issue rather than a feature (I can't think >>>>> of a use case where this behavior would be useful); I have created a >>>>> few test cases (similar to the one you have provided in the other >>>>> thread) that further analyze your discovery but they don't add much >>>>> to what you found (apart from confirming the risk of getting stale >>>>> data). >>>>> However, when we start design/implement a refactoring of this part of >>>>> the system, I would suggest that we also consider how to deal with >>>>> similar scenarios in a clustered deployment (in fact many of the >>>>> production deployment are based on clusters); the simplest use case >>>>> could be: in a cluster, we have two OFBiz instances connected to the >>>>> same database; in one instance the list is cached, in the other >>>>> instance one of the generic values (that are part of the selection) >>>>> is updated. A distributed cache system may help here. >>>>> >>>>> Jacopo >>>>> >>>>> On Apr 21, 2013, at 10:54 AM, Adrian Crum >>>>> <adrian.c...@sandglass-software.com> wrote: >>>>> >>>>>> Last week I discovered a flaw in the EntityListCache implementation: >>>>>> http://mail-archives.apache.org/mod_mbox/incubator-ofbiz-dev/201304.mbox/%3c516ac7b4.2020...@sandglass-software.com%3E >>>>>> >>>>>> To summarize: Entity conditions that are cached become stale when >>>>>> any member of the cached list is changed - making the cache contents >>>>>> invalid. In addition, GenericValues in the cached list are mutable - >>>>>> which is inconsistent with the primary key cache, where >>>>>> GenericValues from the cache are immutable. >>>>>> >>>>>> I would like to fix this, but I think we should discuss it first. >>>>>> >>>>>> One change would be to make the list member GenericValues immutable. >>>>>> This will make the GenericValues retrieved from the entity list >>>>>> cache consistent with the GenericValues retrieved from the primary >>>>>> key cache, but it won't prevent an invalid cache (because the list >>>>>> member GenericValue can be cloned and modified). Also, this change >>>>>> will likely break a lot of code, because it is common to retrieve a >>>>>> list of GenericValues from the cache and then make changes to the >>>>>> list members. We could create a "transitional" GenericValue that >>>>>> would warn developers when they modify a cached list member, and >>>>>> then switch to an immutable GenericValue some time in the future. >>>>>> >>>>>> To fix the stale cache problem, the cache instance can be made a >>>>>> GenericValue listener in all of its list members - so any time a >>>>>> list member is modified the cache will be cleared. This will keep >>>>>> the cache valid, but there might be a performance hit. I'm open to >>>>>> other solutions to this problem. >>>>>> >>>>>> Any thoughts? >>>>>> >>>>>> -Adrian >>>>>> >>>>>> >
