From c7fa16abe9061956858672b102f363a0a592a4e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Galder=20Zamarren=CC=83o?= Date: Thu, 17 Oct 2013 15:12:45 +0200 Subject: [PATCH] HHH-8623 Cache should be up to date after region eviction * Region clear now happens within the transaction of the caller, if any. Otherwise, a new transaction is started in order to do the clear within a transaction and so deal with situations where cache statistics are queried outside of a transaction. * Cache updates after the region eviction should be allowed to happen, so if region eviction happened within the transaction, a putFromLoad() is mapped to a normal put instead of a PFER call, so that the data is accessible for the current transaction. This is not an issue for situations where region has not evicted because the session cache will have data that's been accessed in the transaction. * Transaction manager could be null, if region non-transactional --- .../access/TransactionalAccessDelegate.java | 24 +++---- .../cache/infinispan/impl/BaseRegion.java | 69 +++++++++++++------ .../AbstractFunctionalTestCase.java | 44 ++++++++++++ .../BasicTransactionalTestCase.java | 64 +++++++++++++++-- 4 files changed, 162 insertions(+), 39 deletions(-) diff --git a/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/access/TransactionalAccessDelegate.java b/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/access/TransactionalAccessDelegate.java index dfb7c24a1c..962d685dc5 100755 --- a/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/access/TransactionalAccessDelegate.java +++ b/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/access/TransactionalAccessDelegate.java @@ -23,8 +23,6 @@ */ package org.hibernate.cache.infinispan.access; -import javax.transaction.Transaction; - import org.infinispan.AdvancedCache; import org.infinispan.util.logging.Log; import org.infinispan.util.logging.LogFactory; @@ -138,7 +136,14 @@ public class TransactionalAccessDelegate { } try { - writeCache.putForExternalRead( key, value ); + // Conditional put/putForExternalRead. If the region has been + // evicted in the current transaction, do a put instead of a + // putForExternalRead to make it stick, otherwise the current + // transaction won't see it. + if ( region.isRegionInvalidatedInCurrentTx() ) + writeCache.put( key, value ); + else + writeCache.putForExternalRead( key, value ); } finally { putValidator.releasePutFromLoadLock( key ); @@ -244,15 +249,10 @@ public class TransactionalAccessDelegate { if ( !putValidator.invalidateRegion() ) { throw new CacheException( "Failed to invalidate pending putFromLoad calls for region " + region.getName() ); } - final Transaction tx = region.suspend(); - try { - // Invalidate the local region and then go remote - region.invalidateRegion(); - Caches.broadcastEvictAll( cache ); - } - finally { - region.resume( tx ); - } + + // Invalidate the local region and then go remote + region.invalidateRegion(); + Caches.broadcastEvictAll( cache ); } } diff --git a/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java b/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java index 9b57694ee2..b981fb4546 100644 --- a/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java +++ b/hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java @@ -53,6 +53,7 @@ import org.hibernate.cache.spi.RegionFactory; public abstract class BaseRegion implements Region { private static final Log log = LogFactory.getLog( BaseRegion.class ); + private Transaction currentTransaction; private enum InvalidateState { INVALID, CLEARING, VALID @@ -61,9 +62,12 @@ public abstract class BaseRegion implements Region { private final String name; private final AdvancedCache regionClearCache; private final TransactionManager tm; + private final Object invalidationMutex = new Object(); private final AtomicReference invalidateState = new AtomicReference( InvalidateState.VALID ); + private volatile Transaction invalidateTransaction; + private final RegionFactory factory; protected final AdvancedCache cache; @@ -164,26 +168,29 @@ public abstract class BaseRegion implements Region { boolean valid = isValid(); if ( !valid ) { synchronized (invalidationMutex) { - if ( invalidateState.compareAndSet( - InvalidateState.INVALID, InvalidateState.CLEARING - ) ) { - final Transaction tx = suspend(); + if ( invalidateState.compareAndSet( InvalidateState.INVALID, InvalidateState.CLEARING ) ) { try { - // Clear region in a separate transaction - Caches.withinTx( - cache, new Callable() { - @Override - public Void call() throws Exception { - regionClearCache.clear(); - return null; - } + // Even if no transactions are running, a new transaction + // needs to be started to do clear the region + // (without forcing autoCommit cache configuration). + Transaction tx = getCurrentTransaction(); + if ( tx != null ) { + regionClearCache.clear(); + } else { + Caches.withinTx( cache, new Callable() { + @Override + public Void call() throws Exception { + regionClearCache.clear(); + return null; + } + } ); } - ); + invalidateState.compareAndSet( InvalidateState.CLEARING, InvalidateState.VALID ); } - catch (Exception e) { + catch ( Exception e ) { if ( log.isTraceEnabled() ) { log.trace( "Could not invalidate region: " @@ -191,9 +198,6 @@ public abstract class BaseRegion implements Region { ); } } - finally { - resume( tx ); - } } } valid = isValid(); @@ -244,11 +248,20 @@ public abstract class BaseRegion implements Region { /** * Invalidates the region. */ - public void invalidateRegion() { - if ( log.isTraceEnabled() ) { - log.trace( "Invalidate region: " + name ); + public void invalidateRegion() { + if (log.isTraceEnabled()) { + log.trace("Invalidate region: " + name); + } + + Transaction tx = getCurrentTransaction(); + if ( tx != null ) { + synchronized ( invalidationMutex ) { + invalidateTransaction = tx; + invalidateState.set( InvalidateState.INVALID ); + } + } else { + invalidateState.set( InvalidateState.INVALID ); } - invalidateState.set( InvalidateState.INVALID ); } public TransactionManager getTransactionManager() { @@ -265,4 +278,18 @@ public abstract class BaseRegion implements Region { return cache; } + public boolean isRegionInvalidatedInCurrentTx() { + Transaction tx = getCurrentTransaction(); + return tx != null && tx.equals(invalidateTransaction); + } + + private Transaction getCurrentTransaction() { + try { + // Transaction manager could be null + return tm != null ? tm.getTransaction() : null; + } catch (SystemException e) { + throw new CacheException("Unable to get current transaction", e); + } + } + } diff --git a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/AbstractFunctionalTestCase.java b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/AbstractFunctionalTestCase.java index 3aa316b5fc..c1772e124b 100644 --- a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/AbstractFunctionalTestCase.java +++ b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/AbstractFunctionalTestCase.java @@ -72,4 +72,48 @@ public abstract class AbstractFunctionalTestCase extends SingleNodeTestCase { }); } + @Test + public void testInsertClearCacheDeleteEntity() throws Exception { + final Statistics stats = sessionFactory().getStatistics(); + stats.clear(); + + final Item item = new Item( "chris", "Chris's Item" ); + withTx(tm, new Callable() { + @Override + public Void call() throws Exception { + Session s = openSession(); + s.getTransaction().begin(); + s.persist(item); + s.getTransaction().commit(); + assertEquals(0, stats.getSecondLevelCacheMissCount()); + assertEquals(0, stats.getSecondLevelCacheHitCount()); + assertEquals(1, stats.getSecondLevelCachePutCount()); + s.close(); + return null; + } + }); + + log.info("Entry persisted, let's load and delete it."); + + cleanupCache(); + + withTx(tm, new Callable() { + @Override + public Void call() throws Exception { + Session s = openSession(); + s.getTransaction().begin(); + Item found = (Item) s.load(Item.class, item.getId()); + log.info(stats.toString()); + assertEquals(item.getDescription(), found.getDescription()); + assertEquals(1, stats.getSecondLevelCacheMissCount()); + assertEquals(0, stats.getSecondLevelCacheHitCount()); + assertEquals(2, stats.getSecondLevelCachePutCount()); + s.delete(found); + s.getTransaction().commit(); + s.close(); + return null; + } + }); + } + } diff --git a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java index a0e2ea1db1..f38e935696 100644 --- a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java +++ b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java @@ -24,11 +24,13 @@ package org.hibernate.test.cache.infinispan.functional; import java.io.Serializable; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; +import org.hibernate.Cache; import org.hibernate.Criteria; import org.hibernate.NaturalIdLoadAccess; import org.hibernate.cache.infinispan.access.PutFromLoadValidator; @@ -43,11 +45,10 @@ import org.hibernate.cfg.Configuration; import org.hibernate.stat.SecondLevelCacheStatistics; import org.hibernate.stat.Statistics; +import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static org.infinispan.test.TestingUtil.withTx; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; /** * Functional entity transactional tests. @@ -418,6 +419,9 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase { public void testNaturalIdCached() throws Exception { saveSomeCitizens(); + // Clear the cache before the transaction begins + BasicTransactionalTestCase.this.cleanupCache(); + withTx(tm, new Callable() { @Override public Void call() throws Exception { @@ -428,8 +432,6 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase { criteria.add( Restrictions.naturalId().set( "ssn", "1234" ).set( "state", france ) ); criteria.setCacheable( true ); - BasicTransactionalTestCase.this.cleanupCache(); - Statistics stats = sessionFactory().getStatistics(); stats.setStatisticsEnabled( true ); stats.clear(); @@ -566,7 +568,52 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase { } - private void saveSomeCitizens() throws Exception { + @Test + public void testEntityCacheContentsAfterEvictAll() throws Exception { + final List citizens = saveSomeCitizens(); + + withTx(tm, new Callable() { + @Override + public Void call() throws Exception { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + Cache cache = s.getSessionFactory().getCache(); + + Statistics stats = sessionFactory().getStatistics(); + SecondLevelCacheStatistics slcStats = stats.getSecondLevelCacheStatistics(Citizen.class.getName()); + + assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(0).getId(), + cache.containsEntity(Citizen.class, citizens.get(0).getId())); + assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(1).getId(), + cache.containsEntity(Citizen.class, citizens.get(1).getId())); + assertEquals(2, slcStats.getPutCount()); + + cache.evictEntityRegions(); + + assertEquals(0, slcStats.getElementCountInMemory()); + assertFalse("2lc entity cache is expected to not contain Citizen id = " + citizens.get(0).getId(), + cache.containsEntity(Citizen.class, citizens.get(0).getId())); + assertFalse("2lc entity cache is expected to not contain Citizen id = " + citizens.get(1).getId(), + cache.containsEntity(Citizen.class, citizens.get(1).getId())); + + Citizen citizen = (Citizen) s.load(Citizen.class, citizens.get(0).getId()); + assertNotNull(citizen); + assertNotNull(citizen.getFirstname()); // proxy gets resolved + assertEquals(1, slcStats.getMissCount()); + assertEquals(3, slcStats.getPutCount()); + assertEquals(1, slcStats.getElementCountInMemory()); + assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(0).getId(), + cache.containsEntity(Citizen.class, citizens.get(0).getId())); + + // cleanup + tx.rollback(); + s.close(); + return null; + } + }); + } + + private List saveSomeCitizens() throws Exception { final Citizen c1 = new Citizen(); c1.setFirstname( "Emmanuel" ); c1.setLastname( "Bernard" ); @@ -598,6 +645,11 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase { return null; } }); + + List citizens = new ArrayList(2); + citizens.add(c1); + citizens.add(c2); + return citizens; } private State getState(Session s, String name) {