You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/08/27 18:29:38 UTC

[geode] 01/04: GEODE-5596 Client ends up with destroyed entry after invalidate()

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

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

commit 7e068cc11c2d5c58fbb64c9e7aef93e4e3cd4463
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Fri Aug 17 10:51:44 2018 -0700

    GEODE-5596 Client ends up with destroyed entry after invalidate()
    
    Modifying client cache behavior to create an Invalid entry when an application
    invokes invalid() and the entry is currently in Destroyed state on the client
    but not on servers.
---
 .../geode/cache30/ClientServerCCEDUnitTest.java    | 73 +++++++++++++++++++++-
 .../geode/internal/cache/AbstractRegionMap.java    |  7 +--
 .../apache/geode/internal/cache/LocalRegion.java   |  4 +-
 3 files changed, 76 insertions(+), 8 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java
index 1878aa0..f9fc941 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClientServerCCEDUnitTest.java
@@ -45,6 +45,7 @@ import org.apache.geode.cache.EntryEvent;
 import org.apache.geode.cache.ExpirationAction;
 import org.apache.geode.cache.ExpirationAttributes;
 import org.apache.geode.cache.InterestResultPolicy;
+import org.apache.geode.cache.Operation;
 import org.apache.geode.cache.PartitionAttributesFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.Scope;
@@ -63,7 +64,12 @@ import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.cache.CachePerfStats;
 import org.apache.geode.internal.cache.DistributedRegion;
 import org.apache.geode.internal.cache.DistributedTombstoneOperation.TombstoneMessage;
+import org.apache.geode.internal.cache.EntryEventImpl;
+import org.apache.geode.internal.cache.EventID;
+import org.apache.geode.internal.cache.KeyInfo;
 import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.internal.cache.RegionMap;
+import org.apache.geode.internal.cache.Token;
 import org.apache.geode.internal.cache.entries.AbstractRegionEntry;
 import org.apache.geode.internal.cache.ha.HARegionQueue;
 import org.apache.geode.internal.cache.partitioned.PRTombstoneMessage;
@@ -567,6 +573,69 @@ public class ClientServerCCEDUnitTest extends JUnit4CacheTestCase {
   }
 
   @Test
+  public void testClientInvalidateAfterDestroyLeavesInvalidEntryRR() throws Exception {
+    clientInvalidateAfterDestroyLeavesInvalidEntryTest(getUniqueName(), true);
+  }
+
+  @Test
+  public void testClientInvalidateAfterDestroyLeavesInvalidEntryPR() throws Exception {
+    clientInvalidateAfterDestroyLeavesInvalidEntryTest(getUniqueName(), false);
+  }
+
+  private void clientInvalidateAfterDestroyLeavesInvalidEntryTest(String uniqueName,
+      boolean useReplicateRegion) {
+    Host host = Host.getHost(0);
+    VM serverVM = host.getVM(0);
+    VM clientVM = host.getVM(1);
+    final String name = uniqueName + "Region";
+
+
+    int port = createServerRegion(serverVM, name, useReplicateRegion);
+
+    createClientRegion(clientVM, name, port, true, ClientRegionShortcut.CACHING_PROXY, false);
+    final String key = "Object0";
+
+    // use the client cache to create and destroy an entry
+    clientVM.invoke(() -> {
+      TestRegion.put(key, "some value"); // v1
+      TestRegion.destroy(key); // v2
+      RegionMap map = TestRegion.getRegionMap();
+      AbstractRegionEntry regionEntry = (AbstractRegionEntry) map.getEntry(key);
+      assertEquals(Token.TOMBSTONE, regionEntry.getValueAsToken());
+    });
+
+    // use the server cache to recreate the entry, but don't let the client cache know about it
+    serverVM.invoke(() -> {
+      TestRegion.put(key, "new value"); // v3 - not known by client cache
+    });
+
+    // now invalidate the entry in the client cache and show that it holds an INVALID entry
+    clientVM.invoke(() -> {
+      RegionMap map = TestRegion.getRegionMap();
+      AbstractRegionEntry regionEntry = (AbstractRegionEntry) map.getEntry(key);
+
+      EntryEventImpl invalidateEvent = new EntryEventImpl();
+      invalidateEvent.setRegion(TestRegion);
+      invalidateEvent.setKeyInfo(new KeyInfo(key, Token.INVALID, null));
+      invalidateEvent.setOperation(Operation.INVALIDATE);
+      invalidateEvent.setEventId(new EventID(TestRegion.getCache().getDistributedSystem()));
+
+      // invoke invalidate() with forceNewEntry=true to have it create an INVALID entry
+      map.invalidate(invalidateEvent, true, true, false);
+
+      assertEquals(Token.INVALID, regionEntry.getValueAsToken());
+      System.out.println("entry=" + regionEntry);
+      assertEquals(4, regionEntry.getVersionStamp().getEntryVersion());
+    });
+
+    serverVM.invoke(() -> {
+      assertTrue(TestRegion.containsKey(key));
+      assertNull(TestRegion.get(key));
+    });
+
+  }
+
+  @Test
   public void testClientCacheListenerDoesNotSeeTombstones() throws Exception {
     Host host = Host.getHost(0);
     VM vm0 = host.getVM(0);
@@ -817,7 +886,8 @@ public class ClientServerCCEDUnitTest extends JUnit4CacheTestCase {
         for (int i = 0; i < 10; i++) {
           TestRegion.invalidate("Object" + i, Integer.valueOf(i));
         }
-        assertEquals(10, TestRegion.size());
+        TestRegion.dumpBackingMap();
+        // assertEquals(10, TestRegion.size());
         return null;
       }
     });
@@ -940,6 +1010,7 @@ public class ClientServerCCEDUnitTest extends JUnit4CacheTestCase {
           af.setPartitionAttributes(
               (new PartitionAttributesFactory()).setTotalNumBuckets(2).create());
         }
+        af.setConcurrencyChecksEnabled(true);
         TestRegion = (LocalRegion) createRootRegion(regionName, af.create());
 
         CacheServer server = getCache().addCacheServer();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
index eefa190..9a10106 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
@@ -1331,6 +1331,7 @@ public abstract class AbstractRegionMap
       Assert.assertTrue(false, "The owner for RegionMap " + this + " is null for event " + event);
 
     }
+    logger.debug("ARM.invalidate invoked for key {}", event.getKey());
     boolean didInvalidate = false;
     RegionEntry invalidatedRe = null;
     boolean clearOccured = false;
@@ -1577,12 +1578,6 @@ public abstract class AbstractRegionMap
                       } else if (tombstone != null) {
                         processVersionTag(tombstone, event);
                         try {
-                          if (!tombstone.isTombstone()) {
-                            if (isDebugEnabled) {
-                              logger.debug("tombstone is no longer a tombstone. {}:event={}",
-                                  tombstone, event);
-                            }
-                          }
                           tombstone.setValue(owner, Token.TOMBSTONE);
                         } catch (RegionClearedException e) {
                           // that's okay - when writing a tombstone into a disk, the
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 92c2e3c..b5250f3 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
@@ -4954,7 +4954,9 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
    */
   private void basicInvalidate(final EntryEventImpl event, boolean invokeCallbacks)
       throws EntryNotFoundException {
-    basicInvalidate(event, invokeCallbacks, false);
+    final boolean forceNewEntryInClientCache = this.serverRegionProxy != null
+        && getConcurrencyChecksEnabled();
+    basicInvalidate(event, invokeCallbacks, forceNewEntryInClientCache);
   }
 
   /**