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 2019/04/12 20:21:17 UTC
[geode] branch feature/GEODE-6195b created (now 9f85d83)
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a change to branch feature/GEODE-6195b
in repository https://gitbox.apache.org/repos/asf/geode.git.
at 9f85d83 GEODE-6195 putIfAbsent may get a returned value caused by the same operation due to retry
This branch includes the following new commits:
new 9f85d83 GEODE-6195 putIfAbsent may get a returned value caused by the same operation due to retry
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[geode] 01/01: GEODE-6195 putIfAbsent may get a returned value
caused by the same operation due to retry
Posted by bs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch feature/GEODE-6195b
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 9f85d83e031ad8a46267c014baea53bd8970bea0
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Fri Apr 12 13:15:03 2019 -0700
GEODE-6195 putIfAbsent may get a returned value caused by the same operation due to retry
Moving the check for a retried putIfAbsent to be under the RegionEntry
lock. This allows the operation to propagate throughout the cluster and
allows the client to receive a valid version tag, if concurrency checks
are enabled.
This also fixes GEODE-6630, "RedundancyLogger might throw NPE" because
tests were frequently running into that issue.
---
.../cache/RetryPutIfAbsentIntegrationTest.java | 87 ++++++++++++
.../apache/geode/internal/cache/LocalRegion.java | 62 +--------
.../internal/cache/PRHARedundancyProvider.java | 3 +-
.../cache/event/DistributedEventTracker.java | 7 +
.../geode/internal/cache/event/EventTracker.java | 5 +
.../cache/event/NonDistributedEventTracker.java | 5 +
.../geode/internal/cache/map/RegionMapPut.java | 37 +++++
.../geode/internal/cache/LocalRegionTest.java | 150 ---------------------
.../geode/internal/cache/map/RegionMapPutTest.java | 21 +++
9 files changed, 167 insertions(+), 210 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/RetryPutIfAbsentIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/RetryPutIfAbsentIntegrationTest.java
new file mode 100644
index 0000000..f79c2d2
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/RetryPutIfAbsentIntegrationTest.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache;
+
+import static org.apache.geode.internal.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import org.apache.geode.cache.query.CacheUtils;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.EventID;
+import org.apache.geode.internal.cache.EventIDHolder;
+import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.internal.cache.VMCachedDeserializable;
+import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
+
+public class RetryPutIfAbsentIntegrationTest {
+
+ Cache cache;
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void duplicatePutIfAbsentIsAccepted() {
+ final String key = "mykey";
+ final String value = "myvalue";
+
+ LocalRegion myregion =
+ (LocalRegion) CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE)
+ .setConcurrencyChecksEnabled(true).create("myregion");
+
+ ClientProxyMembershipID id =
+ new ClientProxyMembershipID(new InternalDistributedMember("localhost", 1));
+ EventIDHolder clientEvent = new EventIDHolder(new EventID(new byte[] {1, 2, 3, 4, 5}, 1, 1));
+ clientEvent.setRegion(myregion);
+ byte[] valueBytes = new VMCachedDeserializable("myvalue", 7).getSerializedValue();
+ System.out.println("first putIfAbsent");
+ Object oldValue =
+ myregion.basicBridgePutIfAbsent("mykey", valueBytes, true, null, id, true, clientEvent);
+ assertEquals(null, oldValue);
+ assertTrue(myregion.containsKey(key));
+
+ myregion.getEventTracker().clear();
+
+ clientEvent = new EventIDHolder(new EventID(new byte[] {1, 2, 3, 4, 5}, 1, 1));
+ clientEvent.setRegion(myregion);
+ clientEvent.setPossibleDuplicate(true);
+ clientEvent.setOperation(Operation.PUT_IF_ABSENT);
+ assertFalse(myregion.getEventTracker().hasSeenEvent(clientEvent));
+
+ System.out.println("second putIfAbsent");
+ oldValue =
+ myregion.basicBridgePutIfAbsent("mykey", valueBytes, true, null, id, true, clientEvent);
+ assertEquals(null, oldValue);
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ CacheUtils.startCache();
+ cache = CacheUtils.getCache();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ CacheUtils.closeCache();
+ }
+
+}
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 2dae4a2..c25d614 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
@@ -3128,44 +3128,8 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
@VisibleForTesting
void checkPutIfAbsentResult(EntryEventImpl event, Object value, Object result) {
if (result != null) {
- // we may see a non null result possibly due to retry
- if (event.hasRetried() && putIfAbsentResultHasSameValue(true, value, result)) {
- if (logger.isDebugEnabled()) {
- logger.debug("retried putIfAbsent and result is the value to be put,"
- + " treat as a successful putIfAbsent");
- }
- } else {
- // customers don't see this exception
- throw new EntryNotFoundException("entry existed for putIfAbsent");
- }
- }
- }
-
- @VisibleForTesting
- boolean putIfAbsentResultHasSameValue(boolean isClient, Object valueToBePut, Object result) {
- if (Token.isInvalid(result) || result == null) {
- return valueToBePut == null;
+ throw new EntryNotFoundException("entry existed for putIfAbsent");
}
-
- boolean isCompressedOffHeap =
- isClient ? false : getAttributes().getOffHeap() && getAttributes().getCompressor() != null;
- return ValueComparisonHelper.checkEquals(valueToBePut, result, isCompressedOffHeap, getCache());
- }
-
- @VisibleForTesting
- boolean bridgePutIfAbsentResultHasSameValue(byte[] valueToBePut, boolean isValueToBePutObject,
- Object result) {
- if (Token.isInvalid(result) || result == null) {
- return valueToBePut == null;
- }
-
- boolean isCompressedOffHeap =
- getAttributes().getOffHeap() && getAttributes().getCompressor() != null;
- if (isValueToBePutObject) {
- return ValueComparisonHelper.checkEquals(EntryEventImpl.deserialize(valueToBePut), result,
- isCompressedOffHeap, getCache());
- }
- return ValueComparisonHelper.checkEquals(valueToBePut, result, isCompressedOffHeap, getCache());
}
/**
@@ -10623,15 +10587,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
final boolean ifOld = false;
final boolean requireOldValue = true;
if (!basicPut(event, ifNew, ifOld, oldValue, requireOldValue)) {
- Object result = event.getOldValue();
- if (event.isPossibleDuplicate() && putIfAbsentResultHasSameValue(false, value, result)) {
- if (logger.isDebugEnabled()) {
- logger.debug("possible duplicate putIfAbsent event and result is the value to be put,"
- + " treat this as a successful putIfAbsent");
- }
- return null;
- }
- return result;
+ return event.getOldValue();
} else {
if (!getDataView().isDeferredStats()) {
getCachePerfStats().endPut(startPut, false);
@@ -10832,6 +10788,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
// if this is a replayed operation we may already have a version tag
event.setVersionTag(clientEvent.getVersionTag());
+ event.setPossibleDuplicate(clientEvent.isPossibleDuplicate());
// Set the new value to the input byte[] if it isn't null
if (value != null) {
@@ -10870,19 +10827,6 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
clientEvent.setVersionTag(event.getVersionTag());
clientEvent.isConcurrencyConflict(event.isConcurrencyConflict());
} else {
- if (value != null) {
- assert (value instanceof byte[]);
- }
- if (event.isPossibleDuplicate()
- && bridgePutIfAbsentResultHasSameValue((byte[]) value, isObject, oldValue)) {
- // result is possibly due to the retry
- if (logger.isDebugEnabled()) {
- logger.debug("retried putIfAbsent and got oldValue as the value to be put,"
- + " treat this as a successful putIfAbsent");
- }
- return null;
- }
-
if (oldValue == null) {
// fix for 42189, putIfAbsent on server can return null if the
// operation was not performed (oldValue in cache was null).
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
index 2942154..7bf76b7 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
@@ -1708,12 +1708,13 @@ public class PRHARedundancyProvider {
new ArrayList<ProxyBucketRegion>(proxyBucketArray.length);
+ allBucketsRecoveredFromDisk = new CountDownLatch(proxyBucketArray.length);
+
/*
* Start the redundancy logger before recovering any proxy buckets.
*/
startRedundancyLogger(proxyBucketArray.length);
- allBucketsRecoveredFromDisk = new CountDownLatch(proxyBucketArray.length);
/*
* Spawn a separate thread for bucket that we previously hosted to recover that bucket.
*
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java b/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
index 6301183..e4f9711 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/event/DistributedEventTracker.java
@@ -128,6 +128,13 @@ public class DistributedEventTracker implements EventTracker {
}
@Override
+ public void clear() {
+ recordedEvents.clear();
+ recordedBulkOps.clear();
+ recordedBulkOpVersionTags.clear();
+ }
+
+ @Override
public Map<ThreadIdentifier, EventSequenceNumberHolder> getState() {
Map<ThreadIdentifier, EventSequenceNumberHolder> result = new HashMap<>(recordedEvents.size());
for (Map.Entry<ThreadIdentifier, EventSequenceNumberHolder> entry : recordedEvents.entrySet()) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/event/EventTracker.java b/geode-core/src/main/java/org/apache/geode/internal/cache/event/EventTracker.java
index 43a0458..e947c4b 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/event/EventTracker.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/event/EventTracker.java
@@ -133,4 +133,9 @@ public interface EventTracker {
*/
boolean isInitialImageProvider(DistributedMember mbr);
+ /**
+ * clear the tracker
+ */
+ void clear();
+
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/event/NonDistributedEventTracker.java b/geode-core/src/main/java/org/apache/geode/internal/cache/event/NonDistributedEventTracker.java
index f291c30..67d75e3 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/event/NonDistributedEventTracker.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/event/NonDistributedEventTracker.java
@@ -50,6 +50,11 @@ public class NonDistributedEventTracker implements EventTracker {
}
@Override
+ public void clear() {
+ // nothing to clear
+ }
+
+ @Override
public Map<ThreadIdentifier, EventSequenceNumberHolder> getState() {
return null;
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
index 1b419f0..4554590 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapPut.java
@@ -18,6 +18,8 @@ package org.apache.geode.internal.cache.map;
import java.util.Set;
+import org.apache.logging.log4j.Logger;
+
import org.apache.geode.cache.CacheWriter;
import org.apache.geode.cache.DiskAccessException;
import org.apache.geode.cache.Operation;
@@ -27,10 +29,12 @@ import org.apache.geode.internal.cache.InternalRegion;
import org.apache.geode.internal.cache.RegionClearedException;
import org.apache.geode.internal.cache.RegionEntry;
import org.apache.geode.internal.cache.Token;
+import org.apache.geode.internal.cache.ValueComparisonHelper;
import org.apache.geode.internal.cache.entries.AbstractRegionEntry;
import org.apache.geode.internal.cache.versions.ConcurrentCacheModificationException;
import org.apache.geode.internal.cache.versions.VersionTag;
import org.apache.geode.internal.cache.wan.GatewaySenderEventImpl;
+import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.offheap.OffHeapHelper;
import org.apache.geode.internal.offheap.ReferenceCountHelper;
import org.apache.geode.internal.offheap.annotations.Released;
@@ -38,11 +42,14 @@ import org.apache.geode.internal.offheap.annotations.Unretained;
import org.apache.geode.internal.sequencelog.EntryLogger;
public class RegionMapPut extends AbstractRegionMapPut {
+ protected static final Logger logger = LogService.getLogger();
+
private final CacheModificationLock cacheModificationLock;
private final EntryEventSerialization entryEventSerialization;
private final boolean ifNew;
private final boolean ifOld;
private final boolean overwriteDestroyed;
+ private boolean overwritePutIfAbsent;
private final boolean requireOldValue;
private final boolean retrieveOldValueForDelta;
private final boolean replaceOnClient;
@@ -104,6 +111,10 @@ public class RegionMapPut extends AbstractRegionMapPut {
return replaceOnClient;
}
+ boolean isOverwritePutIfAbsent() {
+ return overwritePutIfAbsent;
+ }
+
boolean isCacheWrite() {
return cacheWrite;
}
@@ -214,6 +225,9 @@ public class RegionMapPut extends AbstractRegionMapPut {
protected void unsetOldValueForDelta() {
OffHeapHelper.release(getOldValueForDelta());
setOldValueForDelta(null);
+ if (this.overwritePutIfAbsent) {
+ getEvent().setOldValue(null);
+ }
}
@Override
@@ -396,12 +410,35 @@ public class RegionMapPut extends AbstractRegionMapPut {
private boolean checkCreatePreconditions() {
if (isIfNew()) {
if (!getRegionEntry().isDestroyedOrRemoved()) {
+ // retain the version stamp of the existing entry for use in processing failures
+ EntryEventImpl event = getEvent();
+ if (event.getRegion().getConcurrencyChecksEnabled() &&
+ event.getOperation() == Operation.PUT_IF_ABSENT &&
+ !event.hasValidVersionTag() &&
+ event.isPossibleDuplicate()) {
+ if (ValueComparisonHelper.checkEquals(getRegionEntry().getValue(),
+ getEvent().basicGetNewValue(),
+ isCompressedOffHeap(event), event.getRegion().getCache())) {
+ logger.info("retried putIfAbsent found same value already in cache "
+ + "- allowing the operation. entry={}; event={}", getRegionEntry(), getEvent());
+ event.setPossibleDuplicate(true);
+ this.overwritePutIfAbsent = true;
+ return true;
+ }
+ }
return false;
}
}
return true;
}
+
+ private boolean isCompressedOffHeap(EntryEventImpl event) {
+ return event.getContext() != null ? false
+ : event.getRegion().getAttributes().getOffHeap()
+ && event.getRegion().getAttributes().getCompressor() != null;
+ }
+
private boolean checkExpectedOldValuePrecondition() {
// replace is propagated to server, so no need to check
// satisfiesOldValue on client
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
index ba4096b..8a4e126 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.internal.cache;
-import static org.assertj.core.api.Java6Assertions.assertThat;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -91,153 +90,4 @@ public class LocalRegionTest {
region.checkPutIfAbsentResult(event, value, result);
}
- @Test(expected = EntryNotFoundException.class)
- public void checkPutIfAbsentResultThrowsIfEventHasRetriedButResultNotHaveSameValue() {
- Object result = new Object();
- when(event.hasRetried()).thenReturn(true);
- when(region.putIfAbsentResultHasSameValue(true, value, result)).thenReturn(false);
- doCallRealMethod().when(region).checkPutIfAbsentResult(event, value, result);
-
- region.checkPutIfAbsentResult(event, value, result);
- }
-
- @Test
- public void checkPutIfAbsentResultSucceedsIfEventHasRetriedAndResultHasSameValue() {
- Object result = new Object();
- when(event.hasRetried()).thenReturn(true);
- when(region.putIfAbsentResultHasSameValue(true, value, result)).thenReturn(true);
- doCallRealMethod().when(region).checkPutIfAbsentResult(event, value, result);
-
- region.checkPutIfAbsentResult(event, value, result);
-
- verify(event).hasRetried();
- verify(region).putIfAbsentResultHasSameValue(true, value, result);
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnTrueIfResultIsInvalidTokenAndValueToBePutIsNull() {
- when(region.putIfAbsentResultHasSameValue(true, null, Token.INVALID)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(true, null, Token.INVALID)).isTrue();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnFalseIfResultIsInvalidTokenAndValueToBePutIsNotNull() {
- when(region.putIfAbsentResultHasSameValue(true, value, Token.INVALID)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(true, value, Token.INVALID)).isFalse();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnTrueIfResultHasSameValue() {
- Object result = "value";
- when(region.putIfAbsentResultHasSameValue(true, value, result)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(true, value, result)).isTrue();
- verify(region, never()).getAttributes();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnFalseIfResultDoesNotHaveSameValue() {
- Object result = "differentValue";
- when(region.putIfAbsentResultHasSameValue(true, value, result)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(true, value, result)).isFalse();
- verify(region, never()).getAttributes();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueChecksRegionAttributesIfNotFromClient() {
- Object result = "value";
- when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnFalseIfResultDoesNotHaveSameValueAndNotFromClient() {
- Object oldValue = "differentValue";
- Object result = new VMCachedDeserializable(EntryEventImpl.serialize(oldValue));
- when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isFalse();
- verify(region).getAttributes();
- }
-
- @Test
- public void putIfAbsentResultHasSameValueReturnTrueIfResultHasSameValueAndNotFromClient() {
- Object result = new VMCachedDeserializable(EntryEventImpl.serialize(value));
- when(region.putIfAbsentResultHasSameValue(false, value, result)).thenCallRealMethod();
-
- assertThat(region.putIfAbsentResultHasSameValue(false, value, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void bridgePutIfAbsentResultHasSameValueCanCheckValueForObject() {
- Object result = "value";
- byte[] valueToBePut = EntryEventImpl.serialize(value);
- when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
- .thenCallRealMethod();
-
- assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void bridgePutIfAbsentResultHasSameValueCanCheckValueForNonObjectByteArray() {
- byte[] valueToBePut = {0, 1, 2, 3};
- Object result = new VMCachedDeserializable(EntryEventImpl.serialize(valueToBePut));
- when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, false, result))
- .thenCallRealMethod();
-
- assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, false, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void bridgePutIfAbsentResultHasSameValueCanCheckValueForIntArray() {
- int[] newValue = {0, 1, 2, 3};
- byte[] valueToBePut = EntryEventImpl.serialize(newValue);
- Object result = newValue;
- when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
- .thenCallRealMethod();
-
- assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void bridgePutIfAbsentResultHasSameValueCanCheckValueForArrayOfArray() {
- String[] array1 = {"0", "1", "2"};
- String[] array2 = {"3", "4", "5"};
- String[] array3 = {"7"};
- String[][] newValue = {array1, array2, array3};
- byte[] valueToBePut = EntryEventImpl.serialize(newValue);
- Object result = new VMCachedDeserializable(EntryEventImpl.serialize(newValue));
- when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
- .thenCallRealMethod();
-
- assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isTrue();
- verify(region).getAttributes();
- }
-
- @Test
- public void bridgePutIfAbsentResultHasSameValueCanCheckDifferentValuesForArrayOfArray() {
- String[] array1 = {"0", "1", "2"};
- String[] array2 = {"3", "4", "5"};
- String[] array3 = {"7"};
- String[] array4 = {"8"};
- String[][] newValue = {array1, array2, array3};
- String[][] returnedValue = {array1, array2, array4};
- byte[] valueToBePut = EntryEventImpl.serialize(newValue);
- Object result = new VMCachedDeserializable(EntryEventImpl.serialize(returnedValue));
- when(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result))
- .thenCallRealMethod();
-
- assertThat(region.bridgePutIfAbsentResultHasSameValue(valueToBePut, true, result)).isFalse();
- verify(region).getAttributes();
- }
-
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
index 0aab7c5..1e76598 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/map/RegionMapPutTest.java
@@ -134,6 +134,27 @@ public class RegionMapPutTest {
}
@Test
+ public void doesNotSetEventOldValueIfRetriedPutIfAbsentOperation() {
+ final byte[] bytes = new byte[] {1, 2, 3, 4, 5};
+ givenExistingRegionEntry();
+ when(existingRegionEntry.getValue()).thenReturn(bytes);
+ when(internalRegion.getConcurrencyChecksEnabled()).thenReturn(true);
+ givenPutIfAbsentOperation(bytes); // duplicate operation
+ doPut();
+ verify(event).setOldValue(null, true);
+ assertThat(instance.isOverwritePutIfAbsent()).isTrue();
+ }
+
+ private void givenPutIfAbsentOperation(byte[] bytes) {
+ when(event.isPossibleDuplicate()).thenReturn(true);
+ when(event.basicGetNewValue()).thenReturn(bytes);
+ when(event.getOperation()).thenReturn(Operation.PUT_IF_ABSENT);
+ when(event.hasValidVersionTag()).thenReturn(false);
+ ifNew = true;
+ ifOld = false;
+ }
+
+ @Test
public void setsEventOldValueToExistingRegionEntryValue_ifOperationGuaranteesOldValue() {
givenExistingRegionEntry();
givenAnOperationThatGuaranteesOldValue();