GEODE-10294: Compare invalid token during putIfAbsent retry. (apache#…

…7679) * During putIfAbsent retry, comparing invalid token value when putIfAbsent of a null value instead.
demery-pivotal · May 13, 2022 · 4f4af2a · 4f4af2a
1 parent 20844a8
commit 4f4af2a
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 6 deletions.
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
@@ -409,9 +409,7 @@ private boolean checkCreatePreconditions() {
             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 +428,16 @@ private boolean checkCreatePreconditions() {
     return true;
   }
 
+  private boolean isSameValueAlreadyInCacheForPutIfAbsent(Object retainedValue) {
+    if (Token.isInvalid(retainedValue)) {
+      return getEvent().getRawNewValue() == null || Token.isInvalid(getEvent().getRawNewValue());
+    }
+
+    return ValueComparisonHelper.checkEquals(retainedValue,
+        getEvent().getRawNewValue(),
+        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
@@ -137,19 +137,30 @@ public void setsEventOldValueToExistingRegionEntryValue_ifOldValueIsGatewaySende
   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();
     verify(event).setOldValue(null, true);
     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 void overWritePutIfAbsentIsTrueIfRetriedPutIfAbsentOperationHavingValidVe
 
   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;