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 <[email protected]> Authored: Thu Oct 27 16:05:08 2016 -0700 Committer: Darrel Schneider <[email protected]> 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) { + } + + } +}
