From d88f65ff0c6e9de465a6d3dfb868515852e0ba31 Mon Sep 17 00:00:00 2001 From: Galder Zamarreno Date: Fri, 12 Feb 2010 12:21:43 +0000 Subject: [PATCH] [HHH-4836] (Infinispan: 2L QueryCache don't considers cached queries which belong to current transaction) Fixed by not suspending transactions on get any more, since no locks are aquired on get. git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18790 1b8cb986-b30d-0410-93ca-fae66ebed9b2 --- .../cache/infinispan/impl/BaseRegion.java | 23 ++++------ .../query/QueryResultsRegionImpl.java | 9 ++-- .../timestamp/TimestampsRegionImpl.java | 2 +- .../BasicTransactionalTestCase.java | 43 +++++++++++++++++++ 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java index 9d586e3751..092282e319 100644 --- a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java +++ b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java @@ -194,28 +194,21 @@ public abstract class BaseRegion implements Region { } /** - * Performs a JBoss Cache get(Fqn, Object) after first - * {@link #suspend suspending any ongoing transaction}. Wraps any exception - * in a {@link CacheException}. Ensures any ongoing transaction is resumed. - * + * Performs a Infinispan get(Fqn, Object) + * * @param key The key of the item to get * @param opt any option to add to the get invocation. May be null * @param suppressTimeout should any TimeoutException be suppressed? * @return The retrieved object * @throws CacheException issue managing transaction or talking to cache */ - protected Object suspendAndGet(Object key, FlagAdapter opt, boolean suppressTimeout) throws CacheException { - Transaction tx = suspend(); - try { - if (suppressTimeout) - return cacheAdapter.getAllowingTimeout(key); - else - return cacheAdapter.get(key); - } finally { - resume(tx); - } + protected Object get(Object key, FlagAdapter opt, boolean suppressTimeout) throws CacheException { + if (suppressTimeout) + return cacheAdapter.getAllowingTimeout(key); + else + return cacheAdapter.get(key); } - + public Object getOwnerForPut() { Transaction tx = null; try { diff --git a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/query/QueryResultsRegionImpl.java b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/query/QueryResultsRegionImpl.java index 63b22417b5..6e2b6f77cd 100644 --- a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/query/QueryResultsRegionImpl.java +++ b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/query/QueryResultsRegionImpl.java @@ -50,12 +50,13 @@ public class QueryResultsRegionImpl extends BaseTransactionalDataRegion implemen if (!checkValid()) return null; - // Don't hold the JBC node lock throughout the tx, as that - // prevents updates + // In Infinispan get doesn't acquire any locks, so no need to suspend the tx. + // In the past, when get operations acquired locks, suspending the tx was a way + // to avoid holding locks that would prevent updates. // Add a zero (or low) timeout option so we don't block // waiting for tx's that did a put to commit - return suspendAndGet(key, FlagAdapter.ZERO_LOCK_ACQUISITION_TIMEOUT, true); - } + return get(key, FlagAdapter.ZERO_LOCK_ACQUISITION_TIMEOUT, true); + } public void put(Object key, Object value) throws CacheException { if (checkValid()) { diff --git a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/timestamp/TimestampsRegionImpl.java b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/timestamp/TimestampsRegionImpl.java index 47ebbd0824..e7de3ded8d 100644 --- a/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/timestamp/TimestampsRegionImpl.java +++ b/cache-infinispan/src/main/java/org/hibernate/cache/infinispan/timestamp/TimestampsRegionImpl.java @@ -58,7 +58,7 @@ public class TimestampsRegionImpl extends BaseGeneralDataRegion implements Times public Object get(Object key) throws CacheException { Object value = localCache.get(key); if (value == null && checkValid()) { - value = suspendAndGet(key, null, false); + value = get(key, null, false); if (value != null) localCache.put(key, value); } diff --git a/cache-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java b/cache-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java index da0a96614b..aa47c521dc 100644 --- a/cache-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java +++ b/cache-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java @@ -287,4 +287,47 @@ public class BasicTransactionalTestCase extends SingleNodeTestCase { commitOrRollbackTx(); } } + + public void testQueryCacheHitInSameTransaction() throws Exception { + Session s = null; + Item item = new Item("galder", "Galder's Item"); + + beginTx(); + try { + s = openSession(); + s.getTransaction().begin(); + s.persist(item); + s.getTransaction().commit(); + s.close(); + } catch (Exception e) { + setRollbackOnlyTx(e); + } finally { + commitOrRollbackTx(); + } + + beginTx(); + try { + s = openSession(); + Statistics stats = s.getSessionFactory().getStatistics(); + s.createQuery("from Item").setCacheable(true).list(); + s.createQuery("from Item").setCacheable(true).list(); + assertEquals(1, stats.getQueryCacheHitCount()); + s.close(); + } catch (Exception e) { + setRollbackOnlyTx(e); + } finally { + commitOrRollbackTx(); + } + + beginTx(); + try { + s = openSession(); + s.createQuery("delete from Item").executeUpdate(); + s.close(); + } catch (Exception e) { + setRollbackOnlyTx(e); + } finally { + commitOrRollbackTx(); + } + } }