Proposal to disable QuerySQLCache by default

[Jeremy Bauer <te...@gmail.com>]

Dev's,

We've had a few regression-type issues (OPENJPA-660 & OPENJPA-731)
since the addition of QuerySQLCache in 1.2.0.  While this cache has
shown to provide significant performance improvements for certain
scenarios, I think we should consider disabling the cache by default.
The main reason is the potential for future regression issues.
Another reason is that we didn't follow the pattern set by existing
caches.  With the exception of the query compilation cache (which is
relatively static after queries are initialized), the other caches are
disabled by default.  While the cache is supposed to be transparent,
it could (and has shown to) affect the behavior of an application that
has been working for quite some time on a prior release.

Arguably, QuerySQLCache may just need more rigorous testing.  While
additional testing may help, testing every scenario is difficult and
if we miss one, regression issues due to performance enhancements are
pretty hard to swallow.  I think a better approach is to allow users
to enable the cache if they choose to do so.

What do y'all think?  If we get a consensus, I'll open an issue to
make the necessary changes to disable the cache.

-Jeremy

Re: Proposal to disable QuerySQLCache by default

[Pinaki Poddar <pp...@apache.org>]

+1

In earlier occasions, I have expressed my reservations about QuerySQLCache
in less public forum than this one. What worries me about the reported
regressions is the are very non-local -- for example, one will find it hard
to explain why a certain type of inheritance modeling gets impacted when SQL
cache is active!

While the idea of gaining performance by caching SQL is noble, the design of
this facility at MappingStrategy level does not seem right to me at all. Not
only this approach bifurcates the critical code paths (which can be a
challenge for maintenance and extension in future), the design will find it
difficult to extend this caching behavior for general purpose query. 


-- 
View this message in context: http://n2.nabble.com/Proposal-to-disable-QuerySQLCache-by-default-tp1330161p1331817.html
Sent from the OpenJPA Developers mailing list archive at Nabble.com.

Re: Proposal to disable QuerySQLCache by default

[Fay Wang <fy...@yahoo.com>]

1+

This QuerySQLCache is to cache SelectImpl and SQLBuffer for the findBy operation without making too much architectural change in the current findBy code path. Although it works fine in the "normal" situations with significant performance gain, it is undoubtedly not tested in all scenarios. A better approach is to have a cleaner architecture sitting on top of the current framework to cover the SelectImpl/SQLBuffer cache for both findBy operation and dynamic JPQL query. 


--- On Mon, 10/13/08, Jeremy Bauer <te...@gmail.com> wrote:

> From: Jeremy Bauer <te...@gmail.com>
> Subject: Proposal to disable QuerySQLCache by default
> To: dev@openjpa.apache.org
> Date: Monday, October 13, 2008, 12:58 PM
> Dev's,
> 
> We've had a few regression-type issues (OPENJPA-660
> & OPENJPA-731)
> since the addition of QuerySQLCache in 1.2.0.  While this
> cache has
> shown to provide significant performance improvements for
> certain
> scenarios, I think we should consider disabling the cache
> by default.
> The main reason is the potential for future regression
> issues.
> Another reason is that we didn't follow the pattern set
> by existing
> caches.  With the exception of the query compilation cache
> (which is
> relatively static after queries are initialized), the other
> caches are
> disabled by default.  While the cache is supposed to be
> transparent,
> it could (and has shown to) affect the behavior of an
> application that
> has been working for quite some time on a prior release.
> 
> Arguably, QuerySQLCache may just need more rigorous
> testing.  While
> additional testing may help, testing every scenario is
> difficult and
> if we miss one, regression issues due to performance
> enhancements are
> pretty hard to swallow.  I think a better approach is to
> allow users
> to enable the cache if they choose to do so.
> 
> What do y'all think?  If we get a consensus, I'll
> open an issue to
> make the necessary changes to disable the cache.
> 
> -Jeremy