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/30 15:00:18 UTC
[geode] branch develop updated: 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 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 c6fe1df GEODE-5596 Client ends up with destroyed entry after invalidate()
c6fe1df is described below
commit c6fe1df8c7273e2122be8505ce0baacccfc1c026
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Thu Aug 30 07:55:14 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.
I also changed the AttributesFactory javadoc for
concurrencyChecksEnabled to note that the default value is true, not
false, and added a test for this setting.
this closes #2384
---
.../geode/cache30/ClientServerCCEDUnitTest.java | 72 +++++++++++++++++++++-
.../org/apache/geode/cache/AttributesFactory.java | 2 +-
.../geode/internal/cache/AbstractRegionMap.java | 7 +--
.../apache/geode/internal/cache/LocalRegion.java | 4 +-
.../geode/cache/AttributesFactoryJUnitTest.java | 1 +
.../sockets/ClientServerMiscDUnitTestBase.java | 31 ++++------
6 files changed, 87 insertions(+), 30 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..08a7edb 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);
@@ -606,9 +675,7 @@ public class ClientServerCCEDUnitTest extends JUnit4CacheTestCase {
private void unregisterInterest(VM vm) {
vm.invoke(new SerializableRunnable("unregister interest in all keys") {
public void run() {
- // TestRegion.dumpBackingMap();
TestRegion.unregisterInterestRegex(".*");
- // TestRegion.dumpBackingMap();
}
});
}
@@ -940,6 +1007,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/cache/AttributesFactory.java b/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
index 2017cf2..93d9a99 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
@@ -193,7 +193,7 @@ import org.apache.geode.internal.i18n.LocalizedStrings;
* others will only read. <br>
* {@link #setConcurrencyLevel} {@link RegionAttributes#getConcurrencyLevel}</dd>
*
- * <dt>ConcurrencyChecksEnabled [<em>default:</em> {@code false}]</dt>
+ * <dt>ConcurrencyChecksEnabled [<em>default:</em> {@code true}]</dt>
* <dd>Enables a distributed versioning algorithm that detects concurrency conflicts in regions and
* ensures that changes to an entry are not applied in a different order in other members. This can
* cause operations to be conflated, so that some cache listeners may see an event while others do
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 76ed86c..bc1ca9b 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;
@@ -1573,12 +1574,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);
}
/**
diff --git a/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
index a1d39e1..9b6ed7d 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
@@ -265,6 +265,7 @@ public class AttributesFactoryJUnitTest {
assertNotNull(diskSizes);
assertEquals(1, diskSizes.length);
assertEquals(DiskStoreFactory.DEFAULT_DISK_DIR_SIZE, diskSizes[0]);
+ assertTrue(attrs.getConcurrencyChecksEnabled());
}
@Test
diff --git a/geode-dunit/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTestBase.java b/geode-dunit/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTestBase.java
index 3a03664..aeb22bf 100755
--- a/geode-dunit/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTestBase.java
+++ b/geode-dunit/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientServerMiscDUnitTestBase.java
@@ -17,6 +17,7 @@ package org.apache.geode.internal.cache.tier.sockets;
import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -46,6 +47,7 @@ import org.apache.geode.cache.CacheException;
import org.apache.geode.cache.CacheWriterException;
import org.apache.geode.cache.DataPolicy;
import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.InterestResultPolicy;
import org.apache.geode.cache.PartitionAttributesFactory;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionAttributes;
@@ -556,27 +558,16 @@ public class ClientServerMiscDUnitTestBase extends JUnit4CacheTestCase {
Region region = static_cache.getRegion(REGION_NAME1);
populateCache();
region.put("invalidationKey", "invalidationValue");
+
region.localDestroy("invalidationKey");
- if (region.containsKey("invalidationKey")) {
- fail("region still contains invalidationKey");
- }
+ assertThat(region.containsKey("invalidationKey")).isFalse();
+
region.invalidate("invalidationKey");
- if (region.containsKey("invalidationKey")) {
- fail(
- "this test expects the entry is not created on invalidate() if not there before the operation");
- }
+ assertThat(region.containsKey("invalidationKey")).isTrue();
+
Object value = region.get("invalidationKey");
- if (value != null) {
- fail("this test expected a null response to get('invalidationKey')");
- }
- if (!region.containsKeyOnServer("invalidationKey")) {
- fail("expected an entry on the server after invalidation");
- }
- // bug 43407 asserts that there should be an entry, but the product does not
- // do this. This verifies that the product does not behave as asserted in that bug
- if (region.containsKey("invalidationKey")) {
- fail("expected no entry after invalidation when entry was not in client but was on server");
- }
+ assertThat(value).isNull();
+ assertThat(region.containsKeyOnServer("invalidationKey")).isTrue();
}
/**
@@ -989,8 +980,8 @@ public class ClientServerMiscDUnitTestBase extends JUnit4CacheTestCase {
assertNotNull(r1);
Region r2 = cache.getRegion(Region.SEPARATOR + REGION_NAME2);
assertNotNull(r2);
- r1.registerInterest("ALL_KEYS", false, false);
- r2.registerInterest("ALL_KEYS", false, false);
+ r1.registerInterestForAllKeys(InterestResultPolicy.KEYS, false, false);
+ r2.registerInterestForAllKeys(InterestResultPolicy.KEYS, false, false);
} catch (CacheWriterException e) {
e.printStackTrace();
fail("Test failed due to CacheWriterException during registerInterestnBothRegions" + e);