You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by es...@apache.org on 2018/05/11 00:51:06 UTC

[geode] branch feature/GEODE-5173-1 updated: Fix an issue when a region entry is evicted using async disk writer thread. It needs to be faulted in by the transaction.

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

eshu11 pushed a commit to branch feature/GEODE-5173-1
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-5173-1 by this push:
     new 0d6374a  Fix an issue when a region entry is evicted using async disk writer thread. It needs to be faulted in by the transaction.
0d6374a is described below

commit 0d6374a289a6ac7bb16ac90caea2c771d7e86643
Author: eshu <es...@pivotal.io>
AuthorDate: Thu May 10 17:47:33 2018 -0700

    Fix an issue when a region entry is evicted using async disk writer thread. It needs to be faulted in by the transaction.
---
 .../apache/geode/internal/cache/LocalRegion.java   |  2 +-
 .../apache/geode/internal/cache/RegionEntry.java   |  4 ++
 .../cache/entries/AbstractDiskLRURegionEntry.java  |  4 ++
 .../PersistentRegionTransactionDUnitTest.java      | 62 ++++++++++++++++------
 4 files changed, 56 insertions(+), 16 deletions(-)

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 4c58c2e..3e903a2 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
@@ -8335,7 +8335,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
         synchronized (regionEntry) {
           if (!regionEntry.isRemoved()) {
             Object value = regionEntry.getValueInVM(this);
-            if (value == Token.NOT_AVAILABLE) {
+            if (value == Token.NOT_AVAILABLE || regionEntry.isEvictBitSet()) {
               // Entry value is on disk
               // Handle the case where we fault in a evicted disk entry
               needsLRUCleanup = txLRUStart();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java
index c16ad23..e2eb850 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionEntry.java
@@ -461,4 +461,8 @@ public interface RegionEntry {
   @Retained(ABSTRACT_REGION_ENTRY_PREPARE_VALUE_FOR_CACHE)
   Object prepareValueForCache(RegionEntryContext context, Object value, EntryEventImpl event,
       boolean isEntryUpdate);
+
+  default boolean isEvictBitSet() {
+    return false;
+  }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractDiskLRURegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractDiskLRURegionEntry.java
index ccfdafc..6dbb527 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractDiskLRURegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractDiskLRURegionEntry.java
@@ -31,4 +31,8 @@ public abstract class AbstractDiskLRURegionEntry extends AbstractOplogDiskRegion
 
   // Do not add any instance variables to this class.
   // Instead add them to the LRU section of LeafRegionEntry.cpp.
+  @Override
+  public boolean isEvictBitSet() {
+    return isEvicted();
+  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/PersistentRegionTransactionDUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/PersistentRegionTransactionDUnitTest.java
index bb356dc..8e7785b 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/PersistentRegionTransactionDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/PersistentRegionTransactionDUnitTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.internal.cache;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import java.util.stream.IntStream;
 
@@ -32,12 +33,14 @@ import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.EvictionAction;
 import org.apache.geode.cache.EvictionAttributes;
 import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.TransactionId;
 import org.apache.geode.cache.client.ClientCache;
 import org.apache.geode.cache.client.ClientCacheFactory;
 import org.apache.geode.cache.client.ClientRegionShortcut;
 import org.apache.geode.cache.server.CacheServer;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.rules.DistributedDiskDirRule;
 import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
 import org.apache.geode.test.junit.categories.DistributedTest;
 
@@ -46,14 +49,16 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
 
   private VM server;
   private VM client;
-  private final int KEY = 5;
-  private final String VALUE = "value 5";
-  private final String REGIONNAME = "region";
+  private static final int KEY = 5;
+  private static final String VALUE = "value 5";
+  private static final String REGIONNAME = "region";
 
   @Rule
   public DistributedRestoreSystemProperties restoreSystemProperties =
       new DistributedRestoreSystemProperties();
 
+  @Rule
+  public DistributedDiskDirRule distributedDiskDir = new DistributedDiskDirRule();
 
   @Before
   public void allowTransactions() {
@@ -71,10 +76,10 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
   @Test
   public void clientTransactionCanGetNotRecoveredEntryOnPersistentOverflowRegion()
       throws Exception {
-    createServer(server, true);
+    createServer(server, true, false);
     putData(server);
     server.invoke(() -> getCache().close());
-    int port = createServer(server, true);
+    int port = createServer(server, true, false);
 
 
     client.invoke(() -> {
@@ -97,15 +102,17 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
     });
   }
 
-  private int createServer(final VM server, boolean isOverflow) {
+  private int createServer(final VM server, boolean isOverflow, boolean isAsyncDiskWrite) {
     return server.invoke(() -> {
       if (!isOverflow) {
         System.setProperty(DiskStoreImpl.RECOVER_VALUE_PROPERTY_NAME, "false");
       }
       CacheFactory cacheFactory = new CacheFactory();
       Cache cache = getCache(cacheFactory);
+      cache.createDiskStoreFactory().setQueueSize(3).setTimeInterval(10000).create("disk");
       if (isOverflow) {
         cache.createRegionFactory(RegionShortcut.REPLICATE_PERSISTENT)
+            .setDiskSynchronous(!isAsyncDiskWrite).setDiskStoreName("disk")
             .setEvictionAttributes(
                 EvictionAttributes.createLRUEntryAttributes(1, EvictionAction.OVERFLOW_TO_DISK))
             .create(REGIONNAME);
@@ -120,7 +127,7 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
 
   @Test
   public void clientTransactionCanGetEvictedEntryOnPersistentOverflowRegion() throws Exception {
-    int port = createServer(server, true);
+    int port = createServer(server, true, false);
     putData(server);
     client.invoke(() -> {
       ClientCacheFactory factory = new ClientCacheFactory().addPoolServer("localhost", port);
@@ -137,7 +144,7 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
 
   @Test
   public void transactionCanGetEvictedEntryOnPersistentOverflowRegion() throws Exception {
-    createServer(server, true);
+    createServer(server, true, false);
     putData(server);
     server.invoke(() -> {
       LocalRegion region = (LocalRegion) getCache().getRegion(REGIONNAME);
@@ -154,10 +161,10 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
 
   @Test
   public void transactionCanGetNotRecoveredEntryOnPersistentOverflowRegion() throws Exception {
-    createServer(server, true);
+    createServer(server, true, false);
     putData(server);
     server.invoke(() -> getCache().close());
-    createServer(server, true);
+    createServer(server, true, false);
     server.invoke(() -> {
       LocalRegion region = (LocalRegion) getCache().getRegion("region");
       getCache().getCacheTransactionManager().begin();
@@ -170,11 +177,11 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
   }
 
   @Test
-  public void TransactionCanGetNotRecoveredEntryOnPersistentRegion() throws Exception {
-    createServer(server, false);
+  public void transactionCanGetNotRecoveredEntryOnPersistentRegion() throws Exception {
+    createServer(server, false, false);
     putData(server);
     server.invoke(() -> getCache().close());
-    createServer(server, false);
+    createServer(server, false, false);
     server.invoke(() -> {
       LocalRegion region = (LocalRegion) getCache().getRegion("region");
       assertThat(region.getValueInVM(KEY)).isNull();
@@ -189,10 +196,10 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
 
   @Test
   public void clientTransactionCanGetNotRecoveredEntryOnPersistentRegion() throws Exception {
-    createServer(server, false);
+    createServer(server, false, false);
     putData(server);
     server.invoke(() -> getCache().close());
-    int port = createServer(server, false);
+    int port = createServer(server, false, false);
 
 
     client.invoke(() -> {
@@ -207,4 +214,29 @@ public class PersistentRegionTransactionDUnitTest extends JUnit4CacheTestCase {
       }
     });
   }
+
+  @Test
+  public void transactionCanUpdateEntryOnAsyncOverflowRegion() throws Exception {
+    createServer(server, true, true);
+    server.invoke(() -> {
+      Cache cache = getCache();
+      DiskStoreImpl diskStore = (DiskStoreImpl) cache.findDiskStore("disk");
+      LocalRegion region = (LocalRegion) cache.getRegion("region");
+      region.put(1, "value1");
+      region.put(2, "value2"); // causes key 1 to be evicted and sits in the async queue
+      TXManagerImpl txManager = getCache().getTxManager();
+      txManager.begin();
+      assertNotEquals(region.getValueInVM(1), Token.NOT_AVAILABLE);
+      region.put(1, "new value");
+      TransactionId txId = txManager.suspend();
+      region.put(3, "value3");
+      region.put(4, "value4");
+      diskStore.flush();
+      txManager.resume(txId);
+
+      txManager.commit();
+
+      assertEquals("new value", region.get(1));
+    });
+  }
 }

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