geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [geode] branch develop updated: Feature/GEODE 3996 (#1073)
Date Mon, 27 Nov 2017 19:51:41 GMT
This is an automated email from the ASF dual-hosted git repository.

dschneider 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 9221905b Feature/GEODE 3996 (#1073)
9221905b is described below

commit 9221905bbf3343e18d5514e14c3dc7f9ba5d0eae
Author: Darrel Schneider <dschneider@pivotal.io>
AuthorDate: Mon Nov 27 11:51:39 2017 -0800

    Feature/GEODE 3996 (#1073)
    
    GEODE-3996: change CacheWriter getOldValue to return value
    
    If the old value is overflowed to disk and a CacheWriter calls getOldValue
    it will now return the value overflowed to disk instead of null (the old behavior).
    
    Serialization of EntryEventImpl now correctly handles old value that is NOT_AVAILABLE.
---
 .../geode/internal/cache/AbstractRegionMap.java    |   8 +-
 .../geode/internal/cache/EntryEventImpl.java       | 131 ++++++----
 .../apache/geode/internal/cache/LocalRegion.java   |   4 +
 .../cache/SearchLoadAndWriteProcessor.java         |  13 +
 .../cache/entries/AbstractRegionEntry.java         |  15 +-
 .../CacheWriterGetOldValueIntegrationTest.java     | 286 +++++++++++++++++++++
 .../geode/internal/cache/EntryEventImplTest.java   | 125 +++++++++
 .../codeAnalysis/sanctionedDataSerializables.txt   |   2 +-
 8 files changed, 532 insertions(+), 52 deletions(-)

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 da0cf59..25b0df0 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
@@ -2969,7 +2969,7 @@ public abstract class AbstractRegionMap implements RegionMap {
         Object oldValueInVMOrDisk = re.getValueOffHeapOrDiskWithoutFaultIn(event.getLocalRegion());
         ReferenceCountHelper.unskipRefCountTracking();
         try {
-          event.setOldValue(oldValueInVMOrDisk, needToSetOldValue);
+          event.setOldValue(oldValueInVMOrDisk, true);
         } finally {
           OffHeapHelper.releaseWithNoTracking(oldValueInVMOrDisk);
         }
@@ -2982,10 +2982,12 @@ public abstract class AbstractRegionMap implements RegionMap {
         Object oldValueInVM = re.getValueRetain(event.getLocalRegion(), true); // OFFHEAP:
re
                                                                                // synced
so can use
                                                                                // its ref.
-
+        if (oldValueInVM == null) {
+          oldValueInVM = Token.NOT_AVAILABLE;
+        }
         ReferenceCountHelper.unskipRefCountTracking();
         try {
-          event.setOldValue(oldValueInVM, needToSetOldValue);
+          event.setOldValue(oldValueInVM);
         } finally {
           OffHeapHelper.releaseWithNoTracking(oldValueInVM);
         }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
index 533eb32..0ea0c87 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
@@ -728,14 +728,9 @@ public class EntryEventImpl
         return null;
       }
       @Unretained
-      Object ov = basicGetOldValue();
-      if (ov == null) {
-        return null;
-      } else if (ov == Token.NOT_AVAILABLE) {
-        return AbstractRegion.handleNotAvailable(ov);
-      }
-      boolean doCopyOnRead = getRegion().isCopyOnRead();
+      Object ov = handleNotAvailableOldValue();
       if (ov != null) {
+        boolean doCopyOnRead = getRegion().isCopyOnRead();
         if (ov instanceof CachedDeserializable) {
           return callWithOffHeapLock((CachedDeserializable) ov, oldValueCD -> {
             if (doCopyOnRead) {
@@ -762,6 +757,44 @@ public class EntryEventImpl
   }
 
   /**
+   * returns the old value after handling one this is NOT_AVAILABLE. If the old value is
+   * NOT_AVAILABLE then it may try to read it from disk. If it can't read an unavailable
old value
+   * from disk then it will return null instead of NOT_AVAILABLE.
+   */
+  @Unretained(ENTRY_EVENT_OLD_VALUE)
+  private Object handleNotAvailableOldValue() {
+    @Unretained
+    Object result = basicGetOldValue();
+    if (result != Token.NOT_AVAILABLE) {
+      return result;
+    }
+    if (getReadOldValueFromDisk()) {
+      try {
+        result = this.region.getValueInVMOrDiskWithoutFaultIn(getKey());
+      } catch (EntryNotFoundException ex) {
+        result = null;
+      }
+    }
+    result = AbstractRegion.handleNotAvailable(result);
+    return result;
+  }
+
+  /**
+   * If true then when getOldValue is called if the NOT_AVAILABLE is found then an attempt
will be
+   * made to read the old value from disk without faulting it in. Should only be set to true
when
+   * product is calling a method on a CacheWriter.
+   */
+  private boolean readOldValueFromDisk;
+
+  public boolean getReadOldValueFromDisk() {
+    return this.readOldValueFromDisk;
+  }
+
+  public void setReadOldValueFromDisk(boolean v) {
+    this.readOldValueFromDisk = v;
+  }
+
+  /**
    * Like getRawNewValue except that if the result is an off-heap reference then copy it
to the
    * heap. Note: to prevent the heap copy use getRawNewValue instead
    */
@@ -858,11 +891,12 @@ public class EntryEventImpl
    * @param v the caller should have already retained this off-heap reference.
    */
   @Released(ENTRY_EVENT_OLD_VALUE)
-  private void basicSetOldValue(@Unretained(ENTRY_EVENT_OLD_VALUE) Object v) {
+  void basicSetOldValue(@Unretained(ENTRY_EVENT_OLD_VALUE) Object v) {
     @Released
     final Object curOldValue = this.oldValue;
-    if (v == curOldValue)
+    if (v == curOldValue) {
       return;
+    }
     if (this.offHeapOk && mayHaveOffHeapReferences()) {
       if (ReferenceCountHelper.trackReferenceCounts()) {
         OffHeapHelper.releaseAndTrackOwner(curOldValue, new OldValueOwner());
@@ -876,9 +910,9 @@ public class EntryEventImpl
 
   @Released(ENTRY_EVENT_OLD_VALUE)
   private void retainAndSetOldValue(@Retained(ENTRY_EVENT_OLD_VALUE) Object v) {
-    if (v == this.oldValue)
+    if (v == this.oldValue) {
       return;
-
+    }
     if (isOffHeapReference(v)) {
       StoredObject so = (StoredObject) v;
       if (ReferenceCountHelper.trackReferenceCounts()) {
@@ -900,7 +934,7 @@ public class EntryEventImpl
   }
 
   @Unretained(ENTRY_EVENT_OLD_VALUE)
-  private Object basicGetOldValue() {
+  Object basicGetOldValue() {
     @Unretained(ENTRY_EVENT_OLD_VALUE)
     Object result = this.oldValue;
     if (!this.offHeapOk && isOffHeapReference(result)) {
@@ -1478,6 +1512,9 @@ public class EntryEventImpl
   private static final boolean EVENT_OLD_VALUE =
       !Boolean.getBoolean(DistributionConfig.GEMFIRE_PREFIX + "disable-event-old-value");
 
+  protected boolean areOldValuesEnabled() {
+    return EVENT_OLD_VALUE;
+  }
 
   void putExistingEntry(final LocalRegion owner, RegionEntry entry) throws RegionClearedException
{
     putExistingEntry(owner, entry, false, null);
@@ -1495,8 +1532,9 @@ public class EntryEventImpl
     // only set oldValue if it hasn't already been set to something
     if (this.oldValue == null) {
       if (!reentry.isInvalidOrRemoved()) {
-        if (requireOldValue || EVENT_OLD_VALUE || this.region instanceof HARegion // fix
for bug
-                                                                                  // 37909
+        if (requireOldValue || areOldValuesEnabled() || this.region instanceof HARegion //
fix for
+                                                                                        //
bug
+        // 37909
         ) {
           @Retained
           Object ov;
@@ -1764,7 +1802,7 @@ public class EntryEventImpl
     if (Token.isInvalidOrRemoved(oldVal)) {
       oldVal = null;
     } else {
-      if (mustBeAvailable || oldVal == null || EVENT_OLD_VALUE) {
+      if (mustBeAvailable || oldVal == null || areOldValuesEnabled()) {
         // set oldValue to oldVal
       } else {
         oldVal = Token.NOT_AVAILABLE;
@@ -1797,24 +1835,26 @@ public class EntryEventImpl
     tx.setCallbackArgument(getCallbackArgument());
   }
 
-  /** @return false if entry doesn't exist */
-  public boolean setOldValueFromRegion() {
+  public void setOldValueFromRegion() {
     try {
       RegionEntry re = this.region.getRegionEntry(getKey());
-      if (re == null)
-        return false;
+      if (re == null) {
+        return;
+      }
       ReferenceCountHelper.skipRefCountTracking();
       Object v = re.getValueRetain(this.region, true);
+      if (v == null) {
+        v = Token.NOT_AVAILABLE;
+      }
       ReferenceCountHelper.unskipRefCountTracking();
       try {
-        return setOldValue(v);
+        setOldValue(v);
       } finally {
         if (mayHaveOffHeapReferences()) {
           OffHeapHelper.releaseWithNoTracking(v);
         }
       }
     } catch (EntryNotFoundException ignore) {
-      return false;
     }
   }
 
@@ -1827,39 +1867,37 @@ public class EntryEventImpl
     basicSetOldValue(Token.DESTROYED);
   }
 
-  /**
-   * @return false if value 'v' indicates that entry does not exist
-   */
-  public boolean setOldValue(Object v) {
-    return setOldValue(v, false);
+  public void setOldValue(Object v) {
+    setOldValue(v, false);
   }
 
 
   /**
-   * @param force true if the old value should be forcibly set, used for HARegions, methods
like
-   *        putIfAbsent, etc., where the old value must be available.
-   * @return false if value 'v' indicates that entry does not exist
+   * @param force true if the old value should be forcibly set, methods like putIfAbsent,
etc.,
+   *        where the old value must be available.
    */
-  public boolean setOldValue(Object v, boolean force) {
-    if (v == null || Token.isRemoved(v)) {
-      return false;
-    } else {
-      if (Token.isInvalid(v)) {
+  public void setOldValue(Object v, boolean force) {
+    if (v != null) {
+      if (Token.isInvalidOrRemoved(v)) {
         v = null;
-      } else {
-        if (force || (this.region instanceof HARegion) // fix for bug 37909
-        ) {
-          // set oldValue to "v".
-        } else if (EVENT_OLD_VALUE) {
-          // TODO Rusty add compression support here
-          // set oldValue to "v".
-        } else {
-          v = Token.NOT_AVAILABLE;
-        }
+      } else if (shouldOldValueBeUnavailable(v, force)) {
+        v = Token.NOT_AVAILABLE;
       }
-      retainAndSetOldValue(v);
-      return true;
     }
+    retainAndSetOldValue(v);
+  }
+
+  private boolean shouldOldValueBeUnavailable(Object v, boolean force) {
+    if (force) {
+      return false;
+    }
+    if (areOldValuesEnabled()) {
+      return false;
+    }
+    if (this.region instanceof HARegion) {
+      return false;
+    }
+    return true;
   }
 
   /**
@@ -2143,6 +2181,7 @@ public class EntryEventImpl
           DataSerializer.writeObjectAsByteArray(cd.getValue(), out);
         }
       } else {
+        ov = AbstractRegion.handleNotAvailable(ov);
         DataSerializer.writeObject(ov, out);
       }
     }
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 bb2e0a6..3687d3f 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
@@ -2917,9 +2917,11 @@ public class LocalRegion extends AbstractRegion implements InternalRegion,
Loade
     if (writer != null && event.getOperation() != Operation.REMOVE
         && !event.inhibitAllNotifications()) {
       final long start = getCachePerfStats().startCacheWriterCall();
+      event.setReadOldValueFromDisk(true);
       try {
         writer.beforeDestroy(event);
       } finally {
+        event.setReadOldValueFromDisk(false);
         getCachePerfStats().endCacheWriterCall(start);
       }
       result = true;
@@ -3097,6 +3099,7 @@ public class LocalRegion extends AbstractRegion implements InternalRegion,
Loade
     if (!isPutIfAbsentOrReplace && localWriter != null && !event.inhibitAllNotifications())
{
       final long start = getCachePerfStats().startCacheWriterCall();
       final boolean newEntry = event.getOperation().isCreate();
+      event.setReadOldValueFromDisk(true);
       try {
         if (!newEntry) {
           localWriter.beforeUpdate(event);
@@ -3104,6 +3107,7 @@ public class LocalRegion extends AbstractRegion implements InternalRegion,
Loade
           localWriter.beforeCreate(event);
         }
       } finally {
+        event.setReadOldValueFromDisk(false);
         getCachePerfStats().endCacheWriterCall(start);
       }
     }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
index f7e8506..5195362 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
@@ -863,6 +863,9 @@ public class SearchLoadAndWriteProcessor implements MembershipListener
{
     if (event.getOperation().isCreate() && action == BEFOREUPDATE) {
       action = BEFORECREATE;
     }
+    if (event instanceof EntryEventImpl) {
+      ((EntryEventImpl) event).setReadOldValueFromDisk(true);
+    }
     try {
       switch (action) {
         case BEFORECREATE:
@@ -885,6 +888,9 @@ public class SearchLoadAndWriteProcessor implements MembershipListener
{
 
       }
     } finally {
+      if (event instanceof EntryEventImpl) {
+        ((EntryEventImpl) event).setReadOldValueFromDisk(false);
+      }
       if (event != pevent) {
         if (event instanceof EntryEventImpl) {
           ((Releasable) event).release();
@@ -2495,6 +2501,9 @@ public class SearchLoadAndWriteProcessor implements MembershipListener
{
           }
 
           if (writer != null) {
+            if (entryEvtImpl != null) {
+              entryEvtImpl.setReadOldValueFromDisk(true);
+            }
             try {
               switch (action) {
                 case BEFORECREATE:
@@ -2524,6 +2533,10 @@ public class SearchLoadAndWriteProcessor implements MembershipListener
{
             } catch (Exception e) {
               NetWriteReplyMessage.sendMessage(NetWriteRequestMessage.this.getSender(), processorId,
                   dm, false, e, false);
+            } finally {
+              if (entryEvtImpl != null) {
+                entryEvtImpl.setReadOldValueFromDisk(false);
+              }
             }
 
           } else {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
index 8cdea63..66677af 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
@@ -838,8 +838,9 @@ public abstract class AbstractRegionEntry implements RegionEntry, HashEntry<Obje
       if (inTokenMode && event.hasOldValue()) {
         proceed = true;
       } else {
-        proceed = event.setOldValue(curValue, curValue instanceof GatewaySenderEventImpl)
-            || removeRecoveredEntry || forceDestroy || region.getConcurrencyChecksEnabled()
+        event.setOldValue(curValue, curValue instanceof GatewaySenderEventImpl);
+        proceed = region.getConcurrencyChecksEnabled() || removeRecoveredEntry || forceDestroy
+            || destroyShouldProceedBasedOnCurrentValue(curValue)
             || (event.getOperation() == Operation.REMOVE && (curValue == null
                 || curValue == Token.LOCAL_INVALID || curValue == Token.INVALID));
       }
@@ -931,6 +932,16 @@ public abstract class AbstractRegionEntry implements RegionEntry, HashEntry<Obje
     }
   }
 
+  private static boolean destroyShouldProceedBasedOnCurrentValue(Object curValue) {
+    if (curValue == null) {
+      return false;
+    }
+    if (Token.isRemoved(curValue)) {
+      return false;
+    }
+    return true;
+  }
+
   public static boolean checkExpectedOldValue(@Unretained Object expectedOldValue,
       @Unretained Object actualValue, InternalRegion region) {
 
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/CacheWriterGetOldValueIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/CacheWriterGetOldValueIntegrationTest.java
new file mode 100644
index 0000000..60c51a2
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/CacheWriterGetOldValueIntegrationTest.java
@@ -0,0 +1,286 @@
+/*
+ * 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.internal.cache;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+
+import org.apache.logging.log4j.Logger;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.CacheWriterException;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.util.CacheWriterAdapter;
+import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class CacheWriterGetOldValueIntegrationTest {
+
+  private GemFireCacheImpl cache = null;
+
+  @Before
+  public void setUp() throws Exception {
+    createCache();
+  }
+
+  private void createCache() {
+    cache =
+        (GemFireCacheImpl) new CacheFactory().set("locators", "").set("mcast-port", "0").create();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    cache.close();
+    expectedValues.clear();
+  }
+
+  @Test
+  public void getOldValueInCacheWriterReturnsValueOfEvictedEntry() {
+    doTest(false);
+  }
+
+  @Test
+  public void getOldValueWithTransactionInCacheWriterReturnsValueOfEvictedEntry() {
+    doTest(true);
+  }
+
+  @Test
+  public void doPutAll() {
+    PutAllCacheWriter<String, String> cw;
+    Region<String, String> r = createOverflowRegion();
+    put(r, "k1", "v1");
+    put(r, "k2", "v2");
+    cw = new PutAllCacheWriter<>();
+    r.getAttributesMutator().setCacheWriter(cw);
+    HashMap<String, String> putAllMap = new HashMap<>();
+    putAllMap.put("k1", "update1");
+    putAllMap.put("k2", "update2");
+    r.putAll(putAllMap);
+
+    assertThat(cw.getSeenEntries()).isEqualTo(this.expectedValues);
+  }
+
+  @Test
+  public void doRemoveAll() {
+    RemoveAllCacheWriter<String, String> cw;
+    Region<String, String> r = createOverflowRegion();
+    put(r, "k1", "v1");
+    put(r, "k2", "v2");
+    cw = new RemoveAllCacheWriter<>();
+    r.getAttributesMutator().setCacheWriter(cw);
+    r.removeAll(Arrays.asList("k1", "k2"));
+
+    assertThat(cw.getSeenEntries()).isEqualTo(this.expectedValues);
+  }
+
+  private void doTest(boolean useTx) {
+    String evictedKey;
+    String unevictedKey;
+    String evictedValue;
+    String unevictedValue;
+    CacheWriterWithExpectedOldValue<String, String> cw;
+
+    Region<String, String> r = createOverflowRegion();
+    put(r, "k1", "v1");
+    put(r, "k2", "v2");
+
+    beginTx(useTx);
+    unevictedKey = getUnevictedKey(r);
+    unevictedValue = expectedValues.get(unevictedKey);
+    cw = new CacheWriterWithExpectedOldValue<>(unevictedValue);
+    r.getAttributesMutator().setCacheWriter(cw);
+    assertThat(put(r, unevictedKey, "update1")).isEqualTo(unevictedValue);
+    assertThat(cw.getUnexpectedEvents()).isEmpty();
+    endTx(useTx);
+
+    beginTx(useTx);
+    evictedKey = getEvictedKey(r);
+    evictedValue = expectedValues.get(evictedKey);
+    cw = new CacheWriterWithExpectedOldValue<>(evictedValue);
+    r.getAttributesMutator().setCacheWriter(cw);
+    assertThat(put(r, evictedKey, "update2")).isEqualTo(useTx ? evictedValue : null);
+    assertThat(cw.getUnexpectedEvents()).isEmpty();
+    endTx(useTx);
+
+    beginTx(useTx);
+    evictedKey = getEvictedKey(r);
+    evictedValue = expectedValues.get(evictedKey);
+    cw = new CacheWriterWithExpectedOldValue<>(evictedValue);
+    r.getAttributesMutator().setCacheWriter(cw);
+    assertThat(r.destroy(evictedKey)).isEqualTo(useTx ? evictedValue : null);
+    assertThat(cw.getUnexpectedEvents()).isEmpty();
+    endTx(useTx);
+  }
+
+  private void beginTx(boolean useTx) {
+    if (useTx) {
+      this.cache.getCacheTransactionManager().begin();
+    }
+  }
+
+  private void endTx(boolean useTx) {
+    if (useTx) {
+      this.cache.getCacheTransactionManager().commit();
+    }
+  }
+
+  private final HashMap<String, String> expectedValues = new HashMap<>();
+
+  private String put(Region<String, String> r, String k, String v) {
+    String result = r.put(k, v);
+    expectedValues.put(k, v);
+    return result;
+  }
+
+  private String getEvictedKey(Region<String, String> r) {
+    InternalRegion<String, String> ir = (InternalRegion<String, String>) r;
+    String evictedKey = null;
+    RegionEntry re = ir.getRegionEntry("k1");
+    if (re.getValueAsToken() == null) {
+      evictedKey = "k1";
+    }
+    re = ir.getRegionEntry("k2");
+    if (re.getValueAsToken() == null) {
+      evictedKey = "k2";
+    }
+    assertThat(evictedKey).isNotNull();
+    return evictedKey;
+  }
+
+  private String getUnevictedKey(Region<String, String> r) {
+    InternalRegion<String, String> ir = (InternalRegion<String, String>) r;
+    String unevictedKey = null;
+    RegionEntry re = ir.getRegionEntry("k1");
+    if (re.getValueAsToken() != null) {
+      unevictedKey = "k1";
+    }
+    re = ir.getRegionEntry("k2");
+    if (re.getValueAsToken() != null) {
+      unevictedKey = "k2";
+    }
+    assertThat(unevictedKey).isNotNull();
+    return unevictedKey;
+  }
+
+  private static class CacheWriterWithExpectedOldValue<K, V> extends CacheWriterAdapter<K,
V> {
+    private final V expectedOldValue;
+    private final ArrayList<EntryEvent<K, V>> unexpectedEvents = new ArrayList<>();
+
+    CacheWriterWithExpectedOldValue(V expectedOldValue) {
+      this.expectedOldValue = expectedOldValue;
+    }
+
+    public List<EntryEvent<K, V>> getUnexpectedEvents() {
+      return this.unexpectedEvents;
+    }
+
+    private void checkEvent(EntryEvent<K, V> event) {
+      if (this.expectedOldValue == null) {
+        if (event.getOldValue() != null) {
+          this.unexpectedEvents.add(event);
+        }
+      } else {
+        if (!this.expectedOldValue.equals(event.getOldValue())) {
+          this.unexpectedEvents.add(event);
+        }
+      }
+    }
+
+    @Override
+    public void beforeCreate(EntryEvent<K, V> event) throws CacheWriterException {
+      checkEvent(event);
+    }
+
+    @Override
+    public void beforeUpdate(EntryEvent<K, V> event) throws CacheWriterException {
+      checkEvent(event);
+    }
+
+    @Override
+    public void beforeDestroy(EntryEvent<K, V> event) throws CacheWriterException {
+      checkEvent(event);
+    }
+  }
+
+  private static class PutAllCacheWriter<K, V> extends CacheWriterAdapter<K, V>
{
+    private final HashMap<K, V> seenEntries = new HashMap<>();
+
+    public HashMap<K, V> getSeenEntries() {
+      return this.seenEntries;
+    }
+
+    @Override
+    public void beforeCreate(EntryEvent<K, V> event) throws CacheWriterException {
+      fail("did not expect beforeCreate to be called by putAll");
+    }
+
+    @Override
+    public void beforeUpdate(EntryEvent<K, V> event) throws CacheWriterException {
+      seenEntries.put(event.getKey(), event.getOldValue());
+    }
+
+    @Override
+    public void beforeDestroy(EntryEvent<K, V> event) throws CacheWriterException {
+      fail("did not expect beforeDestroy to be called by putAll");
+    }
+  }
+
+  private static class RemoveAllCacheWriter<K, V> extends CacheWriterAdapter<K,
V> {
+    private final HashMap<K, V> seenEntries = new HashMap<>();
+
+    public HashMap<K, V> getSeenEntries() {
+      return this.seenEntries;
+    }
+
+    @Override
+    public void beforeCreate(EntryEvent<K, V> event) throws CacheWriterException {
+      fail("did not expect beforeCreate to be called by removeAll");
+    }
+
+    @Override
+    public void beforeUpdate(EntryEvent<K, V> event) throws CacheWriterException {
+      fail("did not expect beforeUpdate to be called by removeAll");
+    }
+
+    @Override
+    public void beforeDestroy(EntryEvent<K, V> event) throws CacheWriterException {
+      seenEntries.put(event.getKey(), event.getOldValue());
+    }
+  }
+
+  private <K, V> Region<K, V> createOverflowRegion() {
+    RegionFactory<K, V> regionFactory = cache.createRegionFactory();
+    regionFactory.setEvictionAttributes(
+        EvictionAttributes.createLRUEntryAttributes(1, EvictionAction.OVERFLOW_TO_DISK));
+    regionFactory.setDataPolicy(DataPolicy.NORMAL);
+    return regionFactory.create("region");
+  }
+
+}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/EntryEventImplTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/EntryEventImplTest.java
index 2ad9d02..eb3e804 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/EntryEventImplTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/EntryEventImplTest.java
@@ -380,6 +380,124 @@ public class EntryEventImplTest {
   }
 
   @Test
+  public void setOldValueUnforcedWithRemoveTokenChangesOldValueToNull() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.REMOVED_PHASE1, false);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedWithRemoveTokenChangesOldValueToNull() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.REMOVED_PHASE1, true);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueUnforcedWithInvalidTokenNullsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.INVALID, false);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedWithInvalidTokenNullsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.INVALID, true);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueUnforcedWithNullChangesOldValueToNull() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(null, false);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedWithNullChangesOldValueToNull() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(null, true);
+    assertEquals(null, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedWithNotAvailableTokenSetsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.NOT_AVAILABLE, true);
+    assertEquals(Token.NOT_AVAILABLE, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueUnforcedWithNotAvailableTokenSetsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue(Token.NOT_AVAILABLE, false);
+    assertEquals(Token.NOT_AVAILABLE, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldUnforcedValueSetsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue("oldValue", false);
+    assertEquals("oldValue", e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedSetsOldValue() {
+    LocalRegion region = mock(LocalRegion.class);
+    EntryEventImpl e = createEntryEvent(region, null);
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue("oldValue", true);
+    assertEquals("oldValue", e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueUnforcedWithDisabledSetsNotAvailable() {
+    EntryEventImpl e = new EntryEventImplWithOldValuesDisabled();
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue("oldValue", false);
+    assertEquals(Token.NOT_AVAILABLE, e.basicGetOldValue());
+  }
+
+  @Test
+  public void setOldValueForcedWithDisabledSetsOldValue() {
+    EntryEventImpl e = new EntryEventImplWithOldValuesDisabled();
+    String UNINITIALIZED = "Uninitialized";
+    e.basicSetOldValue(UNINITIALIZED);
+    e.setOldValue("oldValue", true);
+    assertEquals("oldValue", e.basicGetOldValue());
+  }
+
+  @Test
   public void verifyExternalReadMethodsBlockedByRelease() throws InterruptedException {
     LocalRegion region = mock(LocalRegion.class);
     when(region.getOffHeap()).thenReturn(true);
@@ -539,6 +657,13 @@ public class EntryEventImplTest {
     }
   }
 
+  private static class EntryEventImplWithOldValuesDisabled extends EntryEventImpl {
+    @Override
+    protected boolean areOldValuesEnabled() {
+      return false;
+    }
+  }
+
   private static class TestableEntryEventImpl extends EntryEventImpl {
     private final CountDownLatch releaseCountDown;
     private volatile boolean waitingOnRelease = false;
diff --git a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
index d3b1207..c7b0f3f 100644
--- a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
+++ b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
@@ -990,7 +990,7 @@ toData,17,2ab400022bb800042ab400032bb80004b1
 
 org/apache/geode/internal/cache/EntryEventImpl,2
 fromData,216,2a2bb80016c00017b500182bb800164d2bb800164e2abb0019592c2d01b7001ab5001b2a2bb9001c0100b8001db5001e2a2bb9001f0100b500082ab4001b2bb80016b600202a2bb80016c00021b500092bb900220100990013b200239a003cbb0024591225b70026bf2bb9002201009900212a2bb80027b500282a2ab40028b500062a2ab40028b80029b50005a7000b2a2bb80016b500052bb9002201009900192a2bb80027b5002a2a2ab4002ab80029b50007a7000b2a2bb80016b500072a2bb8002bb5002c2a2bb8002db5000a2a2bb8002eb50013b1
-toData,279,2ab400182bb801472ab600892bb801472ab4001bb601742bb801472b2ab4001eb40175b9017602002b2ab4000811c03f7eb9017702002ab600492bb801472ab400092bb801472b03b9017802002ab6003f4d2cc100803e1d99000d2cc00080b900a901003e2b1db9017802001d99003b2ab40028c6000e2ab400282bb80179a7002e2ab40006c6000e2ab400062bb80179a7001c2cc000803a041904b900b701002bb8017aa700082c2bb801472ab700414d2cc100803e1d99000d2cc00080b900a901003e2b1db9017802001d9900292ab4002ac6000e2ab4002a2bb80179a7001c2cc000803a041904b900b701002bb
[...]
+toData,284,2ab400182bb8014d2ab600882bb8014d2ab4001bb6017a2bb8014d2b2ab4001eb4017bb9017c02002b2ab4000811c03f7eb9017d02002ab600492bb8014d2ab400092bb8014d2b03b9017e02002ab6003f4d2cc1007f3e1d99000d2cc0007fb900ae01003e2b1db9017e02001d99003b2ab40028c6000e2ab400282bb8017fa7002e2ab40006c6000e2ab400062bb8017fa7001c2cc0007f3a041904b900bc01002bb80180a700082c2bb8014d2ab600414d2cc1007f3e1d99000d2cc0007fb900ae01003e2b1db9017e02001d9900292ab4002ac6000e2ab4002a2bb8017fa700212cc0007f3a041904b900bc01002bb
[...]
 
 org/apache/geode/internal/cache/EntrySnapshot,2
 fromData,50,2a03b500052bb9004201003d1c9900112abb000759b70043b50004a7000e2abb000359b70044b500042ab400042bb60045b1

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <commits@geode.apache.org>'].

Mime
View raw message