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 <[email protected]>
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);
