incubator-geode git commit: GEODE-2043: change makeTombstone to handle exception

[ds...@apache.org]

Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-2043 [created] f2dd46182


GEODE-2043: change makeTombstone to handle exception

Now if makeTombstone has an exception but had already changed
the region entry value to a TOMBSTONE, it will now change
the value to REMOVE_PHASE2 instead of leaving it as a TOMBSTONE.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/f2dd4618
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/f2dd4618
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/f2dd4618

Branch: refs/heads/feature/GEODE-2043
Commit: f2dd46182a88e58438b9914d8b27c3638d5e2f1c
Parents: 3ff33be
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Thu Oct 27 16:05:08 2016 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Thu Oct 27 16:05:08 2016 -0700

----------------------------------------------------------------------
 .../internal/cache/AbstractRegionEntry.java     |  15 ++-
 .../internal/cache/AbstractRegionEntryTest.java | 112 +++++++++++++++++++
 2 files changed, 125 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f2dd4618/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
index 4e1f0aa..2138af9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionEntry.java
@@ -209,7 +209,7 @@ public abstract class AbstractRegionEntry implements RegionEntry, HashEntry<Obje
         version.getRegionVersion())) {
       // distributed gc with higher vector version preempts this operation
       if (!isTombstone()) {
-        setValue(r, Token.TOMBSTONE);
+        basicMakeTombstone(r);
         r.incTombstoneCount(1);
       }
       r.getRegionMap().removeTombstone(this, version, false, true);
@@ -220,7 +220,7 @@ public abstract class AbstractRegionEntry implements RegionEntry, HashEntry<Obje
       }
       setRecentlyUsed();
       boolean newEntry = (getValueAsToken() == Token.REMOVED_PHASE1);
-      setValue(r, Token.TOMBSTONE);
+      basicMakeTombstone(r);
       r.scheduleTombstone(this, version);
       if (newEntry) {
         // bug #46631 - entry count is decremented by scheduleTombstone but this is a new entry
@@ -228,6 +228,17 @@ public abstract class AbstractRegionEntry implements RegionEntry, HashEntry<Obje
       }
     }
   }
+  private void basicMakeTombstone(LocalRegion r) throws RegionClearedException {
+    boolean setValueCompleted = false;
+    try {
+      setValue(r, Token.TOMBSTONE);
+      setValueCompleted = true;
+    } finally {
+      if (!setValueCompleted && isTombstone()) {
+        removePhase2();
+      }
+    }
+  }
 
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f2dd4618/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionEntryTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionEntryTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionEntryTest.java
new file mode 100644
index 0000000..36e3e30
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionEntryTest.java
@@ -0,0 +1,112 @@
+/*
+ * 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.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+import org.apache.geode.internal.cache.versions.VersionTag;
+import org.apache.geode.internal.offheap.annotations.Unretained;
+import org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap.HashEntry;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class AbstractRegionEntryTest {
+  
+  @Test
+  public void whenMakeTombstoneHasSetValueThatThrowsExceptionDoesNotChangeValueToTombstone() throws RegionClearedException {
+    LocalRegion lr = mock(LocalRegion.class);
+    RegionVersionVector<?> rvv = mock(RegionVersionVector.class);
+    when(lr.getVersionVector()).thenReturn(rvv);
+    VersionTag<?> vt = mock(VersionTag.class);
+    Object value = "value";
+    AbstractRegionEntry re = new TestableRegionEntry(lr, value);
+    assertEquals(value, re.getValueField());
+    Assertions.assertThatThrownBy(() -> re.makeTombstone(lr, vt))
+    .isInstanceOf(RuntimeException.class)
+    .hasMessage("throw exception on setValue(TOMBSTONE)");
+    assertEquals(Token.REMOVED_PHASE2, re.getValueField());
+  }
+
+  
+  public static class TestableRegionEntry extends AbstractRegionEntry {
+
+    private Object value;
+    
+    protected TestableRegionEntry(RegionEntryContext context, Object value) {
+      super(context, value);
+    }
+    
+    @Override
+    protected Object getValueField() {
+      return this.value;
+    }
+
+    @Override
+    protected void setValueField(Object v) {
+      this.value = v;
+    }
+
+    @Override
+    public void setValue(RegionEntryContext context, @Unretained Object value)
+        throws RegionClearedException {
+      super.setValue(context, value);
+      if (value == Token.TOMBSTONE) {
+        throw new RuntimeException("throw exception on setValue(TOMBSTONE)");
+      }
+    }
+
+    @Override
+    public int getEntryHash() {
+      return 0;
+    }
+
+    @Override
+    public HashEntry<Object, Object> getNextEntry() {
+      return null;
+    }
+
+    @Override
+    public void setNextEntry(HashEntry<Object, Object> n) {
+    }
+
+    @Override
+    public Object getKey() {
+      return null;
+    }
+
+    @Override
+    protected long getlastModifiedField() {
+      return 0;
+    }
+
+    @Override
+    protected boolean compareAndSetLastModifiedField(long expectedValue, long newValue) {
+      return false;
+    }
+
+    @Override
+    protected void setEntryHash(int v) {
+    }
+    
+  }
+}