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/01 20:31:42 UTC
[geode] branch develop updated: GEODE-5505 Cache listener not
invoked on a retried destroy() operation
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 08f1106 GEODE-5505 Cache listener not invoked on a retried destroy() operation
08f1106 is described below
commit 08f110650834c6d03accef8e98c0cf4cc47bc3fb
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Wed Aug 1 13:29:43 2018 -0700
GEODE-5505 Cache listener not invoked on a retried destroy() operation
This started happening when we moved listener/client notification to
LocalRegion.basicDestroyPart2. The code to handle duplicate destroys
from a client was only invoking basicDestroyPart3.
This closes #2233
---
.../geode/internal/cache/DistributedRegion.java | 4 +--
.../geode/internal/cache/map/RegionMapDestroy.java | 3 ++
.../internal/cache/tier/sockets/command/Put61.java | 2 +-
.../internal/cache/tier/sockets/command/Put65.java | 2 +-
.../geode/internal/i18n/LocalizedStrings.java | 2 +-
.../internal/cache/map/RegionMapDestroyTest.java | 35 ++++++++++++++++++++++
6 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
index 2fba672..9b395fc 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java
@@ -381,8 +381,8 @@ public class DistributedRegion extends LocalRegion implements InternalDistribute
// if the listeners have already seen this event, then it has already
// been successfully applied to the cache. Distributed messages and
// return
- if (isTraceEnabled) {
- logger.trace("DR.virtualPut: this cache has already seen this event {}", event);
+ if (logger.isDebugEnabled()) {
+ logger.debug("DR.virtualPut: this cache has already seen this event {}", event);
}
// Fix 39014: when hasSeenEvent, put will still distribute
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
index 6a34c50..4d9eed0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
@@ -551,6 +551,9 @@ public class RegionMapDestroy {
if (regionEntry == null) {
regionEntry = newRegionEntry;
}
+ // invoke listeners and inform clients
+ internalRegion.basicDestroyPart2(regionEntry, event, inTokenMode,
+ false, duringRI, true);
doPart3 = true;
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put61.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put61.java
index b3adcb6..df60d00 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put61.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put61.java
@@ -235,7 +235,7 @@ public class Put61 extends BaseCommand {
} catch (InvalidDeltaException ide) {
logger.info(LocalizedMessage.create(
LocalizedStrings.UpdateOperation_ERROR_APPLYING_DELTA_FOR_KEY_0_OF_REGION_1,
- new Object[] {key, regionName}));
+ new Object[] {key, regionName, ide.getMessage()}));
writeException(clientMessage, MessageType.PUT_DELTA_ERROR, ide, false, serverConnection);
serverConnection.setAsTrue(RESPONDED);
region.getCachePerfStats().incDeltaFullValuesRequested();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
index 8732717..95e8e95 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
@@ -420,7 +420,7 @@ public class Put65 extends BaseCommand {
} catch (InvalidDeltaException ide) {
logger.info(LocalizedMessage.create(
LocalizedStrings.UpdateOperation_ERROR_APPLYING_DELTA_FOR_KEY_0_OF_REGION_1,
- new Object[] {key, regionName}));
+ new Object[] {key, regionName, ide.getMessage()}));
writeException(clientMessage, MessageType.PUT_DELTA_ERROR, ide, false, serverConnection);
serverConnection.setAsTrue(RESPONDED);
region.getCachePerfStats().incDeltaFullValuesRequested();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
index 460b185..768f228 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
@@ -6236,7 +6236,7 @@ public class LocalizedStrings {
public static final StringId RegionAttributesCreation__CLONING_ENABLE_IS_NOT_THE_SAME_THIS_0_OTHER_1 =
new StringId(4741, "Cloning enabled is not the same: this: {0} other: {1}");
public static final StringId UpdateOperation_ERROR_APPLYING_DELTA_FOR_KEY_0_OF_REGION_1 =
- new StringId(4742, "Error applying delta for key {0} of region {1}");
+ new StringId(4742, "Error applying delta for key {0} of region {1}: {2}");
public static final StringId RequestEventValue_0_THE_EVENT_ID_FOR_THE_GET_EVENT_VALUE_REQUEST_IS_NULL =
new StringId(4744, "{0}: The event id for the get event value request is null.");
public static final StringId RequestEventValue_UNABLE_TO_FIND_A_CLIENT_UPDATE_MESSAGE_FOR_0 =
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
index 774f090..b65016a 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapDestroyTest.java
@@ -20,6 +20,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -35,6 +36,7 @@ import org.apache.geode.cache.DataPolicy;
import org.apache.geode.cache.EntryNotFoundException;
import org.apache.geode.cache.EvictionAttributes;
import org.apache.geode.cache.Operation;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.internal.cache.AbstractRegionMap;
import org.apache.geode.internal.cache.CachePerfStats;
import org.apache.geode.internal.cache.EntryEventImpl;
@@ -49,7 +51,9 @@ import org.apache.geode.internal.cache.VMLRURegionMap;
import org.apache.geode.internal.cache.eviction.EvictableEntry;
import org.apache.geode.internal.cache.eviction.EvictionController;
import org.apache.geode.internal.cache.eviction.EvictionCounters;
+import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
import org.apache.geode.internal.cache.versions.RegionVersionVector;
+import org.apache.geode.internal.cache.versions.VersionStamp;
import org.apache.geode.internal.cache.versions.VersionTag;
import org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap;
@@ -172,6 +176,20 @@ public class RegionMapDestroyTest {
arm.getEntryMap().put(KEY, entry);
}
+ private void givenExistingEntryWithValueAndVersion(Object value, VersionTag version) {
+ RegionEntry entry = arm.getEntryFactory().createEntry(arm._getOwner(), KEY, value);
+ ((VersionStamp) entry).setVersions(version);
+ arm.getEntryMap().put(KEY, entry);
+ }
+
+ private void givenExistingEntryWithValueAndSameVersion(Object value, VersionTag version) {
+ givenExistingEntryWithValueAndVersion(value, version);
+
+ RegionVersionVector<?> versionVector = mock(RegionVersionVector.class);
+ when(arm._getOwner().getVersionVector()).thenReturn(versionVector);
+ event.setVersionTag(version);
+ }
+
private void givenExistingEntry(Object value) {
RegionEntry entry = arm.getEntryFactory().createEntry(arm._getOwner(), KEY, value);
arm.getEntryMap().put(KEY, entry);
@@ -224,6 +242,23 @@ public class RegionMapDestroyTest {
}
@Test
+ public void destroyWithDuplicateVersionInvokesListener() {
+ givenEmptyRegionMap();
+ givenConcurrencyChecks(true);
+ VersionTag version = VersionTag.create(new InternalDistributedMember("localhost", 123));
+ version.setEntryVersion(1);
+ version.setRegionVersion(1);
+ givenExistingEntryWithValueAndSameVersion(Token.TOMBSTONE, version);
+ // make this a client/server operation
+ event.setContext(new ClientProxyMembershipID());
+ assertThat(arm.destroy(event, inTokenMode, duringRI, cacheWrite, isEviction, expectedOldValue,
+ removeRecoveredEntry)).isTrue();
+ assertThat(event.getIsRedestroyedEntry()).isTrue();
+ verify(owner).basicDestroyPart2(isA(RegionEntry.class), isA(EntryEventImpl.class),
+ isA(Boolean.class), isA(Boolean.class), isA(Boolean.class), isA(Boolean.class));
+ }
+
+ @Test
public void destroyWithEmptyRegionThrowsException() {
givenConcurrencyChecks(false);
givenEmptyRegionMap();