This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.15 by this push:
new 34e15feb97 GEODE-10294: Compare invalid token during putIfAbsent
retry. (#7738)
34e15feb97 is described below
commit 34e15feb97b0aa932dae6c41e5e8a01332522db1
Author: Eric Shu <[email protected]>
AuthorDate: Wed Jun 1 11:47:59 2022 -0700
GEODE-10294: Compare invalid token during putIfAbsent retry. (#7738)
* During putIfAbsent retry, comparing invalid token value when
putIfAbsent of a null value.
* Do not make putIfAbsent event to update event if current
entry value is null or invalidate and is a retried event.
(cherry picked from commit ea48e7f7c0e8f272957377030ab28f7f87da8eac)
---
.../cache/RetryPutIfAbsentIntegrationTest.java | 99 +++++++++++++---------
.../geode/internal/cache/map/RegionMapPut.java | 20 ++++-
.../geode/internal/cache/map/RegionMapPutTest.java | 17 +++-
3 files changed, 88 insertions(+), 48 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
index f79c2d2972..255eee9401 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/cache/RetryPutIfAbsentIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/cache/RetryPutIfAbsentIntegrationTest.java
@@ -14,17 +14,14 @@
*/
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 static org.assertj.core.api.Assertions.assertThat;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.apache.geode.cache.query.CacheUtils;
+import org.apache.geode.cache.util.CacheListenerAdapter;
import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.internal.cache.EventID;
import org.apache.geode.internal.cache.EventIDHolder;
@@ -32,55 +29,75 @@ 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 {
+class RetryPutIfAbsentIntegrationTest {
- Cache cache;
+ private Cache cache;
+ private LocalRegion myRegion;
+ private final ClientProxyMembershipID id =
+ new ClientProxyMembershipID(new InternalDistributedMember("localhost",
1));
+ private final EventIDHolder clientEvent =
+ new EventIDHolder(new EventID(new byte[] {1, 2, 3, 4, 5}, 1, 1));
+ private final String key = "key";
+ private int updateCount;
- @Rule
- public ExpectedException expectedException = ExpectedException.none();
+ @Test
+ void duplicatePutIfAbsentIsAccepted() {
+ myRegion = (LocalRegion)
cache.createRegionFactory(RegionShortcut.REPLICATE)
+ .setConcurrencyChecksEnabled(true).create("myRegion");
+
+ clientEvent.setRegion(myRegion);
+ byte[] valueBytes = new VMCachedDeserializable("myValue",
7).getSerializedValue();
+ Object oldValue =
+ myRegion.basicBridgePutIfAbsent(key, valueBytes, true, null, id, true,
clientEvent);
+ assertThat(oldValue).isNull();
+ assertThat(myRegion.containsKey(key)).isTrue();
+
+ myRegion.getEventTracker().clear();
+
+ clientEvent.setPossibleDuplicate(true);
+ clientEvent.setOperation(Operation.PUT_IF_ABSENT);
+ assertThat(myRegion.getEventTracker().hasSeenEvent(clientEvent)).isFalse();
+
+ oldValue = myRegion.basicBridgePutIfAbsent(key, valueBytes, true, null,
id, true, clientEvent);
+ assertThat(oldValue).isNull();
+ }
@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");
+ void duplicatePutIfAbsentOfNullValueDoesNotInvokeAfterUpdateListener() {
+ myRegion = (LocalRegion)
cache.createRegionFactory(RegionShortcut.REPLICATE)
+ .setConcurrencyChecksEnabled(true)
+ .addCacheListener(new CacheListenerAdapter<Object, Object>() {
+ @Override
+ public void afterUpdate(EntryEvent<Object, Object> event) {
+ ++updateCount;
+ }
+ }).create("myRegion");
+
+ clientEvent.setRegion(myRegion);
Object oldValue =
- myregion.basicBridgePutIfAbsent("mykey", valueBytes, true, null, id,
true, clientEvent);
- assertEquals(null, oldValue);
- assertTrue(myregion.containsKey(key));
+ myRegion.basicBridgePutIfAbsent(key, null, true, null, id, true,
clientEvent);
+ assertThat(oldValue).isNull();
+ assertThat(myRegion.containsKey(key)).isTrue();
- myregion.getEventTracker().clear();
+ 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));
+ assertThat(myRegion.getEventTracker().hasSeenEvent(clientEvent)).isFalse();
- System.out.println("second putIfAbsent");
- oldValue =
- myregion.basicBridgePutIfAbsent("mykey", valueBytes, true, null, id,
true, clientEvent);
- assertEquals(null, oldValue);
+ oldValue = myRegion.basicBridgePutIfAbsent(key, null, true, null, id,
true, clientEvent);
+ assertThat(oldValue).isNull();
+ assertThat(updateCount).isEqualTo(0);
}
- @Before
- public void setUp() throws Exception {
+ @BeforeEach
+ void setUp() {
CacheUtils.startCache();
cache = CacheUtils.getCache();
}
- @After
- public void tearDown() throws Exception {
+ @AfterEach
+ void tearDown() {
CacheUtils.closeCache();
}
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 e0b62a5976..545d35885f 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
@@ -235,7 +235,8 @@ public class RegionMapPut extends AbstractRegionMapPut {
final EntryEventImpl event = getEvent();
if (getOwner().isInitialized() && isCacheWrite()) {
if (!isReplaceOnClient()) {
- if (getRegionEntry().isDestroyedOrRemoved()) {
+ if (getRegionEntry().isDestroyedOrRemoved()
+ || (getRegionEntry().isInvalid() && event.getOperation() ==
Operation.PUT_IF_ABSENT)) {
event.makeCreate();
} else {
event.makeUpdate();
@@ -343,6 +344,9 @@ public class RegionMapPut extends AbstractRegionMapPut {
if (isReplaceOnClient()) {
return true;
}
+ if (getRegionEntry().isInvalid() && getEvent().getOperation() ==
Operation.PUT_IF_ABSENT) {
+ return false;
+ }
return !getRegionEntry().isRemoved();
}
@@ -409,9 +413,7 @@ public class RegionMapPut extends AbstractRegionMapPut {
event.isPossibleDuplicate()) {
Object retainedValue = getRegionEntry().getValueRetain(getOwner());
try {
- if (ValueComparisonHelper.checkEquals(retainedValue,
- getEvent().getRawNewValue(),
- isCompressedOffHeap(event), getOwner().getCache())) {
+ if (isSameValueAlreadyInCacheForPutIfAbsent(retainedValue)) {
if (logger.isDebugEnabled()) {
logger.debug("retried putIfAbsent found same value already in
cache "
+ "- allowing the operation. entry={}; event={}",
getRegionEntry(),
@@ -430,6 +432,16 @@ public class RegionMapPut extends AbstractRegionMapPut {
return true;
}
+ private boolean isSameValueAlreadyInCacheForPutIfAbsent(Object
retainedValue) {
+ Object rawNewValue = getEvent().getRawNewValue();
+ if (Token.isInvalid(retainedValue)) {
+ return rawNewValue == null || Token.isInvalid(rawNewValue);
+ }
+
+ return ValueComparisonHelper.checkEquals(retainedValue, rawNewValue,
+ isCompressedOffHeap(getEvent()), getOwner().getCache());
+ }
+
private boolean isCompressedOffHeap(EntryEventImpl event) {
return event.getRegion().getAttributes().getOffHeap()
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 7fceaeb43a..8b3d77e062 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
@@ -137,7 +137,7 @@ public class RegionMapPutTest {
public void doesNotSetEventOldValueIfRetriedPutIfAbsentOperation() {
final byte[] bytes = new byte[] {1, 2, 3, 4, 5};
givenExistingRegionEntry();
- when(existingRegionEntry.getValue()).thenReturn(bytes);
+ when(existingRegionEntry.getValueRetain(internalRegion)).thenReturn(bytes);
when(internalRegion.getConcurrencyChecksEnabled()).thenReturn(true);
givenPutIfAbsentOperation(bytes); // duplicate operation
doPut();
@@ -145,11 +145,22 @@ public class RegionMapPutTest {
assertThat(instance.isOverwritePutIfAbsent()).isTrue();
}
+ @Test
+ public void doesNotSetEventOldValueIfRetriedPutIfAbsentOperationOfNull() {
+ givenExistingRegionEntry();
+
when(existingRegionEntry.getValueRetain(internalRegion)).thenReturn(Token.INVALID);
+ when(internalRegion.getConcurrencyChecksEnabled()).thenReturn(true);
+ givenPutIfAbsentOperation(null); // duplicate operation
+ doPut();
+ verify(event).setOldValue(null, true);
+ assertThat(instance.isOverwritePutIfAbsent()).isTrue();
+ }
+
@Test
public void
overWritePutIfAbsentIsTrueIfRetriedPutIfAbsentOperationHavingValidVersionTag() {
final byte[] bytes = new byte[] {1, 2, 3, 4, 5};
givenExistingRegionEntry();
- when(existingRegionEntry.getValue()).thenReturn(bytes);
+ when(existingRegionEntry.getValueRetain(internalRegion)).thenReturn(bytes);
when(internalRegion.getConcurrencyChecksEnabled()).thenReturn(true);
givenPutIfAbsentOperation(bytes); // duplicate operation
when(event.hasValidVersionTag()).thenReturn(true);
@@ -162,7 +173,7 @@ public class RegionMapPutTest {
private void givenPutIfAbsentOperation(byte[] bytes) {
when(event.isPossibleDuplicate()).thenReturn(true);
- when(event.basicGetNewValue()).thenReturn(bytes);
+ when(event.getRawNewValue()).thenReturn(bytes);
when(event.getOperation()).thenReturn(Operation.PUT_IF_ABSENT);
when(event.hasValidVersionTag()).thenReturn(false);
ifNew = true;
