Author: pcl
Date: Mon Oct 30 15:23:11 2006
New Revision: 469313
URL: http://svn.apache.org/viewvc?view=rev&rev=469313
Log:
Fixed OPENJPA-70. Data caching + external modifications could result in stale
data getting stuck in cache.
Added:
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheOptimisticLockRecovery.java
Modified:
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java
Modified:
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java
URL:
http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java?view=diff&rev=469313&r1=469312&r2=469313
==============================================================================
---
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java
(original)
+++
incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/DataCacheStoreManager.java
Mon Oct 30 15:23:11 2006
@@ -38,6 +38,7 @@
import org.apache.openjpa.kernel.StoreQuery;
import org.apache.openjpa.meta.ClassMetaData;
import org.apache.openjpa.meta.MetaDataRepository;
+import org.apache.openjpa.util.OptimisticException;
/**
* StoreManager proxy that delegates to a data cache when possible.
@@ -503,8 +504,16 @@
public Collection flush(Collection states) {
Collection exceps = super.flush(states);
- if (!exceps.isEmpty() || _ctx.isLargeTransaction())
+ if (exceps.isEmpty() && _ctx.isLargeTransaction())
return exceps;
+ else if (!exceps.isEmpty()) {
+ for (Iterator iter = exceps.iterator(); iter.hasNext(); ) {
+ Exception e = (Exception) iter.next();
+ if (e instanceof OptimisticException)
+ evictOptimisticLockFailure((OptimisticException) e);
+ }
+ return exceps;
+ }
OpenJPAStateManager sm;
for (Iterator itr = states.iterator(); itr.hasNext();) {
@@ -533,6 +542,70 @@
}
}
return Collections.EMPTY_LIST;
+ }
+
+ /**
+ * Evict from the cache the OID (if available) that resulted in an
+ * optimistic lock exception iff the
+ * version information in the cache matches the version
+ * information in the state manager for the failed
+ * instance. This means that we will evict data from the
+ * cache for records that should have successfully
+ * committed according to the data cache but did not. The
+ * only predictable reason that could cause this behavior
+ * is a concurrent out-of-band modification to the
+ * database that was not communicated to the cache. This
+ * logic makes OpenJPA's data cache somewhat tolerant of
+ * such behavior, in that the cache will be cleaned up as
+ * failures occur.
+ */
+ private void evictOptimisticLockFailure(OptimisticException e) {
+ Object o = ((OptimisticException) e).getFailedObject();
+ OpenJPAStateManager sm = _ctx.getStateManager(o);
+ ClassMetaData meta = sm.getMetaData();
+
+ // this logic could be more efficient -- we could aggregate
+ // all the cache->oid changes, and then use
+ // DataCache.removeAll() and less write locks to do the
+ // mutation.
+ DataCache cache = meta.getDataCache();
+ cache.writeLock();
+ try {
+ DataCachePCData data = cache.get(sm.getId());
+ boolean remove;
+ switch (compareVersion(sm, sm.getVersion(), data.getVersion())) {
+ case StoreManager.VERSION_LATER:
+ case StoreManager.VERSION_SAME:
+ // This tx's current version is later than the data cache
+ // version. In this case, the commit should have
succeeded.
+ // Remove the instance from cache in the hopes that the
+ // cache is out of sync.
+ remove = true;
+ break;
+ case StoreManager.VERSION_EARLIER:
+ // This tx's current version is earlier than the data
+ // cache version. This is a normal optimistic lock
failure.
+ // Do not clean up the cache; it probably already has the
+ // right values, and if not, it'll get cleaned up by a tx
+ // that fails in one of the other case statements.
+ remove = false;
+ break;
+ case StoreManager.VERSION_DIFFERENT:
+ // The version strategy for the failed object does not
+ // store enough information to optimize for expected
+ // failures. Clean up the cache.
+ remove = true;
+ break;
+ default:
+ // Unexpected return value. Remove to be future-proof.
+ remove = true;
+ break;
+ }
+ if (remove)
+ cache.remove(sm.getId());
+ } finally {
+ cache.writeUnlock();
+ }
}
public StoreQuery newQuery(String language) {
Added:
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java
URL:
http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java?view=auto&rev=469313
==============================================================================
---
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java
(added)
+++
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java
Mon Oct 30 15:23:11 2006
@@ -0,0 +1,39 @@
+package org.apache.openjpa.persistence.datacache;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Version;
+import javax.persistence.GeneratedValue;
+
+import javax.persistence.Table;
+
[EMAIL PROTECTED]
[EMAIL PROTECTED](name="OPTIMISTIC_LOCK_INSTANCE")
+public class OptimisticLockInstance {
+ @Id @GeneratedValue
+ private int pk;
+
+ @Version
+ private int oplock;
+
+ private String str;
+
+ protected OptimisticLockInstance() { }
+
+ public OptimisticLockInstance(String str) {
+ this.str = str;
+ }
+
+ public int getPK() {
+ return pk;
+ }
+
+ public int getOpLock() {
+ return oplock;
+ }
+
+ public String getStr() {
+ return str;
+ }
+}
+
Added:
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheOptimisticLockRecovery.java
URL:
http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheOptimisticLockRecovery.java?view=auto&rev=469313
==============================================================================
---
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheOptimisticLockRecovery.java
(added)
+++
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestDataCacheOptimisticLockRecovery.java
Mon Oct 30 15:23:11 2006
@@ -0,0 +1,132 @@
+package org.apache.openjpa.persistence.datacache;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.Persistence;
+import javax.persistence.RollbackException;
+import javax.persistence.LockModeType;
+
+import junit.framework.TestCase;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.openjpa.persistence.OpenJPAPersistence;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import javax.sql.DataSource;
+
+public class TestDataCacheOptimisticLockRecovery
+ extends TestCase {
+
+ private EntityManagerFactory emf;
+
+ public void setUp() {
+ Map options = new HashMap();
+
+ // turn on caching
+ options.put("openjpa.DataCache", "true");
+ options.put("openjpa.RemoteCommitProvider", "sjvm");
+
+ // ensure that OpenJPA knows about our type, so that
+ // auto-schema-creation works
+ options.put("openjpa.MetaDataFactory",
+ "jpa(Types=" + OptimisticLockInstance.class.getName() + ")");
+
+ emf = Persistence.createEntityManagerFactory("test", options);
+
+ EntityManager em = emf.createEntityManager();
+ em.getTransaction().begin();
+ em.createQuery("delete from OptimisticLockInstance");
+ em.getTransaction().commit();
+ em.close();
+ }
+
+ public void tearDown() {
+ emf.close();
+ }
+
+ public void testOptimisticLockRecovery()
+ throws SQLException {
+
+ EntityManager em;
+
+ // 1. get the instance into the cache via this insert
+ em = emf.createEntityManager();
+ em.getTransaction().begin();
+ OptimisticLockInstance oli = new OptimisticLockInstance("foo");
+ try {
+ em.persist(oli);
+ em.getTransaction().commit();
+ } finally {
+ if (em.getTransaction().isActive())
+ em.getTransaction().rollback();
+ }
+ int pk = oli.getPK();
+ em.close();
+
+ // 2. get the oplock value for the instance after commit and
+ // get a read lock to ensure that we check for the optimistic
+ // lock column at tx commit.
+ em = emf.createEntityManager();
+ em.getTransaction().begin();
+ oli = em.find(OptimisticLockInstance.class, pk);
+ int firstOpLockValue = oli.getOpLock();
+ em.lock(oli, LockModeType.READ);
+
+ // 2. make a change to the instance's optimistic lock column
+ // via direct SQL in a separate transaction
+ int secondOpLockValue = firstOpLockValue + 1;
+
+ DataSource ds = (DataSource) OpenJPAPersistence.cast(em)
+ .getEntityManagerFactory().getConfiguration()
+ .getConnectionFactory();
+ Connection c = ds.getConnection();
+ c.setAutoCommit(false);
+ PreparedStatement ps = c.prepareStatement(
+ "UPDATE OPTIMISTIC_LOCK_INSTANCE SET OPLOCK = ? WHERE PK = ?");
+ ps.setInt(1, secondOpLockValue);
+ ps.setInt(2, pk);
+ assertEquals(1, ps.executeUpdate());
+ c.commit();
+
+ // 3. commit the transaction, catching the expected oplock
+ // exception
+ try {
+ em.getTransaction().commit();
+ fail("tx should have failed due to out-of-band oplock change");
+ } catch (RollbackException re) {
+ // expected
+ } finally {
+ if (em.getTransaction().isActive())
+ em.getTransaction().rollback();
+ }
+
+ // 4. obtain the object in a new persistence context and
+ // assert that the oplock column is set to the one that
+ // happened in the out-of-band transaction
+ em.close();
+ em = emf.createEntityManager();
+ oli = em.find(OptimisticLockInstance.class, pk);
+
+ // If this fails, then the data cache has the wrong value.
+ // This is what this test case is designed to exercise.
+ assertEquals("data cache is not being cleared when oplock "
+ + "violations occur", secondOpLockValue, oli.getOpLock());
+
+ // 5. get a read lock on the instance and commit the tx; this
+ // time it should go through
+ em.getTransaction().begin();
+ em.lock(oli, LockModeType.READ);
+ try {
+ em.getTransaction().commit();
+ } catch (RollbackException e) {
+ e.printStackTrace();
+ throw e;
+ } finally {
+ if (em.getTransaction().isActive())
+ em.getTransaction().rollback();
+ }
+ em.close();
+ }
+}