Re: too much problems with new functions was: Re: ofbiz twitter account was: Re: svn commit: r918926 - in /ofbiz/site: images/follow_us-b.png index.html

Posted by Adam Heath-2 on Mar 11, 2010; 7:14am
URL: http://ofbiz.116.s1.nabble.com/Re-svn-commit-r918926-in-ofbiz-site-images-follow-us-b-png-index-html-tp1578166p1588517.html

David E Jones wrote:
> [snip]

You know, I was going to start a thread like this, but you beat me to
it.  I actually have a concrete example of a major problem, so I'm
going to chime in.

As many are aware, I have been going thru adding many test cases to
lots of low-level utility code.  *Every* time I have done so, I have
*always* found a problem.  Without fail.

It's not that A with B with C with D fails.  It's that A and B and C
and D have issues all by themselves.  I consider this extremely
unacceptable.

The latest problem I have is quite the complex.  It's the
DelegatorFactory change.  It's very broken.  The original code only
existed in GenericDelegator.getGenericDelegator().  It would check a
local map, looking for an already constructed delegator, returning
*very* fast if one existed.

However, the new code inverts this.  Instead, DelegatorFactory calls
directly into UtilObject.getObjectFromFactory.  No caching at all.
The caching is deferred to DelegatorFactoryImpl.  So far, things seem
perfectly fine.

However, there are parts of ofbiz that don't store delegator
instances.  Instead, they store only the delegatorName.
org.ofbiz.entity.cache.AbstractCache is one of these.  It calls
DelegatorFactory.getDelegator every time it needs the actual delegator
instance.

So, with this long explanation, I still haven't shown any problem.
But just wait.

ServiceLoader, which is used by getObjectFromFactory, will walk the
*entire* classpath, each and every time it is called.

Again, still no exact problem.  But those who have been following may
be starting to see the trees thru this forest.

I had an importer written for a client, to convert a legacy database
to ofbiz schema.  On our internal ofbiz package based on 811524, this
importer ran in 15 minutes.  However, when I upgrade to 902021, I
didn't do a full db drop, and reimport.  When I did so today, I had to
kill the importer after running for 2 hours, with no end in site.

This was due to the entity cache for ProductAssoc getting large, and
creation of new ProductAssoc, which was causing the AbstractCache to
try and load delegator instances, which was then going thru the entire
classpath.  This completely fell over.

This slowdown is completely unacceptable.

However, even once identified, the problem is not straight forward.
Caching at a higher level, in DelegatorFactory.getDelegator, is not
correct either.

Moving the caching higher implies that the constructed delegator won't
be saved until it is completely done being constructed.  However,
during construction, the delegator creates several helper entities.
These include EntityCrypto, and the cache support classes.  These
classes also still have the same problem of not storing delegator
instances, instead just storing a name.  So, they try to load a
delegator with the correct name, and don't find it, because the
original delegator is not done being constructed.

But, we still aren't done with this particular problem.

When GenericPK and GenericValue get created, they try to set their
fields.  Setting any field on a GenericEntity requires that it knows
it's delegator.  However, creation doesn't actually set the delegator.
 Even the delegator name is null.  So any GenericEntity created during
delegator instantiation ends up trying to load a delegator named
"default", *not* with whatever delegator is actually being created.

There are a whole series of changes done over the years that have
caused these cascading problems.  And this problem is just one that I
have personally seen that has caused me to worry greatly.

What I am about to propose may sound rather severe, but I would like
to see a moratorium on any new features.  Test cases and bug fixes
only.  Ofbiz has gotten *WAY* *TOO* BUGGY*.

ps: I'm well on the way to fixing this huge delegator problem, and
should have it ready tomorrow.