This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new da5da79  GEODE-4651: fix transactional destroy entry leak (#1433)
da5da79 is described below

commit da5da792874f04b276a015f658f3a1a6a8301e4c
Author: Darrel Schneider <dschnei...@pivotal.io>
AuthorDate: Tue Feb 13 14:25:33 2018 -0800

    GEODE-4651: fix transactional destroy entry leak (#1433)
    
    When the transaction marks a region entry as no
    longer being used by the transaction, it no longer
    added it back to the eviction list if the entry
    is removed or destroyed.
---
 .../geode/internal/cache/InternalRegion.java       |  8 ++++
 .../apache/geode/internal/cache/LocalRegion.java   |  7 +---
 .../cache/entries/AbstractRegionEntry.java         |  8 ++--
 .../eviction/TransactionsWithOverflowTest.java     | 48 ++++++++++++++++++++++
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java 
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
index ed1f5d8..eeb2023 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
@@ -94,6 +94,14 @@ public interface InternalRegion extends Region, 
HasCachePerfStats, RegionEntryCo
 
   void addExpiryTaskIfAbsent(RegionEntry entry);
 
+  /**
+   * Used by unit tests to get access to the EntryExpiryTask of the given key. 
Returns null if the
+   * entry exists but does not have an expiry task.
+   *
+   * @throws EntryNotFoundException if no entry exists key.
+   */
+  EntryExpiryTask getEntryExpiryTask(Object key);
+
   DistributionManager getDistributionManager();
 
   void generateAndSetVersionTag(InternalCacheEvent event, RegionEntry entry);
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 77dc518..e6202ab 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -8045,12 +8045,7 @@ public class LocalRegion extends AbstractRegion 
implements LoaderHelperFactory,
     }
   }
 
-  /**
-   * Used by unit tests to get access to the EntryExpiryTask of the given key. 
Returns null if the
-   * entry exists but does not have an expiry task.
-   *
-   * @throws EntryNotFoundException if no entry exists key.
-   */
+  @Override
   public EntryExpiryTask getEntryExpiryTask(Object key) {
     RegionEntry re = this.getRegionEntry(key);
     if (re == null) {
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
index a57fd81..d614d1d 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
@@ -1468,9 +1468,11 @@ public abstract class AbstractRegionEntry implements 
HashRegionEntry<Object, Obj
     if (TXManagerImpl.decRefCount(this)) {
       if (isInUseByTransaction()) {
         setInUseByTransaction(false);
-        appendToEvictionList(evictionList);
-        if (region != null && region.isEntryExpiryPossible()) {
-          region.addExpiryTaskIfAbsent(this);
+        if (!isDestroyedOrRemoved()) {
+          appendToEvictionList(evictionList);
+          if (region != null && region.isEntryExpiryPossible()) {
+            region.addExpiryTaskIfAbsent(this);
+          }
         }
       }
     }
diff --git 
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/TransactionsWithOverflowTest.java
 
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/TransactionsWithOverflowTest.java
index 37e2735..6ef2121 100644
--- 
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/TransactionsWithOverflowTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/TransactionsWithOverflowTest.java
@@ -15,8 +15,12 @@
 
 package org.apache.geode.internal.cache.eviction;
 
+
+import static com.googlecode.catchexception.CatchException.catchException;
+import static com.googlecode.catchexception.CatchException.caughtException;
 import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.File;
 import java.util.Properties;
@@ -27,6 +31,9 @@ import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
 
 import org.apache.geode.cache.*;
+import org.apache.geode.internal.cache.InternalRegion;
+import org.apache.geode.internal.cache.VMLRURegionMap;
+import org.apache.geode.pdx.PdxInitializationException;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 /**
@@ -66,6 +73,47 @@ public class TransactionsWithOverflowTest {
     mgr.commit();
   }
 
+  @Test
+  public void verifyThatTransactionalDestroysRemoveFromTheEvictionList() 
throws Exception {
+    Cache cache = getCache();
+    RegionFactory<?, ?> rf = cache.createRegionFactory();
+    rf.setDataPolicy(DataPolicy.REPLICATE);
+    rf.setEvictionAttributes(
+        EvictionAttributes.createLRUEntryAttributes(1, 
EvictionAction.DEFAULT_EVICTION_ACTION));
+    rf.setConcurrencyChecksEnabled(false);
+    InternalRegion r = (InternalRegion) rf.create(name.getMethodName());
+    CacheTransactionManager mgr = cache.getCacheTransactionManager();
+    for (int i = 0; i < 2; i++) {
+      r.put(i, "value" + i);
+      mgr.begin();
+      r.destroy(i);
+      mgr.commit();
+    }
+
+    VMLRURegionMap regionMap = (VMLRURegionMap) r.getRegionMap();
+    assertThat(regionMap.getEvictionList().size()).isEqualTo(0);
+  }
+
+  @Test
+  public void verifyThatTransactionalDestroysRemoveFromExpiration() throws 
Exception {
+    Cache cache = getCache();
+    RegionFactory<?, ?> rf = cache.createRegionFactory();
+    rf.setDataPolicy(DataPolicy.REPLICATE);
+    rf.setEntryTimeToLive(new ExpirationAttributes(11, 
ExpirationAction.DESTROY));
+    rf.setConcurrencyChecksEnabled(false);
+    InternalRegion r = (InternalRegion) rf.create(name.getMethodName());
+    CacheTransactionManager mgr = cache.getCacheTransactionManager();
+    for (int i = 0; i < 1; i++) {
+      r.put(i, "value" + i);
+      mgr.begin();
+      r.destroy(i);
+      mgr.commit();
+    }
+
+    catchException(r).getEntryExpiryTask(0);
+    assertThat((Exception) 
caughtException()).isExactlyInstanceOf(EntryNotFoundException.class);
+  }
+
   private Cache getCache() {
     if (cache == null) {
       Properties props = new Properties();

-- 
To stop receiving notification emails like this one, please contact
dschnei...@apache.org.

Reply via email to