Jeff - > As I said earlier, if there¹s work to be done, I¹m happy to submit a patch - but I¹d like some consensus around what the patch should do first... I talked with a few teammates yesterday and I think we're on the same page as you. I will do some digging to try to figure out why I backed this change out originally(rather than changed the testcase). I'll get you and update a bit later today.
Thanks, Rick On Thu, Feb 13, 2014 at 12:41 PM, Jeff Oh <jeff...@elasticpath.com> wrote: > >As I eluded to in the JIRA, the root problem is that in the JPA 2.0 spec, > >the concept of Cache/CacheStoreMode/CacheRetrieveMode/etc/etc was added > >and > >this change was not honoring that contract. The crux of the problem is > >that > >the default value for CacheStoreMode is USE and the spec says[1] that > >elements in the cache are not to be updated. So in your example, after > >Foo-1[b,c] gets loaded the subsequent load of Foo-1[a] will not be > >reflected in the cache. I agree that most likely isn't the expected > >behavior, but we need to satisfy the spec. > > > Hmm. So upon reading the spec, this is one possible interpretation of the > spec - but I¹m not sure that it¹s the only possible interpretation. > > The spec for CacheStoreMode.USE states: > > /** > * Insert/update entity data into cache when read > * from database and when committed into database: > * this is the default behavior. Does not force refresh > * of already cached items when reading from database. > */ > > And the spec for CacheStoreMode.REFRESH states: > > /** > * Insert/update entity data into cache when read > * from database and when committed into database. > * Forces refresh of cache for items read from database. > */ > > > So the interesting thing is that CacheStoreMode.USE does not forbid > insert/update operations - it merely states that refreshes of already > cached items should not be ³forced². > > > I think there¹s multiple ways to interpret this. > > Assume we have an entity A with a eagerly fetched many-to-one relationship > to B. If we load an instance of A from db, OpenJPA will almost certainly > also load B via a join when it issues the SQL select for A. Normally, if > B is already in the L2 cache, and CacheStoreMode.USE is in effect, then I > agree that OpenJPA should not refresh the instance of B in the L2 cache. > However, if B is already in the L2 cache, *and* B¹s data loaded from the > db is both newer and different than B¹s representation in cache, then I > think there is a grey area in the spec. Specifically, the spec does not > claim that the JPA implementation must refresh the item in cache, but it > also does not claim that the JPA implementation must not. In an ideal > world, I think if JPA becomes aware that cached data is out of date it > should remove/replace it, and I don¹t think that anything in the spec > forbids this. So ideally, in the second case, I claim that OpenJPA should > update the entity data for B in cache, even if the data is read using > CacheStoreMode.USE. > > I think that there is a similar argument to be had with the fetch group > example I used earlier. CacheStoreMode.USE states that refreshes of > already cached items should not be forced, but it does not *forbid* > inserts and updates of already cached items when reading from the > database, particularly when reading data that was not already in cache. > Reading the JSR-317 spec (pg. 105) doesn¹t shed any more light in the > issue. > > While CacheStoreMode.REFRESH may be an option to work around this, it¹s > not a one-to-one replacement. My worry is that using > CacheStoreMode.REFRESH instead of CacheStoreMode.USE is that depending on > the object model and the load, CacheStoreMode.REFRESH may prevent objects > from ever expiring from cache, because they¹re constantly being REFRESHed. > Worse, the data merge mechanics mean that being refreshed in cache > doesn¹t necessarily imply that the data in cache is being refreshed as > well. That could be a pretty significant downside for us. > > As I said earlier, if there¹s work to be done, I¹m happy to submit a patch > - but I¹d like some consensus around what the patch should do first... > > > Cheers, > > Jeff > > On 2/13/2014, 7:00 AM, "Rick Curtis" <curti...@gmail.com> wrote: > > >> Another question we have is what exactly was the "internal test > >regression" that caused the rollback? > >The company I work for (IBM) does a huge amount of functional server based > >testing internally. When changes are made, the OpenJPA unit tests are the > >first line of tests and occasionally the UTs miss a scenario that the > >internal tests catch. This is one of those scenarios. > > > >As I eluded to in the JIRA, the root problem is that in the JPA 2.0 spec, > >the concept of Cache/CacheStoreMode/CacheRetrieveMode/etc/etc was added > >and > >this change was not honoring that contract. The crux of the problem is > >that > >the default value for CacheStoreMode is USE and the spec says[1] that > >elements in the cache are not to be updated. So in your example, after > >Foo-1[b,c] gets loaded the subsequent load of Foo-1[a] will not be > >reflected in the cache. I agree that most likely isn't the expected > >behavior, but we need to satisfy the spec. > > > >It might make sense to see how we behave if you change the CacheStoreMode > >to REFRESH. Start by passing the CacheStoreMode into > >EntityManager.find(...) and from there check to see if you can pass > >javax.persistence.CacheStoreMode as a persistence.xml level property. > >Honestly I don't have many cycles to look at this problem right now, but > >I'll help where I can. > > > >Thanks, > >Rick > > > >[1] > >* > http://docs.oracle.com/javaee/6/api/javax/persistence/CacheStoreMode.html > >#USE > >< > http://docs.oracle.com/javaee/6/api/javax/persistence/CacheStoreMode.html > >#USE>* > >-- > >"Does not force refresh of already cached items when reading from > >database." > > > >On Wed, Feb 12, 2014 at 5:03 PM, Jeff Oh <jeff...@elasticpath.com> wrote: > > > >> Hello, > >> > >> We're currently migrating an application from OpenJPA 1.2.2 to OpenJPA > >> 2.3.0, and have found that DataCache efficacy in our application > >>declines > >> significantly from 1.2.2 to 2.3.0, to the point where our overall system > >> performance has declined by about 25% with the version upgrade. > >> > >> We've found that we can restore DataCache performance to 1.2.2 levels by > >> re-applying OPENJPA-2285< > >> https://issues.apache.org/jira/browse/OPENJPA-2285>, which was rolled > >> back due to an "internal test regression" issue. > >> > >> The behaviour we've found is this: > >> > >> Given: > >> > >> An entity Foo, with fields [a, b, c]. > >> Foo-1 is loaded into the datacache using a fetch group that loads fields > >> [a, b]. > >> > >> When: > >> > >> A query is executed that loads Foo-1 using fields [b,c]. This query is > >> executed twice in succession, using two different EntityManagers. > >> > >> Then: > >> > >> In OpenJPA 2.3.0, both queries result in a SQL query being run to load > >>the > >> value of field c. > >> In OpenJPA 1.2.2, only the first query results in a SQL query being run > >>to > >> load the value of field c. The second query is serviced from cache. > >> > >> Expected: > >> > >> The OpenJPA 1.2.2 behaviour seems more consistent with expected cache > >> behaviour. The cache cannot be expected to successfully fulfill the > >> request on the first invocation - but it should be able to fulfill it on > >> all subsequent invocations. > >> > >> The first question is, what should OpenJPA 2's behaviour be in this > >> circumstance? OPENJPA-2285< > >> https://issues.apache.org/jira/browse/OPENJPA-2285> merges the new > >>loaded > >> fields into the original cached entity and recaches the entity. OpenJPA > >> 1.2.2 does a similar thing, except that it rejects the merge if the new > >> entity has an earlier version than the cached entity. Note that merging > >> still occurs in OpenJPA 1.2.2 if the new entity has a later version than > >> the cached entity. > >> > >> Another question we have is what exactly was the "internal test > >> regression" that caused the rollback? > >> > >> > >> Some potential solutions: > >> > >> 1. Merge cached and loaded data as per OPENJPA-2285/OpenJPA 1.2.2. > >> This minimizes database operations required. On the other hand, the > >>merge > >> of cached and non-cached data raises the possibility of entities with > >> "mixed" data from various versions. It's probably best to reject the > >> object entirely if a version conflict is detected and reload the entire > >> object from the db (and recache) in that instance. > >> > >> 2. Load object from db using the query's fetch group and recache. > >> Instead of merging cached fields and db loaded fields, OpenJPA could > >>load > >> the entire object using the query's fetch group. This object should be > >>in > >> a consistent state, and the object in data cache could be replaced with > >>the > >> newly loaded object. The disadvantage would be if there are two > >>different > >> queries being run for the same entity, one with the fetch group [a, b] > >>and > >> another with the fetch group [b,c], then the object in cache will > >>"whipsaw" > >> back and forth between representations. This also results in more db > >> access than option #1. > >> > >> 3. Load object from db using the superset of the query's fetch group and > >> the datacache's fetch group and recache. > >> Instead of merging cached fields and db loaded fields, OpenJPA could > >>load > >> the entire object using the union of the query's fetch group and the > >>cached > >> entities' fetch group. This object should be in a consistent state, and > >> the object in data cache could be replaced with the newly loaded object. > >> The disadvantage would be it's hard to see how this could reliably > >>extend > >> to entities related to the original entity that are loaded as part of > >>the > >> query. However, it avoids the whipsaw problems in option #2. This > >>option > >> also results in the most db access, at least initially. > >> > >> 4. Remove incomplete entities from cache. > >> Merge cached and loaded data together as is done currently, but remove > >>the > >> cached entity afterwards. This isn't much of a solution, but at least a > >> sparsely loaded entity doesn't have the potential to degrade the cache > >> indefinitely... > >> > >> 5. Some ability to enable one or more of the solutions via a config > >> option, if none of these solutions are considered acceptable for "core" > >>use. > >> > >> > >> If there's agreement on what behaviour folks would like to see, I'd be > >> happy to submit a patch. > >> > >> > >> Cheers, > >> > >> Jeff > >> > >> > >> > >> > >> > >> > >> > >> [http://elasticpath.com/images/ep.gif] > >> Jeff Oh, Sr. Software Engineer > >> Phone: 604.408.8078 ext. 104 > >> Email: jeff...@elasticpath.com<mailto:jeff...@elasticpath.com> > >> > >> Elastic Path Software Inc. > >> Web elasticpath.com <http://www.elasticpath.com/> | Blog getelastic.com > >>< > >> http://www.getelastic.com/> | Twitter twitter.com/elasticpath < > >> http://www.twitter.com/elasticpath> > >> Careers elasticpath.com/jobs<http://www.elasticpath.com/jobs> | > >>Community > >> grep.elasticpath.com <http://grep.elasticpath.com/> > >> > >> Confidentiality Notice: This message is intended only for the use of the > >> designated addressee(s), and may contain information that is privileged, > >> confidential and exempt from disclosure. Any unauthorized viewing, > >> disclosure, copying, distribution or use of information contained in > >>this > >> e-mail is prohibited and may be unlawful. If you received this e-mail in > >> error, please reply to the sender immediately to inform us you are not > >>the > >> intended recipient and delete the email from your computer system. > >> > >> > >> > >> > > > > > >-- > >*Rick Curtis* > > > > > Jeff Oh, Sr. Software Engineer > Phone: 604.408.8078 ext. 104 > Email: jeff...@elasticpath.com > > Elastic Path Software Inc. > Web: www.elasticpath.com > Blog: www.getelastic.com > Community: grep.elasticpath.com > Careers: www.elasticpath.com/jobs > > Confidentiality Notice: This message is intended only for the use of the > designated addressee(s), and may contain information that is privileged, > confidential and exempt from disclosure. Any unauthorized viewing, > disclosure, copying, distribution or use of information contained in this > e-mail is prohibited and may be unlawful. If you received this e-mail in > error, please reply to the sender immediately to inform us you are not the > intended recipient and delete the email from your computer system. Thank > you. > > > -- *Rick Curtis*
