Jeff - I'll recommit the changes in OPENJPA-2285 back to trunk on Monday. Will those changes be sufficient for your scenarios?
Thanks, Rick On Fri, Feb 14, 2014 at 9:15 AM, Rick Curtis <curti...@gmail.com> wrote: > 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* > -- *Rick Curtis*
