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
www.sandglass-software.com

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.TYPE_SCROLL_INSENSITIVE,
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)


Reply via email to