ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Crum <adrian.c...@sandglass-software.com>
Subject Re: view cache
Date Sun, 13 Jul 2014 11:23:39 GMT
I had some time to look at the source code.

The comparison code that doesn't work is located in 
org.ofbiz.entity.cache.AbstractEntityConditionCache, line 184. I created 
some entity cache tests in rev 1476296 that prove it doesn't work.

Even if the comparison logic was fixed, it seems rather pointless from 
my perspective. By the time you reach the end of all those comparisons, 
there is a good chance that the value of shouldRemove is invalid - 
because other processes have changed the things being compared. It would 
be nice to have a multi-threaded test to confirm my theory, but for now 
I erred on the side of data integrity and bypassed the complicated 
condition checking.

Adrian Crum
Sandglass Software

On 7/8/2014 2:49 PM, Adrian Crum wrote:
> I apologize for the ambiguous explanation. I was trying to comment from
> what I recall working on, but I wasn't looking at the code. Even now,
> I'm a bit too busy to dig into it, so my description will be somewhat
> generalized.
> The expression cache was not working because it was doing complicated
> comparisons. The comparisons failed even if the instance being sought
> was in the cache. In other words, stale data was guaranteed in the
> expression cache. There were flaws in the comparison code, and I didn't
> look into them further because...
> If you have a complicated condition, and you are navigating a large list
> of entities, by the time you reach the end of the list, there is a good
> chance the conditions at the beginning of the list changed - due to
> multiple processes changing the underlying data source and the cache.
> So, I changed the code to eliminate the condition checking and instead
> just clear the cache if the cache key matched. It's a brute-force
> approach, but it eliminates the stale cache problem. I updated the
> entity tests to test the situation I'm trying to describe here.
> In addition, there was a flaw in the view entity cache and I fixed that
> too.
> I'm sorry I can't be more help. I'm starting up a new project and I
> won't have time to dig into it deeper until later.
> -Adrian
> On 7/4/2014 4:02 AM, Adam Heath wrote:
>> Let's continue on the list, as view cache clearing isn't related to
>> the bug.
>> What do you mean, "entities change DURING evaluation."?  Could you
>> please expand on that?
>> I have found some bugs in the cache clearing.  There were multiple
>> bugs, I need to create separate commits for each bug fix that I have.
>> * If there has been no directly looking on a entity by a PK, then when
>> the entity was updated/stored, and remove() called, it would not
>> cascade to the views that mention it.
>> * If a view is a member of another view, then none of the 3 caches
>> would cascade.  Basically, only one level of view membership was
>> considered.
>> I still need to produce more tests, for <entity-condition> in all the
>> valid locations.
>> On 07/02/2014 05:15 PM, Adrian Crum (JIRA) wrote:
>>>      [
>>> https://issues.apache.org/jira/browse/OFBIZ-4053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14050792#comment-14050792
>>> ]
>>> Adrian Crum commented on OFBIZ-4053:
>>> ------------------------------------
>>> Exactly. Checking ALL conditions on EVERY view member was unreliable
>>> - because the member entities change DURING evaluation. So, I chose
>>> to skip condition checking and just clear all member entities. The
>>> entity cache tests prove it works.
>>>> Implement an Entity Query Builder
>>>> ---------------------------------
>>>>                  Key: OFBIZ-4053
>>>>                  URL: https://issues.apache.org/jira/browse/OFBIZ-4053
>>>>              Project: OFBiz
>>>>           Issue Type: New Feature
>>>>           Components: framework
>>>>     Affects Versions: SVN trunk
>>>>             Reporter: Scott Gray
>>>>             Assignee: Scott Gray
>>>>          Attachments: builder.patch
>>>> As discussed on the dev list here:
>>>> http://ofbiz.markmail.org/thread/l6kiywqzfj656dhc
>>>> Attached is an initial implementation of builder classes (of sorts)
>>>> that make use of method chaining in order to simplify use of the
>>>> Delegator interface to query entities.
>>>> Rather than taking all possible query parameters into a single
>>>> method as the delegator does, this implementation instead builds a
>>>> query through a succession of distinct method calls.
>>>> A simple example:
>>>> {code}
>>>> // Using the Delegator interface directly
>>>> eli = delegator.find("FinAccountTrans", condition, null, null,
>>>> UtilMisc.toList("-transactionDate"), null);
>>>> // Using the new implementation
>>>> eli =
>>>> EntityBuilderUtil.list(delegator).from("FinAccountTrans").where(condition).orderBy("-transactionDate").iterator();
>>>> {code}
>>>> A more complex example:
>>>> {code}
>>>> // Delegator
>>>> EntityCondition queryConditionsList =
>>>> EntityCondition.makeCondition(allConditions, EntityOperator.AND);
>>>> EntityFindOptions options = new EntityFindOptions(true,
>>>> EntityFindOptions.CONCUR_READ_ONLY, true);
>>>> options.setMaxRows(viewSize * (viewIndex + 1));
>>>> EntityListIterator iterator = delegator.find("OrderHeader",
>>>> queryConditionsList, null, null, UtilMisc.toList("orderDate DESC"),
>>>> options);
>>>> // becomes
>>>> EntityListIterator iterator =
>>>> EntityBuilderUtil.list(delegator).distinct()
>>>> .from("OrderHeader")
>>>> .where(allConditions)
>>>> .orderBy("orderDate DESC")
>>>> .maxRows(viewSize * (viewIndex + 1))
>>>> .cursorScrollInsensitive()
>>>> .iterator();
>>>> {code}
>>>> A couple of issues with the implementation so far that I'm not
>>>> entirely happy with:
>>>> - I'm not so sure that I like EntityBuilderUtil.list(delegator) in
>>>> place of something like EntityList.use(delegator).  The latter
>>>> requires less typing and I think is more intuitive than typing
>>>> EntityBuilderUtil because of its similarities to the corresponding
>>>> minilang method calls, I actually kept having trouble remembering
>>>> what it was called when converting some delegator calls over.  It
>>>> also requires a little more typing (16-17 characters vs. 12-13), not
>>>> a big deal but every little bit helps with what would be a very
>>>> commonly used class.
>>>> - I'm struggling to see the point of having the GenericQueryBuilder
>>>> interface, the two classes share very little in the way of API and
>>>> its use seems a little superfluous to me.
>>>> Opinions on the above points or anything else to do with the
>>>> implementation are most welcome.  I'd like to get the API locked
>>>> down (and hopefully some javadocs in place) before committing.
>>> --
>>> This message was sent by Atlassian JIRA
>>> (v6.2#6252)

View raw message